Jump to content
  • Advertisement
Sign in to follow this  
Kronecker

Yet another STL:bad_alloc Issue

This topic is 4592 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Sorry for such a newbie-esque question, I'm sure this will be easy to answer for one of you C++ gurus out there. I'm traditionally a Java programmer with knowledge of C, and this project is my first attempt at a real foray into C++. I have searched through multiple books, google, and these forums, but I can't find a reason why (and a fix) I get a bad_alloc exception thrown in my code. I dont use any STL classes, I dont have any arrays (yet), and The place its being thrown doesnt seem to even have a new call. Sorry for posting so much code, but here it goes (the exception appears to happen right after the "hmm" comment):
CollisionPacket* Sphere::getIntersectionRays(Line* l){
	Point* collisionPoint;
	Line* transLine;
	Line* reflLine;
	Vect* unitNorm;
	Vect* incident;
	Vect* s1;
	Vect* s2;
	float t;
	float a = pow(l->r0->i,2) + pow(l->r0->j,2) + pow(l->r0->k,2);
	float b = 2.0*(l->r0->i*(l->p0->x-x0) +
				   l->r0->j*(l->p0->y-y0) + 
				   l->r0->k*(l->p0->z-z0));
	float c = pow(l->p0->x-x0,2)+pow(l->p0->y-y0,2)+pow(l->p0->z-z0,2) - r*r;

	float disc = b*b-4*a*c;

	if(disc < -ZERO){
		return NULL;
	} else if(disc <= ZERO && disc >= -ZERO){
		t = -b / (2*a);
		if(t <= 0)
			return NULL;
	} else {
		float t0 = (-b + sqrt(disc)) / (2*a);
		float t1 = (-b - sqrt(disc)) / (2*a);

		if(t0 < t1){
			if(t0 > 0)
				t = t0;
			else if(t1 > 0)
				t = t1;
			else
				return NULL;
		} else if(t1 < t0){
			if(t1 > 0)
				t = t1;
			else if(t0 > 0)
				t = t0;
			else
				return NULL;
		} else {
			return NULL;
		}
	}
	float x = l->p0->x+l->r0->i*t;
	float y = l->p0->y+l->r0->j*t;
	float z = l->p0->z+l->r0->k*t;
	//---------------------------------
	//         Hmmm.......
	//---------------------------------
	collisionPoint = l->getPointAtTime(t);
	printf("Collision at (%0.2f, %0.2f, %0.2f)\n",collisionPoint->getX(),collisionPoint->getY(),collisionPoint->getZ());

	 
	unitNorm = new Vect((collisionPoint->x-x0)/r, 
							(collisionPoint->y-y0)/r, 
							(collisionPoint->z-z0)/r);

	if(mat->getTrans() > 0.0){
		// application of snell's law
		float n1 = 0.0;
		if(l->getLastMaterial() == NULL){
			n1 = REFRACT_AIR;
		} else {
			n1 = l->getLastMaterial()->getRefr();
		}
		float n2 = mat->getRefr();
		incident = l->r0->getNormalized();
		incident->setI(-incident->i);
		incident->setJ(-incident->j);
		incident->setK(-incident->k);
		s1 = unitNorm->scalarMultiple(unitNorm->dot(incident))->sub(incident);
		s2 = s1->scalarMultiple(n1/n2);
		transLine = new Line(collisionPoint, s2->sub(unitNorm), l->getIntensity()*mat->getTrans());
		transLine->setLastMaterial(mat);
		//delete(s1);
		//delete(s2);
		//delete(incident);
	}

	if(mat->getRefl() > 0.0){
		reflLine = new Line(collisionPoint, l->getR0()->findReflection(unitNorm), l->getIntensity()*mat->getRefl());
		reflLine->setLastMaterial(l->getLastMaterial());
	}

	//delete(unitNorm);

	return new CollisionPacket(collisionPoint,transLine,reflLine);
}


Here is the getPointAtTime(t) function as well:
Point* Line::getPointAtTime(float t){
	float x = this->p0->x+this->r0->i*t;
	float y = this->p0->y+this->r0->j*t;
	float z = this->p0->z+this->r0->k*t;
	return new Point(x,y,z);
}


The exception also happens before the new Point(blabla) line is called. This has been driving me crazy for almost a week now, and any help will be greatly appreciated. If you're curious, I'm attempting to write a raytracer. [Edited by - Kronecker on April 20, 2006 11:31:26 PM]

Share this post


Link to post
Share on other sites
Advertisement
You seem to be using a lot of calls to new. In C++, unlike (IIRC) Java, you can instantiate objects on the stack just as you can instantiate integers, floats etc.

Maybe the sheer number of new calls is causing the problem.

Share this post


Link to post
Share on other sites
A lot of lines that look like: //delete(....)


Memory leak city, all your deletes are commented out!!! In Java, garbage collection would kick in and free up memory from all those news you have. This is not Java, which means that memory will be hoarded like pirate loot, and you will run out of memory on one of those other calls to new - which throws std::bad_alloc.

Quote:
The exception also happens before the new Point(blabla) line is called.


I find it likely you're misinterpreting your debugger or are running in release mode where lines do not allways directly correspond to the generated code - but what line do you believe it to be throwing the exception from?

Share this post


Link to post
Share on other sites
Quote:
Original post by MaulingMonkey
A lot of lines that look like: //delete(....)


Memory leak city, all your deletes are commented out!!! In Java, garbage collection would kick in and free up memory from all those news you have. This is not Java, which means that memory will be hoarded like pirate loot, and you will run out of memory on one of those other calls to new - which throws std::bad_alloc.


I commented those out because I saw somewhere that delete could throw bad_alloc... It happened before I put the comments in.

edit: Oh, and as far as the memory usage is concerned... I haven't even finished the raytracer, this exception occurrs during the following tiny main function:


int main(){
Sphere* s1 = new Sphere(0.0,0.0,0.0,2.0,new Material(1.0,0,0,0.5,0.5,1.33));
Line* l1 = new Line(3,1,0,-1,0,0,1.0);
printf("Incoming Line:\n %s\n",l1->toString());

CollisionPacket* cp;

cp = s1->getIntersectionRays(l1);

if(cp != NULL){
if(cp->getCollisionPoint() != NULL){
printf("%s ->> Collision Point\n",cp->getCollisionPoint()->toString());
}
if(cp->getReflectLine() != NULL){
printf("Reflection Line:\n %s\n",cp->getReflectLine()->toString());
}
if(cp->getTransmitLine() != NULL){
printf("Refraction Line:\n %s\n",cp->getTransmitLine()->toString());
}
}
return 0;
}

Share this post


Link to post
Share on other sites
Quote:
Original post by Kronecker
I commented those out because I saw somewhere that delete could throw bad_alloc... It happened before I put the comments in.


Delete isn't supposed too throw bad_alloc...

How much memory is this program taking up when it throws? Edit: Basically answered in your edit :)

Nothing obviously suspect in the code posted so far...

Share this post


Link to post
Share on other sites
WOW

I feel totally ashamed. Even as a java programmer, I know what an array out-of-bounds problem is, I just didn't see it in this code because we dont get handy stack traces in C++.

I had the character array (string) bounds set too low for the Line class's toString functions.

Wow. Ok, at least I wont make this mistake again.

/noob

Share this post


Link to post
Share on other sites
Quote:
printf printf printf

Quote:
I had the character array (string) bounds set too low


It's not often you get to see bad C-flavoured habits AND bad Java-flavoured habits in the same C++ program. :) (Plus a general lack of good factoring that's a bad habit in any language.)

This is how we do things in C++:


struct vec3 {
float x, y, z;
// A full suite of arithmetic overloads can go here. I'm only showing the
// ones that I will actually use.
float squaredMagnitude() { return x*x + y*y + z*z; }
vec3& operator-=(const vec3& other) {
x -= other.x; y -= other.y; z -= other.z;
return *this;
}
};

inline vec3 operator-(const vec3& lhs, const vec3& rhs) {
return (vec3(lhs) -= rhs);
}

inline float dot(const vec3& lhs, const vec3& rhs) {
return lhs.x * rhs.x + lhs.y * rhs.y + lhs.z * rhs.z;
}

std::ostream& operator<<(std::ostream& os, const vec3& v) {
os << "(" << v.x << ", " << v.y << ", " << v.z << ")";
}

// Now we go and replace i,j,k of Line.r0, x,y,z of Line.p0, etc. with
// vec3s, and do:

CollisionPacket Sphere::getIntersectionRays(const Line& l) {
float a = l.r0.squaredMagnitude();
float b = 2.0*(dot(l.r0, l.p0 - position));
// where "position" is the vec3 put together from (x0, y0, z0) which I
// assume are Sphere members. By the way, why be so sparing with your
// variable names?
float c = (l.p0 - position).squaredMagnitude();
float discriminant = b*b-4*a*c;

float t;
// Get the smallest positive root.
if (discriminant < -ZERO) {
t = -1; // we'll check after the if-condition, in one place, to ensure
// we found a positive root.
} else if (discriminant <= ZERO) {
// This is an ELSE if, so we already *know* the discriminant is >= -ZERO.
t = -b / (2*a);
} else {
// We know the discriminant is > ZERO, which makes the sqrt valid and
// positive. Also, since a is a squaredMagnitude, it is positive. We are
// forced to conclude that (using the old values) t0 > t1 always.
t = (-b - sqrt(disc)) / (2*a);
if (t < 0) {
// then try the larger root. If it's still negative, we'll catch that
// at the end.
t = (-b + sqrt(disc)) / (2*a);
}
}
if (t < 0) {
return CollisionPacket::NO_COLLISION;
// Some special instance of the class representing a non-collision.
}

// You used to calculate some variables "x, y, z" here, but didn't use them.

// Uh? You already had a Point class, but seemed not to be using it for
// all kinds of things it could be used for, and also put a bulky interface
// on it?
vec3 collisionPoint = l.getPointAtTime(t);

// Behold, modern C++ I/O.
streamsize oldPrecision = std::cout.precision();
cout.precision(2);
std::cout << "Collision at " << collisionPoint;
// That's the point (pun intended) of the helper operator<< above.

// And similarly a Vect class that's also just a triple of points. There are
// ways to make these as separate classes wrapping around the base vec3
// implementation while maintaining type safety, but for simplicity:
vec3 unitNormal = (collisionPoint - position) / radius;
float transparency = mat.transparency();

Line transLine; // leave as a default object if we are opaque.
if (transparency > 0.0) {
// application of snell's law
// Don't violate the 'Law of Demeter'. Use delegation here instead:
// Have the Line implement getRefraction by delegating to its material
// or returning a default if there is none).

vec3 incident = -l.r0.normalized();
// The negation here takes care of that weird setI/J/K logic you had.
// By the way, don't you think it's a bit strange to have a mutator for
// a public member?
vec3 s1 = unitNormal * dot(unitNormal, incident) - incident;
vec3 s2 = s1 * (l.getRefraction() / mat.getRefraction());
transLine = Line(collisionPoint, s2 - unitNormal,
l.intensity() * transparency);
// Unless you can show me that getLastMaterial and setLastMaterial do more
// than a direct get/set, I will be telling you to use a public member.
// Sorry, but the alternative is adding extra code that won't really
// protect you or create a meaningful abstraction.
transLine.lastMaterial = mat;
}
Line reflLine; // Leave as default object if material is non-reflective.
float reflectivity = mat.reflectivity();
if (reflectivity > 0.0){
reflLine = Line(collisionPoint, l.r0.findReflection(unitNormal),
l.intensity * reflectivity);
reflLine.lastMaterial = l.lastMaterial;
}
return CollisionPacket(collisionPoint, transLine, reflLine);
}



As for a "Line::toString()", outputting objects that way is idiomatic for Java but not for C++. In C++, you feed objects to a stream with the appropriate operator overload, as demonstrated by the vec3 version in the above code. But for the cases where you *do* want a string in memory - the C++ standard library provides std::string; for heavens' sake, use it. You don't muck around with reallocating and concatenating char[]'s in Java, do you? (C++'s std::string is a mutable object, so you have to be careful if you want to use it as a key for an associative container, but it lets you treat it like a StringBuffer without having to convert back to String for printing.)

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
C++'s std::string is a mutable object, so you have to be careful if you want to use it as a key for an associative container
Forgive me for the OT, but how would you go about "being careful"? Do you make the key template type a const std::string? If so, and in general, should all key types be const?


jfl.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!