Test buffer contents on GLES3 & WebGL, fix global symbol duplication in GL test libraries #565
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If a GL test library would link to MagnumOpenGLTesterTestLib (which has
CORRADE_GRACEFUL_ASSERT enabled to be able to verify assertions) and
then to MagnumDebugTools for DebugTools::bufferData(), it'd mean there's
one GL::Context global from MagnumGLTestLib and one from MagnumGL. Then,
depending on whatever random order the linker uses, different parts of
the library would see a different global, ultimately leading to a
dreaded
GL::Context::current(): no current context
assertion. Right now this only manifested on the macOS static CI build,
but depending on a phase of the moon could happen for any platform in
any circumstance.
First attempt was to switch to linking to MagnumDebugToolsTestLib and
then making MagnumDebugToolsTestLib depend on MagnumGLTestLib instead of
MagnumGL, HOWEVER because DebugTools also depend on Primitives and
Shaders and whatnot for some features, it just moved the conflict
between MagnumGL and MagnumGLTestLib elsewhere -- and ASan started
loudly complaining about GL::defaultFramebuffer being duplicated.
So instead there's now a dedicated subset of DebugTools just for the GL
test themselves, containing currently just DebugTools::bufferData(), as
nothing else is needed ATM. It may grow further when needed, such as
with textureImage(), or CompareImage, etc.
But of course that wouldn't be enough -- MagnumOpenGLTesterTestLib
actually still links to MagnumGL for Other Reasons, meaning we have to
pass it last to prefer symbols from MagnumGLTestLib which have graceful
asserts enabled. Hopefully this works well enough, otherwise I'd have to
figure out yet another variant of the fix.
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Split off of #560, because it uncovered a deeper problem in how
*TestLiblibraries are built.FooTestLibis mostly the same asFoo, except that it hasCORRADE_GRACEFUL_ASSERTdefined in order to be able to test assertion messages -- then an assert won't abort, but instead gracefully return, allowing the message to be captured.MagnumGLhas a global "current context" pointer, and then there'sMagnumGLTestLibwhich has also a global "current context" pointer. To avoid having two copies of the same, a GL test should then link to either one or the other and never both.MagnumOpenGLTesterTestLib, which links toMagnumGLTestLibinstead ofMagnumGL. Unfortunately it does so only on Windows, which has to be fixed.DebugTools::bufferData()to query buffer contents, there also needs to be (a subset of)DebugToolsthat links toMagnumGLTestLibinstead ofMagnumGL.Things to do:
/** @todo How to verify the contents in ES? */)MagnumOpenGLTesterTestLiblink againstMagnumGLTestLibeverywhere, not just on WindowsMagnumDebugToolsGLTestLibSubsetlinking toMagnumGLTestLib