Jump to content
  • Advertisement
Sign in to follow this  
Soeholm

OpenGL Can Render Triangle And A Quad, But More Complex Shape Results In Segfault

This topic is 777 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

Hello there!

 

I am currently making my own little game engine with OpenGL, but I have been stuck with this problem for a while now.

I can render a triangle with no problems, but I get a segfault from glDrawElements with a more complex shape.

I'll try and post the relevant code snippets.

 

Here is how I send mesh data to the buffer:

void Mesh::sendToBuffer()
{
	GLuint vbo = 0;
	glGenBuffers(1, &vbo);
	glBindBuffer(GL_ARRAY_BUFFER, vbo);
	glBufferData(GL_ARRAY_BUFFER, verticesBufferSize(), vertices, GL_STATIC_DRAW);


	GLuint ibo;
	glGenBuffers(1, &ibo);
	glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, ibo);
	glBufferData(GL_ELEMENT_ARRAY_BUFFER, indicesBufferSize(), indices, GL_STATIC_DRAW);

	vao = 0;
	glGenVertexArrays(1, &vao);
	glBindVertexArray(vao);
	glEnableVertexAttribArray(0);
	glBindBuffer(GL_ARRAY_BUFFER, vbo);
	glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, ibo);

	// Position
	glVertexAttribPointer(0, numVertices, GL_FLOAT, GL_FALSE, sizeof(Vertex), nullptr);

	// Normals
	glVertexAttribPointer(1, numVertices, GL_FLOAT, GL_FALSE, sizeof(Vertex), (const void *)3);
}

Here is the code for creating a triangle, this works flawlessly:

Mesh Primitives::createTriangle()
{
	Mesh tri;
	tri.numVertices = 3;

	Vertex vertices[] =
	{
			glm::vec3(0.0f, 0.5f, 0.0f), // Position
			glm::vec3(0.0f, 0.0f, 1.0f), // Normal
			glm::vec3(1.0f, 1.0f, 1.0f), // Color

			glm::vec3(0.5f, -0.5f, 0.0f),
			glm::vec3(0.0f, 0.0f, 1.0f),
			glm::vec3(1.0f, 1.0f, 1.0f),

			glm::vec3(-0.5f, -0.5f, 0.0f),
			glm::vec3(0.0f, 0.0f, 1.0f),
			glm::vec3(1.0f, 1.0f, 1.0f)
	};

	tri.vertices = new Vertex[tri.numVertices];
	memcpy(tri.vertices, vertices, sizeof(vertices));

	tri.numIndices = 3;

	GLushort indices[] =
	{
		0, 1, 2
	};
	
	tri.indices = new GLushort[tri.numIndices];
	memcpy(tri.indices, indices, sizeof(indices));

	tri.sendToBuffer();

	return tri;
}

However rendering a cube, or part of a cube as shown here, fails:

Mesh Primitives::createCube()
{
	Mesh cube;
	cube.numVertices = 6;

	Vertex vertices[] =
	{
			glm::vec3(-0.5f, -0.5f, 0.5f),
			glm::vec3(0.0f, 0.0f, 1.0f),
			glm::vec3(5.0f, 5.0f, 5.0f),

			glm::vec3(-0.5f, 0.5f, 0.5f),
			glm::vec3(0.0f, 0.0f, 1.0f),
			glm::vec3(5.0f, 5.0f, 5.0f),

			glm::vec3(0.5f, 0.5f, 0.5f),
			glm::vec3(0.0f, 0.0f, 1.0f),
			glm::vec3(5.0f, 5.0f, 5.0f),

			glm::vec3(-2.5f, -0.5f, 0.5f),
			glm::vec3(0.0f, 0.0f, 1.0f),
			glm::vec3(5.0f, 5.0f, 5.0f),

			glm::vec3(-2.5f, 0.5f, 0.5f),
			glm::vec3(0.0f, 0.0f, 1.0f),
			glm::vec3(5.0f, 5.0f, 5.0f),

			glm::vec3(-1.0f, 0.5f, 0.5f),
			glm::vec3(0.0f, 0.0f, 1.0f),
			glm::vec3(5.0f, 5.0f, 5.0f),
	};

	cube.vertices = new Vertex[cube.numVertices];
	memcpy(cube.vertices, vertices, sizeof(vertices));

	cube.numIndices = 6;

	GLushort indices[] =
	{
		0, 1, 2,		0, 1, 2 // Front
		//4, 5, 6,		6, 7, 4, // Back
		//8, 9, 10,		10, 11, 8, // Bottom
		//12, 13, 14,		14, 15, 12, // Top
		//16, 17, 18,		18, 19, 16, // Left
		//20, 21, 22,		22, 23, 20 // Right
	};

	cube.indices = new GLushort[cube.numIndices];
	memcpy(cube.indices, indices, sizeof(indices));

	cube.sendToBuffer();

	return cube;
}

And to draw the mesh I simply call:

mesh.bind();
glDrawElements(GL_TRIANGLES, mesh.getNumIndices(), GL_UNSIGNED_SHORT, 0);

I think this is the relevant code, the full source is on GitHub: https://github.com/mathiassoeholm/OpenGLPlayground/tree/master/GameEngine

 

I'm sorry for posting so much code here, but I'm at a loss for what causes the Segmentation fault to happen.

 

Any help is much appreciated.

Share this post


Link to post
Share on other sites
Advertisement

You say numIndices = 6, but then you have an array of only three indices?

cube.numIndices = 6;

GLushort indices[] =
{
	0, 1, 2,		0, 1, 2 // Front
	//4, 5, 6,		6, 7, 4, // Back
	//8, 9, 10,		10, 11, 8, // Bottom
	//12, 13, 14,		14, 15, 12, // Top
	//16, 17, 18,		18, 19, 16, // Left
	//20, 21, 22,		22, 23, 20 // Right
};

You seem to be using alot of magic numbers - that can lead to bugs.

Also, you're manually allocating memory (new GLushort) - if done carelessly, that can also lead to bugs.

 

Basically, I have a pretty much zero tolerance for magic numbers and manual allocation of memory (outside of utility-classes) in my code. It just makes the code overall less error-prone.

Share this post


Link to post
Share on other sites

Thanks for the fast reply!

 

There's six indices, it's just the same triangle being repeated (0,1,2), which should still work right?

I tried changing it to: 0, 1, 2, 2, 3, which still doesn't work.

 

I have to agree with you on the magic numbers part. Things are being done very manually here though, because it's a utility method that creates a primitive.

The idea being that all the magic numbers are hidden inside a method call.

I don't think it would makes sense to introduce a constant called NUM_VERTICES_IN_A_TRIANGLE or something like that.

Share this post


Link to post
Share on other sites

Hey!

 

There might be other issues, but this is definitely one. The glVertexAttribPointer needs the offset in bytes (ie, the starting position inside your buffer for that attrib). So in your case, that's 3 * sizeof(float), rather than 3.

Edited by sirpalee

Share this post


Link to post
Share on other sites

There's six indices

 
You're right; the spacing between them confused me.
 

Things are being done very manually here though, because it's a utility method that creates a primitive.
The idea being that all the magic numbers are hidden inside a method call.

All magic numbers ever, are "hidden inside" a function or class or source file. The point is to explain the magic numbers (and gain the benefit of strongly-typed variables), as well as centralizing the location to change, so that different pieces of code relying on the same value don't have to be manually updated if the value changes in the future.
 

I don't think it would makes sense to introduce a constant called NUM_VERTICES_IN_A_TRIANGLE or something like that.

 
Actual lines from my code:

static const size_t VerticesPerQuad = 4;
static const size_t IndicesPerQuad = 6; //Two triangles, sharing 4 vertices, have 6 indices.

 
The point is, when I see something like this:

glVertexAttribPointer(1, numVertices, GL_FLOAT, GL_FALSE, sizeof(Vertex), (const void *)3);

I go, "wait, what is '3' supposed to represent in this context?"

'1' is fine, though.

 

 

For example, isn't the final parameter of glVertexAttribPointer() supposed to be an offset in bytes? Why are you hardcoding a byte offset into your vertex structure?

[Edit:] Bah, I'm super distracted in multiple browser tabs. sirapalee already mentioned that.

 

Also, my OpenGL skills are very poor, but I don't think the second parameter is the number of vertices.  :ph34r:

Edited by Servant of the Lord

Share this post


Link to post
Share on other sites
Oh right, the second parameter is the number of components.

Also, op, you have to double check your classes. For example the mesh class does not cleanup the vbos and ibos when deleted. If those are managed by a different class, then it's a bad idea to store them on the mesh.

Share this post


Link to post
Share on other sites

Thank you so much!

 

Changing the number to 3 (pos + normal + color) gets rid of the Segfault, and the cube is now rendered :-)

 

I've removed some magic numbers, as suggested.

 

Good point about the buffers. I've added a reference count to the Mesh class, which is incremented and decremented by the MeshRenderer class.

When the ref count becomes 0, the Mesh deletes the ibo and vbo.

Share this post


Link to post
Share on other sites
You answer is a bit misleading, so was mine. Just to clarify this, to avoid issues in the future, the second parameter is the number of components for that element. Ie, for positions it's three because you have 3 components, xyz.

Share this post


Link to post
Share on other sites

I think you're still misunderstanding how glVertexAttribPointer is used; the answer is not "3". The absence of crashing doesn't mean the code is correct, unfortunately.  :( 
 
Imagine you have vertex structure like this:

struct Position
{
     float x, y, z;
};

struct Color
{
     uint8_t r, g, b, a;
};

struct Vertex
{
     Position position;
     Color coloration;
};

 
"Position" (in this example) is one entire attribute that has three float components. "Color" is a separate attribute with four uint8 components.
 
In this example, you'd set it up like this:

glVertexAttribPointer(TheIndexOfYourPositionAttribute,
 	3, GL_FLOAT, //Position has three floats. 
 	GL_FALSE, //We don't want it normalized.
 	sizeof(Vertex), //The entire stride, from one Position to the next Position (normally the size of the Vertex).
 	(GLvoid*)offsetof(Vertex, position)); //The offset of the 'Position' struct within the 'Vertex' struct.

glVertexAttribPointer(TheIndexOfYourColorAttribute,
 	4, GL_UNSIGNED_BYTE, //Color has four unsigned bytes. 
 	GL_TRUE, //We want 0-255 normalized to be 0.0 to 1.0.
 	sizeof(Vertex),
 	(GLvoid*)offsetof(Vertex, coloration)); //The offset of the 'Color' struct within the 'Vertex' struct.

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!