1

I have an OpenGL GLUT-based (yes, I know, GLUT is ancient) C++ program that alternatingly prints out a blue rectangle and a red square, located at the point in the GLUT window where the user clicks the mouse. This is working fine.

I encountered a mind-boggling amount of trouble, however, when I attempted to modify the above program so that the aforementioned shapes REMAIN on the canvas after their initial animation.

I have created a Shape class which holds such information as the number of vertices, color, coordinates, etc. The class seems fully functional when the program is only drawing one shape at a time.

In order to solve the multiple-shapes-at-once problem, I created a std::list<Shape> linked list. However, when I iterate through the linked list via the std::list<Shape>::iterator mechanism, the objects appear to be tied together in memory. That is, iteration yields the exact same shape object, coordinates and all, for each index in the Linked List.

I have tried the following solutions:

making the linked list a std::list<Shape*> instead of a std::list<Shape>,

  • utilizing the heap for object allocation via Shape* my_shape = new Shape(params),
  • and combining the the two above methods.

Here is my void display() GLUT function, along with the relevant global variables AND the class definition/declaration:

class Shape
{

public:
    Shape();
    Shape(int, double[], int[]);
    int type; //0 = rectangle, 1 = circle, 2 = triangle
    int numVertices; //stores total number of vertices in the 2D object.
    double* vertexArray; //a dynamic array that stores each vertex's (x, y)-coordinates in alternating successive indices
    int* rgb; //an array that contains the 3 rgb values s.t. rgb = {r, g, b}
    double* center; //an array that contains the (x, y)-coordinates of the shape's center on the 2d plane.
    int velocity[2]; //a array of size 2 that holds the object's x-velocity in index 0 and the object's y-velocity in index 1

};

    //default Shape constructor
Shape::Shape()
{

}

//Shape constructor 
Shape::Shape(int shapeType, double vertices[], int color[]) //constructor for creating a stationary 2D shape
{
    type = shapeType;

    if (shapeType!=1) //as long as shape is NOT a circle, interpret the second constructor parameter as a list of vertices 
    {
        vertexArray = vertices;
    }

    rgb = color;

    if (shapeType==0) //shape is a rectangle
    {
        numVertices = 4;
    }
    else if(shapeType==1) //shape is a circle
    {
        //shape is a circle, therefore the second array param is in fact an array of size 2 containing the (x, y)-coordinates of the circle origin...
        center = vertices;
    }
    else if (shapeType==2) //shape is a triangle
    {
        numVertices = 3; 
    }
}


std::list<Shape> shapeList;
void my_display(void) 
{
  /* clear the buffer */
  glClear(GL_COLOR_BUFFER_BIT);


  //altFlag is just a value to allow alternating between rectangles/circles being printed

  if (altFlag==1)
  {
      printf("Drawing rectangle at: (%g, %g)\n", my_x, my_y);

      /*instantiate a Shape() object representing a blue rectangle*/
      int rgbColor[3] = {0, 0, 1};

      double vertices[4] = {my_x/window_w, my_y/window_h, my_x/window_w + my_rect_w, my_y/window_h + my_rect_h};

      Shape my_rectangle(0, vertices, rgbColor); //uses constructor (shape type, list of vertex coordinates, length of coordinate list, color)

      glColor3f((GLfloat)my_rectangle.rgb[0], (GLfloat)my_rectangle.rgb[1], (GLfloat)my_rectangle.rgb[2]) ; /* (Red, Green, Blue); so here we ask for Blue */
      glRectf(my_rectangle.vertexArray[0], my_rectangle.vertexArray[1], my_rectangle.vertexArray[2], my_rectangle.vertexArray[3]); //call to function to draw a rectangle
      altFlag=0;

      shapeList.push_front(my_rectangle);
  }
  else
  {
      /*instantiate a Shape() object representing a red circle*/
      int circleColor[3] = {1, 0, 0};
      double circleCenter[2] = {(my_x), (my_y)}; //{center x coord, center y coord}
      //Shape* my_circle = new Shape(1, circleCenter, circleColor); 
      Shape my_circle(1, circleCenter, circleColor);

      glColor3f(my_circle.rgb[0], my_circle.rgb[1], my_circle.rgb[2]);
      glCirclef(my_circle.center[0], my_circle.center[1]); //call to function to draw pseudocircle

      altFlag=1;

      shapeList.push_front(my_circle);
  }

  //iterate over shapeList, print out values of the rgb array.
  for (std::list<Shape>::iterator iter = shapeList.begin(); iter != shapeList.end(); iter++)
  {
      printf("%d, %d, %d\n", iter->rgb[0], iter->rgb[1], iter->rgb[2]);
  }

  glutSwapBuffers();

  return;

While this is part of an assignment, the question pertains to the language being used, as opposed to the graphics library which is the focus of the course.

genpfault
  • 51,148
  • 11
  • 85
  • 139
insomniac
  • 131
  • 14
  • Please show the definition of `Shape` and its member functions (at least the constructor). – Angew is no longer proud of SO Aug 29 '13 at 11:46
  • You've been writing too much Java. There are so many memory management mistakes in that code that it's impossible to point them all out. At the very least, use `std::vector` for your arrays. – Nicol Bolas Aug 29 '13 at 11:52
  • I completely agree; I wish C++ was taught in more classes. Alas, this is how I must learn. The embarrassing mistakes of today will become invaluable lessons for tomorrow. – insomniac Aug 29 '13 at 11:55
  • Hint: use inline code formatting (backtick marks) to prevent the display "eating" angle brackets, as I did in editing your question; it makes the post even more readable. – Angew is no longer proud of SO Aug 29 '13 at 12:04
  • Also, for what it's worth, I took this course because it's literally the only one that uses mostly C++. The game dev graphics stuff is just a bonus. – insomniac Aug 29 '13 at 12:06

1 Answers1

2

Your parameterised constructor of Shape does this:

vertexArray = vertices;

Where vertices is a function parameter declared as double vertices[]. This is syntactic sugar to double *vertices, i.e., you're copying a pointer (because arrays decay to pointers when passed to functions).

vertexArray is defined as a pointer inside the class. You even mention it's a "dynamic array," but then you initialise it with an array allocated on the stack (a local variable inside display()). This makes the pointer dangling.

You will have to make vertexArray an array and copy the data, not just the pointer. Something like this:

class Shape
{
  // ...
  double vertexArray[4];
  // ...
};

Shape::Shape(int shapeType, double vertices[], int color[])
{
    type = shapeType;

    if (shapeType==0) //shape is a rectangle
    {
        numVertices = 4;
    }
    else if(shapeType==1) //shape is a circle
    {
        //shape is a circle, therefore the second array param is in fact an array of size 2 containing the (x, y)-coordinates of the circle origin...
        center = vertices;
    }
    else if (shapeType==2) //shape is a triangle
    {
        numVertices = 3; 
    }

    if (shapeType!=1) //as long as shape is NOT a circle, interpret the second constructor parameter as a list of vertices 
    {
        std::copy(vertices, vertices + numVertices, vertexArray);
    }
}

And analogically for color as well, of course.

Extra suggestions

Of course, this code would be much better using std::vector or something similar instead of plain arrays, using and enum instead of int for shapeType etc. I suggest you pick up a good C++ book.

Community
  • 1
  • 1
Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • Thank you so much for the detailed feedback! I'm off to class but will look at this in detail within the hour. – insomniac Aug 29 '13 at 12:13
  • You have helped SO MUCH! In addition to opening my eyes in the direction of my (now obviously foolish) mistake, you showed me a fantastic STL copy algorithm for future use. – insomniac Aug 29 '13 at 13:44