-1

There is this typedef for openGL

typedef struct
{
float Position[3];
float Color[4];
} Vertex;

The example hard-codes the postions and colors which works:

Vertex Vertices[] =
{
{{1, -1, 0},   {1, 0, 0, 1}},
{{1, 1,  0},    {1, 0, 0, 1}},
{{-1, 1, 0},   {0, 1, 0, 1}},
{{-1, -1, 0},  {0, 1, 0, 1}},
{{1, -1, -1},  {1, 0, 0, 1}},
{{1, 1, -1},   {1, 0, 0, 1}},
{{-1, 1, -1},  {0, 1, 0, 1}},
{{-1, -1, -1}, {0, 1, 0, 1}}
};

I want to NOT hard-code my colors and positions so that I can create a class with a constructor for my OpenGL drawing data.

I was wondering why constructing my data as below doesn't work and what the correct way to do it would be.

Vertices                = malloc(sizeof(Vertex)*8); //make 8 vertice pointers
Vertices[0].Position[0] = 1;    Vertices[0].Position[1] = -1; Vertices[0].Position[2] = 0;
Vertices[0].Color[0]    = 1;    Vertices[0].Color[1]    = 0;    Vertices[0].Color[2]    = 0; Vertices[0].Color[3]= 1;

Vertices[1].Position[0] = 1;    Vertices[1].Position[1] = 1;    Vertices[1].Position[2] = 0;
Vertices[1].Color[0]    = 1;    Vertices[1].Color[1]    = 0;    Vertices[1].Color[2]    = 0; Vertices[1].Color[3]= 1;

Vertices[2].Position[0] = -1; Vertices[2].Position[1] = 1;  Vertices[2].Position[2] = 0;
Vertices[2].Color[0]    = 0;    Vertices[2].Color[1]    = 1;    Vertices[2].Color[2]    = 0; Vertices[2].Color[3]= 1;

Vertices[3].Position[0] = -1; Vertices[3].Position[1] = -1; Vertices[3].Position[2] = 0;
Vertices[3].Color[0]    = 0;    Vertices[3].Color[1]    = 1;    Vertices[3].Color[2]    = 0; Vertices[3].Color[3]= 1;

Vertices[4].Position[0] = 1;    Vertices[4].Position[1] = -1; Vertices[4].Position[2] = -1;
Vertices[4].Color[0]    = 1;    Vertices[4].Color[1]    = 0;    Vertices[4].Color[2]    = 0;  Vertices[4].Color[3]= 1;

Vertices[5].Position[0] = 1;    Vertices[5].Position[1] = 1;    Vertices[5].Position[2] = -1;
Vertices[5].Color[0]    = 1;    Vertices[5].Color[1]    = 0;    Vertices[5].Color[2]    = 0;  Vertices[5].Color[3]= 1;

Vertices[6].Position[0] = -1; Vertices[6].Position[1] = 1;  Vertices[6].Position[2] = -1;
Vertices[6].Color[0]    = 0;    Vertices[6].Color[1]    = 1;    Vertices[6].Color[2]    = 0;  Vertices[6].Color[3]= 1;

Vertices[7].Position[0] = -1; Vertices[7].Position[1] = -1; Vertices[7].Position[2] = -1;
Vertices[7].Color[0]    = 0;    Vertices[7].Color[1]    = 1;    Vertices[7].Color[2]    = 0;  Vertices[7].Color[3]= 1;

Although printing out both data structures look identical with

    NSLog(@"print it");
for(unsigned int i = 0; i < 8; i ++)
{
    printf("i: %f\t%f\t%f\t%f\t%f\t%f\t%f\n", Vertices[i].Position[0], Vertices[i].Position[1], Vertices[i].Position[2], Vertices[i].Color[0], Vertices[i].Color[1], Vertices[i].Color[2], Vertices[i].Color[3]);
}

my open GL cube does not draw

user1709076
  • 2,538
  • 9
  • 38
  • 59

3 Answers3

3

As the author of the question posted a link to the source code in use in comment to Sami Kuhmonen's question, the problem can be indentified to this part:

glBufferData(GL_ARRAY_BUFFER, sizeof(Vertices), Vertices, GL_STATIC_DRAW);

You can't exchange Vertex Vertices[8] by Vertex* Vertices and expect this to still work. Arrays and pointers are not the same thing in C/C++. sizeof on the array will return the size of the whole array in bytes (in this case, 8*sizeof(Vertex), while sizeof on the pointer returns the size of the pointer, (so typically 4 or 8 on an unsual platform). So you currently load only the first 1 or 2 floats into the buffer.

derhass
  • 43,833
  • 2
  • 57
  • 78
  • Thanks derhass, I tried changing Vertex *Vertices to Vertex Vertices[8], to no avail. I also then tried leaving Vertex *Vertices and changing the line to glBufferData(GL_ARRAY_BUFFER, sizeof(Vertices)*8, Vertices, GL_STATIC_DRAW); – user1709076 Oct 11 '15 at 16:39
2

First, don't use sizeof(int) to get the size of a pointer. Use the appropriate type, in this case sizeof(Vertex*).

But the main problem is in the memory allocation. You don't have an array of pointers, you have an array. So you should allocate the memory based on that. If you want eight vertices and assuming you have Vertex* Vertices, then

Vertices = malloc(sizeof(Vertex) * 8);

This allocates enough memory to store them in one block.

Sami Kuhmonen
  • 30,146
  • 9
  • 61
  • 74
  • Thanks Sami, let me try this. I thought since the size of a pointer (not the data it points to) is always an 'int', that I should malloc size of int – user1709076 Oct 11 '15 at 15:21
  • Sami, I did do this and have gone out to print both my data structure and the hard-coded one (which look the same) however - something is not 'the same' about them since my cube doesn't draw in openGL. I am using this tutorial by ray wenderlich: http://www.raywenderlich.com/3664/opengl-tutorial-for-ios-opengl-es-2-0 – user1709076 Oct 11 '15 at 16:00
0

Posting as a response to all the great feedback from Sami and Derhas, the code works for me when i do the following 'not hard-coding' the vertex points or colors from the get-go (so i can later put this inside a cube class that takes position and color as input to my constructor)

Vertex Vertices[8];

.....

Vertices[0].Position[0] = 1;    Vertices[0].Position[1] = -1; Vertices[0].Position[2] = 0;
Vertices[0].Color[0]    = 1;    Vertices[0].Color[1]    = 0;    Vertices[0].Color[2]    = 0; Vertices[0].Color[3]= 1;


Vertices[1].Position[0] = 1;    Vertices[1].Position[1] = 1;    Vertices[1].Position[2] = 0;
Vertices[1].Color[0]    = 1;    Vertices[1].Color[1]    = 0;    Vertices[1].Color[2]    = 0; Vertices[1].Color[3]= 1;

Vertices[2].Position[0] = -1; Vertices[2].Position[1] = 1;  Vertices[2].Position[2] = 0;
Vertices[2].Color[0]    = 0;    Vertices[2].Color[1]    = 1;    Vertices[2].Color[2]    = 0; Vertices[2].Color[3]= 1;

Vertices[3].Position[0] = -1; Vertices[3].Position[1] = -1; Vertices[3].Position[2] = 0;
Vertices[3].Color[0]    = 0;    Vertices[3].Color[1]    = 1;    Vertices[3].Color[2]    = 0; Vertices[3].Color[3]= 1;

Vertices[4].Position[0] = 1;    Vertices[4].Position[1] = -1; Vertices[4].Position[2] = -1;
Vertices[4].Color[0]    = 1;    Vertices[4].Color[1]    = 0;    Vertices[4].Color[2]    = 0;  Vertices[4].Color[3]= 1;

Vertices[5].Position[0] = 1;    Vertices[5].Position[1] = 1;    Vertices[5].Position[2] = -1;
Vertices[5].Color[0]    = 1;    Vertices[5].Color[1]    = 0;    Vertices[5].Color[2]    = 0;  Vertices[5].Color[3]= 1;

Vertices[6].Position[0] = -1; Vertices[6].Position[1] = 1;  Vertices[6].Position[2] = -1;
Vertices[6].Color[0]    = 0;    Vertices[6].Color[1]    = 1;    Vertices[6].Color[2]    = 0;  Vertices[6].Color[3]= 1;

Vertices[7].Position[0] = -1; Vertices[7].Position[1] = -1;     Vertices[7].Position[2] = -1;
Vertices[7].Color[0]    = 0;    Vertices[7].Color[1]    = 1;    Vertices[7].Color[2]    = 0;  Vertices[7].Color[3]= 1;


....

- (void)setupVBOs
{
GLuint vertexBuffer;
glGenBuffers(1, &vertexBuffer);
glBindBuffer(GL_ARRAY_BUFFER, vertexBuffer);

//works with Vertices[]
//glBufferData(GL_ARRAY_BUFFER, sizeof(Vertices), Vertices, GL_STATIC_DRAW);
glBufferData(GL_ARRAY_BUFFER, sizeof(Vertices)*8, Vertices, GL_STATIC_DRAW);

GLuint indexBuffer;
glGenBuffers(1, &indexBuffer);
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indexBuffer);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, sizeof(Indices), Indices, GL_STATIC_DRAW);
}
user1709076
  • 2,538
  • 9
  • 38
  • 59