Bad performance when rendering medium amount of meshes

When I was working on performance with alot of meshs, I found it very profitable to 'flag' which of my meshs was static, and then, on runtime, combine all static meshs into one large mesh object and then render that. It worked rather well because it was iterating through a small number of meshs (the vertex count was high for each mesh, however)

Do you have a lot of overdraw (many overlapping pixels)? Since what you described is the amount of vertices you have, when there are A LOT more pixels than vertices when rendering a mesh. You can quickly check whether fill rate (overdraw, or just slow pixel shader) is a problem by changing the window size (changes number of pixels but keeps vertices the ~same).

EDIT:

btw, you can associate the index buffer with the VAO (just like VBOs) if you dont specifically need to use multiple index buffers with the same VAO

and you probably should use 16-bit indices if your meshes only have ~28 verts.

It is the number of draw calls you are making. Every time you call glDrawElements you incur an overhead. You are much better off combining meshes together. Like sethhope said, you will want to combine anything static into batches. If you have a bunch of crates in your scene, for example, combine them all into a single mesh and draw that once instead of drawing them individually.

Yeah, draw calls. Around ~1,000 is a decent maximum to aim for. ~4,000 is a rough upper limit for the older APIs.

You want both instancing and mesh combining (baking). Which is better is a trade-off you have to evaluate for your specific case.

Baking "all crates" is a bad idea, since that creates a single mesh that spans your whole level, which is really bad for culling and any dynamic bounding box system you have in place. You can bake localized clusters of objects, but then you lose out on instancing, and of course the objects must be static for baking for be possible.

Instancing doesn't scale forever, though, so just relying on instancing to solve everything isn't a guaranteed solution either. But for ~7,000 objects, it's almost probably what you want, assuming most of those 7,000 objects are the same mesh drawn with different transforms.

How are you measuring time? I'm guessing that's total CPU per frame?
Add some more timing code to measure glSwapBuffers, so you can exclude it from the per/frame total. Also get some timings for how long your mesh loop takes.
You can also use ARB_timer_query to measure GPU time per frame.

If your problem is that your GPU time per frame is the bottleneck, then you'll have to optimize your shaders / data formats / overdraw / etc.
If you problem is your CPU e per frame is the bottleneck, then it's a more traditional optimization problem. Measure your CPU-side code to see where the time is going.

What is also bad, unless this is just a test, is that your objects are so small that every 28 triangles it draws, you have to stall the GPU to figure out what is going to happen next and setup things. You want the GPU to just draw as many triangles in one go as you can.

Material Changes (glBindTexture): 239

This is extreme, post your gpu first, so we can tell wheather you have the performance issue or not at all.

Uhm am i wrong or might it just be the high amount of BufferBinds(both) ?

Would be way better to pack stuff into bigger vao's and use an offset in glDrawElements

glBindVertexArray(vao) // Vertex Array (Vertex +UV +Normal Buffers)

Yep, thats going to be slow.

As Ryokeen suggested, you totally can pack meshes into a single buffer and just send an offset to the draw call. Check ARB_draw_elements_base_vertex, there are similar calls for drawing plain arrays, or instanced arrays/elements draws.

That way you can just pack all your static meshes in one big buffer, managing the offsets yourself (which is fun :P) and have only a couple VAO switches. Since you're essentially doing memory management there, you need to have in mind things like memory fragmentation (ie, what happens if you pack 500 meshes then remove 200 randomly from the same buffer, things get fragmented), so beware.

The idea is not to use VAOs to specify "this is a single mesh that I can draw and the buffers attached have only that mesh" but more like "this is one kind of vertex format I support, and the buffers attached have tons of meshes with the same format".

That way you can just pack all your static meshes in one big buffer, managing the offsets yourself (which is fun ) and have only a couple VAO switches. Since you're essentially doing memory management there, you need to have in mind things like memory fragmentation (ie, what happens if you pack 500 meshes then remove 200 randomly from the same buffer, things get fragmented), so beware.

So, basically I need 3 "global" buffers (1 for vertices, 1 for normals, 1 for uv coordinates), then pack all static (Why just static? My dynamic meshes have the same format, can't I just include them as well?) mesh data in those three. During rendering I then just bind these three buffers once at the beginning (=1 vao switch) and use glDrawElementsBaseVertex for each mesh with the appropriate offset.

Is that about right?

How are you measuring time? I'm guessing that's total CPU per frame?

No, it's just the time for the render loop (The pseudo code). I've used std::chrono::high_resolution_clock to measure it, so it's just the CPU time. I'll give ARB_timer_query a try.

According to the profiler "Very Sleepy", the main CPU bottleneck is with "DrvPresentBuffers". I'm not sure if that means it's the GPU itself, or the synchronization/data transfer from CPU to GPU.

If your problem is that your GPU time per frame is the bottleneck, then you'll have to optimize your shaders / data formats / overdraw / etc.
If you problem is your CPU e per frame is the bottleneck, then it's a more traditional optimization problem. Measure your CPU-side code to see where the time is going.

I'm pretty sure the shader isn't the problem, the fps stay the same even if I simply discard all fragments and deactivate the vertex shader.

Changing the resolution also changes nothing (I've tried switching between 640x480 and 1920x1080, fps is the same), so I think I can also throw out overdraw as a possible candidate?

You don't need one buffer per attribute, you can put them all in the same buffer (either interleaved or separate).

You don't need one buffer per attribute, you can put them all in the same buffer (either interleaved or separate).

Hm... I don't think I understand how that's supposed to work.

So, I create a single buffer, and push all of my vertex, normal, uv and index data into that buffer:

V = Vertex

N = Normal

I = Index

|x| = 4 Bytes

Buffer Data: ...|V1|V1|V1|V2|V2|V2|V3|V3|V3|V4|V4|V4|N1|N1|N1|N2|N2|N2|N3|N3|N3|N4|N4|N4|UV1|UV1|UV2|UV2|UV3|UV3|UV4|UV4|I1|I2|I3|I4|I5|I6|...

Then, during rendering, I can use glDrawElementsBaseVertex to point it to the first index (I1) and draw the mesh:

offsetToFirstIndex = grabOffset()

glDrawElementsBaseVertex(GL_TRIANGLES,2,GL_UNSIGNED_INT,(void*)0,offsetToFirstIndex)

But what about the normals and uv coordinates? I'd still have to use glVertexAttribPointer for both to specify their respective offsets, which means I'd still need a VAO for each mesh.

What am I missing?

You don't need one buffer per attribute, you can put them all in the same buffer (either interleaved or separate).

Hm... I don't think I understand how that's supposed to work.

So, I create a single buffer, and push all of my vertex, normal, uv and index data into that buffer:

V = Vertex

N = Normal

I = Index

|x| = 4 Bytes

Buffer Data: ...|V1|V1|V1|V2|V2|V2|V3|V3|V3|V4|V4|V4|N1|N1|N1|N2|N2|N2|N3|N3|N3|N4|N4|N4|UV1|UV1|UV2|UV2|UV3|UV3|UV4|UV4|I1|I2|I3|I4|I5|I6|...

Then, during rendering, I can use glDrawElementsBaseVertex to point it to the first index (I1) and draw the mesh:

offsetToFirstIndex = grabOffset()

glDrawElementsBaseVertex(GL_TRIANGLES,2,GL_UNSIGNED_INT,(void*)0,offsetToFirstIndex)

But what about the normals and uv coordinates? I'd still have to use glVertexAttribPointer for both to specify their respective offsets, which means I'd still need a VAO for each mesh.

What am I missing?

V1|V1|V1|N1|N1|N1|UV1|UV1|V2|V2|V2|N2|N2|N2|UV2|UV2|V3|V3|V3|N3|N3|N3|UV3|UV3|V4|V4|V4|N4|N4|N4|UV4|UV4 and use GL_ARB_vertex_attrib_binding so that the buffer is decoupled from the format.

You don't need one buffer per attribute, you can put them all in the same buffer (either interleaved or separate).

Hm... I don't think I understand how that's supposed to work.
So, I create a single buffer, and push all of my vertex, normal, uv and index data into that buffer.
I'd still have to use glVertexAttribPointer for both to specify their respective offsets, which means I'd still need a VAO for each mesh.
What am I missing?

I didn't mention VAO's. Just that you can use a single VBO for all attributes, rather than multiple VBOs.

Some people recommend making one VAO per "type" of object / "set of attributes" -- e.g. a VAO for use when your object has positions + normals, and a different VAO for when your object has positions + normals + UV's (and then you constantly modify these VAOs for different objects, if they're use different VBO pointers).
Other people recommend using one VAO per object so that you don't have to modify them.
Other people still, recommend using one global VAO that's shared by everything, and just constantly modify the hell out of it for every draw call as if you're writing pre-VAO OpenGL code...

Also, it's common for attributes to be interleaved, such as:
V1|N1|UV1||V2|N2|UV2||V3|N3|UV3
Instead of separated:
V1|V2|V3||N1|N2|N3||UV1|UV2|UV3
Or, semi-interleaving is also common if the same mesh is rendered with multiple different shaders. e.g. if one shader only reads positions, but another reads position+normal+uv, then you might use a compromise layout such as:
V1|V2|V3||N1|UV1||N2|UV2||N3|UV3

These layout make good use of the stride and offset parameters when binding a VBO
If multiple different objects exist within the same VBO, you don't have to rebind the VBO/attributes (i.e. don't have to modify the VAO) -- the offset from glDrawElementsBaseVertex is added to the offset that was passed into glVertexAttribPointer for each attribute.

Buffer Data: ...|V1|V1|....|UV4|I1|I2|I3|I4|I5|I6|...
Then, during rendering, I can use glDrawElementsBaseVertex to point it to the first index (I1) and draw the mesh:
offsetToFirstIndex = grabOffset()
glDrawElementsBaseVertex(GL_TRIANGLES,2,GL_UNSIGNED_INT,(void*)0,offsetToFirstIndex)

In your example code, you're passing "0" as the index buffer offset, and "offsetToFirstIndex" as the vertex buffer offset.

Indices will be fetched from the index buffer (aka "element array buffer") using: eab + 0 * sizeof(byte).
Vertex attributes will be fetched from their vbo using: attribute.vbo + attribute.offset * sizeof(byte) + (index + offsetToFirstIndex) * attribute.stride

So you want to replace that "0" with the offset in bytes from the start of the buffer to the first index that you want to fetch, and replace "offsetToFirstIndex" with the offset in vertices that should be added to every index value that is fetched / how far into the VBO vertex #0 is located. Because this parameter is an offset-in-vertices, not an offset-in-bytes, it works across all attributes equally, no matter how they are laid out in memory.

[edit] Also, GL_UNSIGNED_SHORT is a much more performant (and memory-saving) format for indices -- try to use 16bit indices instead of 32bit ones wherever possible, even on modern hardware (and especially on old hardware).

Thank you, but I'm still unclear on a couple of things.

I've switched the data order to:

V1|N1|UV1|V2|N2|UV2|V3|N3|UV3

But what about the indices? Is it not possible to just append them to the same buffer (i.e. V1|N1|UV1|V2|N2|UV2|V3|N3|UV3|I1|I2|I3|I4|I5|I6), or is an element buffer absolutely required?

Either way, I've created a test-scenario with just one object and no vao.

There are two buffers, the vbo with the data as described above, and the element buffer with the vertex indices.

During rendering I then use:

glBindBuffer(GL_ARRAY_BUFFER,dataBuffer) // vbo
// Vertex Data
glEnableVertexAttribArray(0)
glVertexAttribPointer(
	0,
	3, // 3 Floats
	GL_FLOAT,
	GL_FALSE,
	sizeof(float) *5, // Offset between vertices is sizeof(normal) +sizeof(uv)
	(void*)0 // First vertex starts at the beginning
);
//

// Normal Data
glEnableVertexAttribArray(1)
glVertexAttribPointer(
	1,
	3, // 3 Floats
	GL_FLOAT,
	GL_FALSE,
	sizeof(float) *5, // Offset between normals is sizeof(uv) +sizeof(vertex)
	(void*)(sizeof(float) *3) // First normal starts after first vertex
);
//

// UV Data
glEnableVertexAttribArray(2)
glVertexAttribPointer(
	2,
	2, // 2 Floats
	GL_FLOAT,
	GL_FALSE,
	sizeof(float) *6, // Offset between uvs is sizeof(vertex) +sizeof(normal)
	(void*)(sizeof(float) *6) // First uv starts after first normal
);
//
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER,indexBuffer); // index/element buffer
glDrawElementsBaseVertex(
    GL_TRIANGLES,
    numTriangles,
    GL_UNSIGNED_INT,
    (void*)0, // For testing purposes; Index buffer contains only one mesh, which starts at index 0
    0 // Not sure about this one? VBO vertex #0 is located at position 0 in the data buffer
);

(I know this isn't effective code, I'm doing it this way to help me understand. I'll optimize it once I got it working)

The mesh is rendered, however not correctly (Vertices, normals and uv coordinates are wrong).

I've switched the data order to:
V1|N1|UV1|V2|N2|UV2|V3|N3|UV3

During rendering I then use:

...

The mesh is rendered, however not correctly (Vertices, normals and uv coordinates are wrong).

Your stride and offset values are all wacky.

If your vertex structure looks like:

struct Vertex { float px, py, pz, nx, ny, nz, u, v; }

Then:

position has offset=0, stride=sizeof(Vertex)

normal has offset = sizeof(float)*3, stride=sizeof(Vertex)

uv has offset = sizeof(float)*6, stride=sizeof(Vertex)

Offset is the number of bytes from the start of the vertex to the start of that element. Stride is how many bytes you have to advance by to arrive at the next vertices value of that same element.

But what about the indices? Is it not possible to just append them to the same buffer (i.e. V1|N1|UV1|V2|N2|UV2|V3|N3|UV3|I1|I2|I3|I4|I5|I6), or is an element buffer absolutely required?

"Element array buffer" is a binding slot in the API. You should be able to bind the same VBO to that slot (and then the indices parameter to glDrawElementsBaseVertex is an offset in bytes into that VBO of where to read the indices from).

You'll find that things will be much easier for you if you define a struct for your vertex format; e.g:

struct myVertex {

float Position[3];

float Normal[3];

float TexCoord[2];

};

You'll then be able to use sizeof (myVertex) and offsetof (myVertex, Position) instead of messing around with multiplying sizeof (float) by whatever and introducing possibility of errors into your program.

But what about the indices? Is it not possible to just append them to the same buffer (i.e. V1|N1|UV1|V2|N2|UV2|V3|N3|UV3|I1|I2|I3|I4|I5|I6), or is an element buffer absolutely required?

"Element array buffer" is a binding slot in the API. You should be able to bind the same VBO to that slot (and then the indices parameter to glDrawElementsBaseVertex is an offset in bytes into that VBO of where to read the indices from).

Thanks, I think I got it now.

One more thing however:

The last parameter of glDrawElementsBaseVertex is an offset in vertices. So that means I can't add the indices to the same buffer after all, unless I make sure they 'fit' into 'packages' the same size as a multiple of that of one vertex (sizeof(position) +sizeof(normal) +sizeof(uv)) by leaving some bytes after the indices unused, is that correct?

I think just using a second global buffer for the indices would be easier and shouldn't have any performance impact, since I still only need to bind it once, right?

// EDIT:

Well, sadly it turns out that this method didn't help at all, in fact it made matters worse. Execution time for the render loop went up from 23ms to 39ms. My render loop now looks like this:

glBindVertexArray(vao) // Enables the vertex/uv/normal arrays and binds the global buffer
foreach shader
	glUseProgram(shader)
	foreach material
                glBindTexture(material)
		foreach mesh
                        glDrawElementsBaseVertex(GL_TRIANGLES,vertexCount,GL_UNSIGNED_INT,...);
		end
	end
end

There are no buffer binds or changes at all between meshes, but since the performance dropped substantially, I suppose that wasn't the issue after all. Or perhaps glDrawElementsBaseVertex just isn't supported by my drivers very well?

// EDIT2:

On second try, it did actually increase performance slightly, but it's definitely not the main bottleneck.

The last parameter of glDrawElementsBaseVertex is an offset in vertices. So that means I can't add the indices to the same buffer after all, unless I make sure they 'fit' into 'packages' the same size as a multiple of that of one vertex (sizeof(position) +sizeof(normal) +sizeof(uv)) by leaving some bytes after the indices unused, is that correct?
I think just using a second global buffer for the indices would be easier and shouldn't have any performance impact, since I still only need to bind it once, right?

It doesn't mean anything about where you put the indices, because it's an offset of where to begin reading vertices from... not where to read indices from.
It does mean that if you want to put different types of vertices into the same buffer, you can end up with alignment considerations though, yeah.
Yes, putting indices into a separate buffer is much easier to handle

it's definitely not the main bottleneck

Put some timing code into your loops to find out what part is taking up the most time.