tags:

views:

144

answers:

7

I'm getting a Segmentation Fault when trying to call a function that is within a object that is part of a vector of "Shape" Pointers

My problem is in this function::

    Point findIntersection(Point p, Point vecDir, int *status)
    {

        Point noPt;
        for (int i = 0; i < shapes.size(); i++)
        {
         Point temp;
         cout << "Shapes size" << shapes.size() << endl;
**SEGMENTATIONFAULT HERE >>>>>**         bool intersect = shapes[0]->checkIntersect(p, vecDir, &temp);
         if (intersect)
         {
          *status = 1; // Code 1 for intersecting the actual shape
          return temp;
         }

        }

        return noPt;
    }

Initially, I am only adding one shape:

void createScene()
{

    image = QImage(width, height, 32); // 32 Bit

    Sphere s(Point(0.0,0.0,-50.0), 40.0);
    shapes.push_back(&s);
    cout << shapes.size() <<endl;
}

So I have a vector of "shapes" which is global. vector shapes;

I have a class shape

#include "Point.h"
#ifndef SHAPE_H
#define SHAPE_H
using namespace std;
class Shape
{
    public: 
    Shape() {}
    ~Shape(){}
    virtual bool checkIntersect(Point p, Point d, Point *temp) {}; // If intersects, return true else false.
    virtual void printstuff() {};

};
#endif

and also a Sphere class

#include "shape.h"
#include <math.h>
#include <algorithm>
using std::cout;
using std:: endl;
using std::min;

class Sphere : public Shape
{
    public:
    Point centerPt;
    double radius;

    Sphere(Point center, double rad)
    {
     centerPt = center;
     radius = rad;
    }

    bool checkIntersect(Point p, Point vecDir, Point *temp)
    {
     cout << "Hi" << endl;
    /*
     Point _D = p - centerPt;
     double a = Point :: dot(vecDir, vecDir);
     double b = 2 * ( Point :: dot(vecDir, _D) );
     double c = (Point :: dot(_D,_D)) - (radius * radius);

     // Quadratic Equation
     double tempNum = b * b - 4 * a * c;
     if (tempNum < 0)
     {
      return false;
     } else
     {
      double t1 = ( -b + sqrt(tempNum) ) / (2 * a);
      double t2 = ( -b - sqrt(tempNum) ) / (2 * a);
      double t;

      if (t1 < 0 && t2 > 0) { t = t2; }
      else if (t2 < 0 && t1 > 0) { t = t1; }
      else if ( t1 < 0 && t2 < 0 ) { return false; }
      else
      {
       t = min(t1, t2);
      }

      Point p1 = p + (vecDir * t);

      if (p1.z > 0)   // Above our camera
      {
       return false;
      } else 
      {
       temp = &p1;
       return true;
      }

     }
    */
     return false;
    }
};
A: 

Your example will not compile like this. This implementation must return a value:

virtual bool checkIntersect(Point p, Point d, Point *temp) {};

If your global vector of shapes contains a NULL pointer, shapes.size() will be non-zero yet shapes[0]->checkIntersect will fail.

In general, use a debugger (gdb, Visual Studio) and step through your code. Check every variable and you will surely find the problem.

Sebastian
+2  A: 

Apparently you do not have a vector of shapes, but a vector of pointers to shapes. In createScene you add a pointer to a local variable to that vector. When the method finishes, that local is destroyed. When you try to do a call on the object later, you try to make a call on an object that does not exists.

You can solve that problem by using boost::shared_ptr as the argument type of you vector.

// definition of you vector
std::vector< boost::shared_ptr< Shape > > shapes;

void createScene()
{

    image = QImage(width, height, 32); // 32 Bit

    boost::shared_ptr< Shape >( new Sphere s(Point(0.0,0.0,-50.0), 40.0) );
    shapes.push_back(s);
    cout << shapes.size() <<endl;
}

UPDATE: You can also use Boost.PointerContainer instead of a smart pointer, if the container is supposed to actually own the objects.

Space_C0wb0y
Actually, if you create a `boost::shared_ptr` to a local, the local still gets destroyted.
MSalters
Well, sometimes you have to spell it out really clear. Of course, that is not what I meant, and the sample code I added now should make that clear.
Space_C0wb0y
@MSalters, it is being allocated with "new".
Michael Aaron Safyan
Yup, the new code makes the Sphere no longer a local. That's the key change he needs to make. The shared_ptr is not critical, just easy.
MSalters
+3  A: 

Quoting:

Sphere s(Point(0.0,0.0,-50.0), 40.0);
shapes.push_back(&s);

You're putting the address of a local variable in a global collection. Two lines later, s goes out of scope and &s becomes invalid.

MSalters
+4  A: 

You put an address of a local variable in your vector shapes.After exit from your function createScene() this address becomes invalid.

void createScene()
{

    image = QImage(width, height, 32); // 32 Bit

    Sphere s(Point(0.0,0.0,-50.0), 40.0);
    shapes.push_back(&s);
    cout << shapes.size() <<endl;
}
skwllsp
+5  A: 

The problem is here:

Sphere s(Point(0.0,0.0,-50.0), 40.0);
shapes.push_back(&s);

at this point, you've created the Sphere s locally on the stack, and you've pushed its address into your vector. When you leave scope, local objects are freed, and so the address you've stored in your vector now points to memory you no longer own and its contents is undefined.

Instead, do

Sphere *s = new Sphere(Point(0.0,0.0,-50.0), 40.0);
shapes.push_back(s);

to allocate the Sphere from the heap so that it persists. Make sure to delete it when you are done with it.

moonshadow
Or even better, use a shared_ptr<Sphere>
Billy ONeal
@Billy: he still has to allocate the memory he passes to the shared_ptr using new, he can't just pass it an address on the stack; so use of shared_ptr doesn't remove the need to understand what's going on with his original code.
moonshadow
+2  A: 
Michael Aaron Safyan
A: 
void createScene()
{

    image = QImage(width, height, 32); // 32 Bit

    Sphere s(Point(0.0,0.0,-50.0), 40.0);
    shapes.push_back(&s);
    cout << shapes.size() <<endl;
}

your Sphere s(Point(0.0,0.0,-50.0), 40.0); here is a local variable that gets destroyed when CreateScene ends. You push &s, a pointer to s into your shapes vector, so once CreateScene finishes (i.e. after the last cout statement above) shapes[0] points to some destroyed object. Change to:

void createScene()
{

    image = QImage(width, height, 32); // 32 Bit

    Sphere* s_p = new Sphere(Point(0.0,0.0,-50.0), 40.0);
    shapes.push_back(s_p);
    cout << shapes.size() <<endl;
}

And don't forget to delete all items in shapes before you remove them from shapes.

jilles de wit