Jump to content

  • Log In with Google      Sign In   
  • Create Account


#ActualNercury

Posted 10 March 2013 - 03:48 AM

Ok, I am just going to glance over your code...

 

for(mAngle = 0.0; mAngle <= 2 * 3.14; mAngle += (3.14/this->mNumSegments))
{
    //
}

 

 

Never use floating-point values for iteration, because floating point errors accumulate and can cause problems sooner than you think. Use integers:

 

double segmentSize = 3.14/this->mNumSegments;
for (int segment = 0; segment <= this->mNumSegments; segment++) {
     mAngle = segment * segmentSize;
}

 

 

You would make your life easier if you avoid modifying or creating your data on draw. Set up everything beforehand, and just read what to draw.

I especially suggest to rethink this one:

 

points.reserve(i);
points.resize(i);
points.push_back(new Points);

glColor3d(1,0,0);
for(vector<Points*>::iterator i = points.begin(); i != points.end(); i++)
{
	Points* p = (*i);
	if(p != NULL) {
		p->setPosition(mXpos + mRadius * cos(mAngle),
				  mYpos + mRadius * sin(mAngle));
		glPointSize(5);
		p->draw();
	}
	else
	{
		delete p;
		p = 0;
	}
}

 

It looks like it would be extremely hard to manage point lifecycle when they are created and deleted in this way. Of course, I do notice that you try to attempt to delete a null pointer sometimes:

Points* p = (*i);
if(p != NULL) {
	// ...
}
else
{
	delete p; // p is NULL!
	p = 0;
}

 

And.. it looks like for every new triangle point, you are iterating and drawing all the points.

Ok, so my advice is to create/delete objects away from drawing, use update step for that. Only read data in draw.


#2Nercury

Posted 10 March 2013 - 03:48 AM

Ok, I am just going to glance over your code...

 

for(mAngle = 0.0; mAngle <= 2 * 3.14; mAngle += (3.14/this->mNumSegments))
{
    //
}

 

 

Never use floating-point values for iteration, because floating point errors accumulate and can cause problems sooner than you think. Use integers:

 

double segmentSize = 3.14/this->mNumSegments;
for (int segment = 0; segment <= this->mNumSegments; segment++) {
     mAngle = segment * segmentSize;
}

 

 

You would make your life easier if you avoid modifying or creating your data on draw. Set up everything beforehand, and just read what to draw.

I especially suggest to rethink this one:

 

points.reserve(i);
points.resize(i);
points.push_back(new Points);

glColor3d(1,0,0);
for(vector<Points*>::iterator i = points.begin(); i != points.end(); i++)
{
	Points* p = (*i);
	if(p != NULL) {
		p->setPosition(mXpos + mRadius * cos(mAngle),
				  mYpos + mRadius * sin(mAngle));
		glPointSize(5);
		p->draw();
	}
	else
	{
		delete p;
		p = 0;
	}
}

 

It looks like it would be extremely hard to manage point lifecycle when they are created and deleted in this way. Of course, I do notice that you try to attempt to delete a null pointer sometimes:

Points* p = (*i);
if(p != NULL) {
	// ...
}
else
{
	delete p; // p is NULL!
	p = 0;
}

 

And.. it looks like for every new triangle point, you are iterating and drawing all the points.

Ok, so my advice is to create/delete objects away from drawing, use update step for that. Use draw only for reading data to draw.


#1Nercury

Posted 10 March 2013 - 03:46 AM

Ok, I am just going to glance over your code...

 

for(mAngle = 0.0; mAngle <= 2 * 3.14; mAngle += (3.14/this->mNumSegments))
{
    //
}

 

 

Never use floating-point values for iteration, because floating point errors accumulate and can cause problems sooner than you think. Use integers:

 

double segmentSize = 3.14/this->mNumSegments;
for (int segment = 0; segment <= this->mNumSegments; segment++) {
     mAngle = segment * segmentSize;
}

 

 

You would make your life easier if you avoid modifying or creating your data on draw. Set up everything beforehand, and just read what to draw.

I especially suggest to rethink this one:

 

points.reserve(i);
points.resize(i);
points.push_back(new Points);

glColor3d(1,0,0);
for(vector<Points*>::iterator i = points.begin(); i != points.end(); i++)
{
	Points* p = (*i);
	if(p != NULL) {
		p->setPosition(mXpos + mRadius * cos(mAngle),
				  mYpos + mRadius * sin(mAngle));
		glPointSize(5);
		p->draw();
	}
	else
	{
		delete p;
		p = 0;
	}
}

 

It looks like it would be extremely hard to manage point lifecycle when they are created and deleted in this way. Of course, I do notice that you try to attempt to delete a null pointer sometimes:

Points* p = (*i);
if(p != NULL) {
	// ...
}
else
{
	delete p; // p is NULL!
	p = 0;
}

 

And.. it looks like for every new triangle point, you are iterating and drawing all the points.

Ok, so my advice is to create/delete objects away from drawing, use update step before drawing things for that. Use draw only for reading data to draw.


PARTNERS