bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
15.08k stars 1.95k forks source link

OpenGL attribute error #765

Open danielwallner opened 8 years ago

danielwallner commented 8 years ago

Hi, the iOS OpenGL ES frame capture tool in Xcode reports the following as an error:

First draw with the following declaration and length: vertex_decl_x.add(bgfx::Attrib::Position, 2, bgfx::AttribType::Float).add(bgfx::Attrib::Color0, 4, bgfx::AttribType::Float) bgfx::setVertexBuffer(vertex_buffer_x, 0, 8);

Then draw again a greater number of vertices with fewer attributes declared: vertex_decl_y.add(bgfx::Attrib::Position, 2, bgfx::AttribType::Float) bgfx::setVertexBuffer(vertex_buffer_y, 0, 768);

Drawing the second time will then be done with the second attribute array of the first draw call still enabled (and too short). If it makes things different this is done on a sequential view.

We haven't seen this crashing yet, but it is a bit worrying.

bkaradzic commented 8 years ago

Can you change existing example to reproduce this issue?

danielwallner commented 8 years ago

Is the GL_CHECK(glDisableVertexAttribArray(loc) ); line in ProgramGL::bindAttributes() intended to also disable previously enabled but currently unused attributes? Considering how m_used is initialized doesn't look like that line is normally meant to ever run. Before the offending draw call the loop in bindAttributes() simply runs once, any only enables the first attribute array, but doesn't disable the still enabled second one from the previous draw call.

bkaradzic commented 8 years ago

Usage depends is program use attribute, not if it's available in VertexDecl.

danielwallner commented 8 years ago

If this is intended behaviour, since this is the only place where glDisableVertexAttribArray is used, is this then supposed to be handled by switching VAO? The same VAO is now used for both the previous program and the one with fewer attributes.

bkaradzic commented 8 years ago

Yeah VAO should handle it, but it's probably a bug. Anyhow try disabling VAO. Just above this line set m_vaoSupport = false; https://github.com/bkaradzic/bgfx/blob/master/src/renderer_gl.cpp#L1845

danielwallner commented 8 years ago

It was in fact the opposite. VAO support is disabled on GL ES 2 on iOS (line 829 renderer_gl.cpp). Any reason for that? git blame doesn't give a clue.

When I enable it we don't get any errors any more. So then, when VAO support is disabled it behaves like I described before, no attributes ever gets disabled, but this is then no longer a problem for us.