Browse Source

GL: actually fix the VAO / element array buffer binding state interaction.

Fixing the new (and now failing test) from the previous commit. In
setIndexBuffer(), I was resetting the state tracker to a VAO state that
was about to be set in the very next step, and then, when doing that
next step the state tracker "optimized away" the state change because it
thought it was already done (even though it wasn't). Reordering the two
operations fixes it.

In comparison to how this was meant to be done in the original
169031fb7b, the new way should do the same
but additionally avoid a bunch of redundant state calls. Let's hope no
more bugs related to this appear.
pull/324/head
Vladimír Vondruš 7 years ago
parent
commit
d6e0186dbf
  1. 3
      doc/changelog.dox
  2. 14
      src/Magnum/GL/Mesh.cpp

3
doc/changelog.dox

@ -111,6 +111,9 @@ See also:
- The @cb{.cmake} android_create_apk() @ce macro for
@ref platforms-android-apps-cmake "Gradle-less Android builds" now properly
creates the library output directory beforehand
- Fixed a bad interaction of @ref GL::Mesh and @ref GL::Buffer state trackers
resulting in corrupted rendering on ES/WebGL platforms and systems without
@gl_extension{ARB,direct_state_access} / @gl_extension{EXT,direct_state_access}
- Updated the `base-qt` bootstrap project to correctly reset state tracker
when going from and back to Qt code, and to use the framebuffer provided by
Qt instead of @ref GL::defaultFramebuffer (see

14
src/Magnum/GL/Mesh.cpp

@ -340,6 +340,11 @@ Mesh& Mesh::setIndexBuffer(Buffer&& buffer, GLintptr offset, MeshIndexType type,
"GL::Mesh::setIndexBuffer(): the buffer has unexpected target hint, expected" << Buffer::TargetHint::ElementArray << "but got" << buffer.targetHint(), *this);
#endif
/* It's IMPORTANT to do this *before* the _indexBuffer is set, since the
bindVAO() function called from here is resetting element buffer state
tracker to _indexBuffer.id(). */
(this->*Context::current().state().mesh->bindIndexBufferImplementation)(buffer);
_indexBuffer = std::move(buffer);
_indexOffset = offset;
_indexType = type;
@ -350,7 +355,6 @@ Mesh& Mesh::setIndexBuffer(Buffer&& buffer, GLintptr offset, MeshIndexType type,
static_cast<void>(start);
static_cast<void>(end);
#endif
(this->*Context::current().state().mesh->bindIndexBufferImplementation)(_indexBuffer);
return *this;
}
@ -547,9 +551,11 @@ void Mesh::bindVAO() {
/* Reset element buffer binding, because binding a different VAO with a
different index buffer will change that binding as well. (GL state,
what the hell.) We could also theoretically reset the binding
directly to _indexBuffer->id(), but let's play safe and force the
rebind every time. */
what the hell.). The _indexBuffer.id() is the index buffer that's
already attached to this particular VAO (or 0, if there's none). In
particular, the setIndexBuffer() buffers call this function *and
then* sets the _indexBuffer, which means at this point the ID will
be still 0. */
Context::current().state().buffer->bindings[Implementation::BufferState::indexForTarget(Buffer::TargetHint::ElementArray)] = _indexBuffer.id();
}
}

Loading…
Cancel
Save