From 86972cb07f031e535f8a42ddf2167ffbcb53e726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 12 Jul 2025 19:44:43 +0200 Subject: [PATCH] GL: repro cases for wrap() without ObjectFlag::Created causing asserts. This happened to me from time to time and I always treated it as user error. But when thinking more about it, calling wrap() without actually knowing if the object was already created or not, is a completely valid use case, especially in cases of making non-owning references to existing GL objects that might only get created at some point after. These tests now cause assertions in all cases on GLES, on desktop some only if ARB_direct_state_access is disabled. In all cases the assertion is triggered by querying object label (which is a no-op if no debug extension is present, but it calls createIfNotAlready() always), because that's the simplest, the real-world cases are usually some texture bind() calls. Fix in the next commits. --- src/Magnum/GL/Test/BufferGLTest.cpp | 25 +++++ src/Magnum/GL/Test/BufferTextureGLTest.cpp | 34 +++++++ .../GL/Test/CubeMapTextureArrayGLTest.cpp | 34 +++++++ src/Magnum/GL/Test/CubeMapTextureGLTest.cpp | 26 ++++++ src/Magnum/GL/Test/FramebufferGLTest.cpp | 31 +++++++ src/Magnum/GL/Test/MeshGLTest.cpp | 40 ++++++++ .../GL/Test/MultisampleTextureGLTest.cpp | 68 ++++++++++++++ src/Magnum/GL/Test/RectangleTextureGLTest.cpp | 25 +++++ src/Magnum/GL/Test/RenderbufferGLTest.cpp | 32 +++++++ src/Magnum/GL/Test/TextureArrayGLTest.cpp | 67 ++++++++++++++ src/Magnum/GL/Test/TextureGLTest.cpp | 91 +++++++++++++++++++ .../GL/Test/TransformFeedbackGLTest.cpp | 34 +++++++ 12 files changed, 507 insertions(+) diff --git a/src/Magnum/GL/Test/BufferGLTest.cpp b/src/Magnum/GL/Test/BufferGLTest.cpp index 569d5167f..43bfe5769 100644 --- a/src/Magnum/GL/Test/BufferGLTest.cpp +++ b/src/Magnum/GL/Test/BufferGLTest.cpp @@ -49,6 +49,7 @@ struct BufferGLTest: OpenGLTester { void constructFromData(); void constructMove(); void wrap(); + void wrapCreateIfNotAlready(); void targetHint(); @@ -110,6 +111,7 @@ BufferGLTest::BufferGLTest() { &BufferGLTest::constructFromData, &BufferGLTest::constructMove, &BufferGLTest::wrap, + &BufferGLTest::wrapCreateIfNotAlready, &BufferGLTest::targetHint, @@ -241,6 +243,29 @@ void BufferGLTest::wrap() { glDeleteBuffers(1, &id); } +void BufferGLTest::wrapCreateIfNotAlready() { + /* Make an object and ensure it's created */ + Buffer buffer{"hello"}; + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(buffer.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. Here the "already bound" case + only happens if GL_ARB_direct_state_access is disabled. */ + Buffer wrapped = Buffer::wrap(buffer.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} void BufferGLTest::targetHint() { Buffer buffer; CORRADE_COMPARE(buffer.targetHint(), Buffer::TargetHint::Array); diff --git a/src/Magnum/GL/Test/BufferTextureGLTest.cpp b/src/Magnum/GL/Test/BufferTextureGLTest.cpp index f1852c179..4688feabf 100644 --- a/src/Magnum/GL/Test/BufferTextureGLTest.cpp +++ b/src/Magnum/GL/Test/BufferTextureGLTest.cpp @@ -49,6 +49,7 @@ struct BufferTextureGLTest: OpenGLTester { void construct(); void constructMove(); void wrap(); + void wrapCreateIfNotAlready(); void label(); @@ -75,6 +76,7 @@ BufferTextureGLTest::BufferTextureGLTest() { addTests({&BufferTextureGLTest::construct, &BufferTextureGLTest::constructMove, &BufferTextureGLTest::wrap, + &BufferTextureGLTest::wrapCreateIfNotAlready, &BufferTextureGLTest::label, @@ -151,6 +153,38 @@ void BufferTextureGLTest::wrap() { glDeleteTextures(1, &id); } +void BufferTextureGLTest::wrapCreateIfNotAlready() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::texture_buffer_object::string() << "is not supported."); + #else + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::EXT::texture_buffer::string() << "is not supported."); + #endif + + /* Make an object and ensure it's created */ + BufferTexture texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + BufferTexture wrapped = BufferTexture::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + void BufferTextureGLTest::label() { /* No-Op version is tested in AbstractObjectGLTest */ if(!Context::current().isExtensionSupported() && diff --git a/src/Magnum/GL/Test/CubeMapTextureArrayGLTest.cpp b/src/Magnum/GL/Test/CubeMapTextureArrayGLTest.cpp index 13d81db78..873e0495c 100644 --- a/src/Magnum/GL/Test/CubeMapTextureArrayGLTest.cpp +++ b/src/Magnum/GL/Test/CubeMapTextureArrayGLTest.cpp @@ -53,6 +53,7 @@ struct CubeMapTextureArrayGLTest: OpenGLTester { void construct(); void constructMove(); void wrap(); + void wrapCreateIfNotAlready(); void label(); @@ -273,6 +274,7 @@ CubeMapTextureArrayGLTest::CubeMapTextureArrayGLTest() { &CubeMapTextureArrayGLTest::construct, &CubeMapTextureArrayGLTest::constructMove, &CubeMapTextureArrayGLTest::wrap, + &CubeMapTextureArrayGLTest::wrapCreateIfNotAlready, &CubeMapTextureArrayGLTest::label, @@ -413,6 +415,38 @@ void CubeMapTextureArrayGLTest::wrap() { glDeleteTextures(1, &id); } +void CubeMapTextureArrayGLTest::wrapCreateIfNotAlready() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::texture_cube_map_array::string() << "is not supported."); + #else + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::EXT::texture_cube_map_array::string() << "is not supported."); + #endif + + /* Make an object and ensure it's created */ + CubeMapTextureArray texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + CubeMapTextureArray wrapped = CubeMapTextureArray::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + void CubeMapTextureArrayGLTest::label() { #ifndef MAGNUM_TARGET_GLES if(!Context::current().isExtensionSupported()) diff --git a/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp b/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp index 4e3d7421c..fa2fe065e 100644 --- a/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp +++ b/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp @@ -63,6 +63,7 @@ struct CubeMapTextureGLTest: OpenGLTester { void construct(); void constructMove(); void wrap(); + void wrapCreateIfNotAlready(); #ifndef MAGNUM_TARGET_WEBGL void label(); @@ -354,6 +355,7 @@ CubeMapTextureGLTest::CubeMapTextureGLTest() { &CubeMapTextureGLTest::construct, &CubeMapTextureGLTest::constructMove, &CubeMapTextureGLTest::wrap, + &CubeMapTextureGLTest::wrapCreateIfNotAlready, #ifndef MAGNUM_TARGET_WEBGL &CubeMapTextureGLTest::label, @@ -554,6 +556,30 @@ void CubeMapTextureGLTest::wrap() { glDeleteTextures(1, &id); } +void CubeMapTextureGLTest::wrapCreateIfNotAlready() { + /* Make an object and ensure it's created */ + CubeMapTexture texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + CubeMapTexture wrapped = CubeMapTexture::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + #ifndef MAGNUM_TARGET_WEBGL void CubeMapTextureGLTest::label() { /* No-Op version is tested in AbstractObjectGLTest */ diff --git a/src/Magnum/GL/Test/FramebufferGLTest.cpp b/src/Magnum/GL/Test/FramebufferGLTest.cpp index 1a1ec62df..efa275372 100644 --- a/src/Magnum/GL/Test/FramebufferGLTest.cpp +++ b/src/Magnum/GL/Test/FramebufferGLTest.cpp @@ -69,6 +69,7 @@ struct FramebufferGLTest: OpenGLTester { void construct(); void constructMove(); void wrap(); + void wrapCreateIfNotAlready(); #ifndef MAGNUM_TARGET_WEBGL void label(); @@ -236,6 +237,7 @@ FramebufferGLTest::FramebufferGLTest() { addTests({&FramebufferGLTest::construct, &FramebufferGLTest::constructMove, &FramebufferGLTest::wrap, + &FramebufferGLTest::wrapCreateIfNotAlready, #ifndef MAGNUM_TARGET_WEBGL &FramebufferGLTest::label, @@ -445,6 +447,35 @@ void FramebufferGLTest::wrap() { glDeleteFramebuffers(1, &id); } +void FramebufferGLTest::wrapCreateIfNotAlready() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::framebuffer_object::string() << "is not supported."); + #endif + + /* Make an object and ensure it's created */ + Framebuffer framebuffer({{}, Vector2i{4}}); + framebuffer.bind(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(framebuffer.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + Framebuffer wrapped = Framebuffer::wrap(framebuffer.id(), {{}, Vector2i{4}}); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + #ifndef MAGNUM_TARGET_WEBGL void FramebufferGLTest::label() { #ifndef MAGNUM_TARGET_GLES diff --git a/src/Magnum/GL/Test/MeshGLTest.cpp b/src/Magnum/GL/Test/MeshGLTest.cpp index 82811cef4..0146d0f45 100644 --- a/src/Magnum/GL/Test/MeshGLTest.cpp +++ b/src/Magnum/GL/Test/MeshGLTest.cpp @@ -65,6 +65,7 @@ struct MeshGLTest: OpenGLTester { void construct(); void constructMove(); void wrap(); + void wrapCreateIfNotAlready(); void destructMovedOutInstance(); @@ -563,6 +564,7 @@ MeshGLTest::MeshGLTest() { addTests({&MeshGLTest::construct, &MeshGLTest::constructMove, &MeshGLTest::wrap, + &MeshGLTest::wrapCreateIfNotAlready, &MeshGLTest::destructMovedOutInstance, @@ -930,6 +932,44 @@ void MeshGLTest::wrap() { #endif } +void MeshGLTest::wrapCreateIfNotAlready() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::vertex_array_object::string() << "is not supported."); + #elif defined(MAGNUM_TARGET_GLES2) + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::OES::vertex_array_object::string() << "is not supported."); + #endif + + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::framebuffer_object::string() << "is not supported."); + #endif + + /* Make an object and ensure it's created */ + Mesh mesh; + mesh.addVertexBuffer(Buffer{}, 0, Attribute<0, Vector3>{}); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(mesh.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. Here the "already bound" case + only happens if GL_ARB_direct_state_access is disabled. */ + Mesh wrapped = Mesh::wrap(mesh.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + void MeshGLTest::destructMovedOutInstance() { { Containers::ScopeGuard restoreCurrentContext{&GL::Context::current(), GL::Context::makeCurrent}; diff --git a/src/Magnum/GL/Test/MultisampleTextureGLTest.cpp b/src/Magnum/GL/Test/MultisampleTextureGLTest.cpp index 968614822..3e3456df0 100644 --- a/src/Magnum/GL/Test/MultisampleTextureGLTest.cpp +++ b/src/Magnum/GL/Test/MultisampleTextureGLTest.cpp @@ -47,6 +47,8 @@ struct MultisampleTextureGLTest: OpenGLTester { void wrap2D(); void wrap2DArray(); + void wrapCreateIfNotAlready2D(); + void wrapCreateIfNotAlready2DArray(); void label2D(); void label2DArray(); @@ -80,6 +82,8 @@ MultisampleTextureGLTest::MultisampleTextureGLTest() { &MultisampleTextureGLTest::wrap2D, &MultisampleTextureGLTest::wrap2DArray, + &MultisampleTextureGLTest::wrapCreateIfNotAlready2D, + &MultisampleTextureGLTest::wrapCreateIfNotAlready2DArray, &MultisampleTextureGLTest::label2D, &MultisampleTextureGLTest::label2DArray, @@ -200,6 +204,70 @@ void MultisampleTextureGLTest::wrap2DArray() { glDeleteTextures(1, &id); } +void MultisampleTextureGLTest::wrapCreateIfNotAlready2D() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::texture_multisample::string() << "is not supported."); + #else + if(!Context::current().isVersionSupported(Version::GLES310)) + CORRADE_SKIP("OpenGL ES 3.1 is not supported."); + #endif + + /* Make an object and ensure it's created */ + MultisampleTexture2D texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + MultisampleTexture2D wrapped = MultisampleTexture2D::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + +void MultisampleTextureGLTest::wrapCreateIfNotAlready2DArray() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::texture_multisample::string() << "is not supported."); + #else + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::OES::texture_storage_multisample_2d_array::string() << "is not supported."); + #endif + + /* Make an object and ensure it's created */ + MultisampleTexture2DArray texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + MultisampleTexture2DArray wrapped = MultisampleTexture2DArray::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + void MultisampleTextureGLTest::label2D() { /* No-Op version is tested in AbstractObjectGLTest */ if(!Context::current().isExtensionSupported() && diff --git a/src/Magnum/GL/Test/RectangleTextureGLTest.cpp b/src/Magnum/GL/Test/RectangleTextureGLTest.cpp index 6dae88c35..a7cf81123 100644 --- a/src/Magnum/GL/Test/RectangleTextureGLTest.cpp +++ b/src/Magnum/GL/Test/RectangleTextureGLTest.cpp @@ -51,6 +51,7 @@ struct RectangleTextureGLTest: OpenGLTester { void construct(); void constructMove(); void wrap(); + void wrapCreateIfNotAlready(); void label(); @@ -128,6 +129,7 @@ RectangleTextureGLTest::RectangleTextureGLTest() { &RectangleTextureGLTest::construct, &RectangleTextureGLTest::constructMove, &RectangleTextureGLTest::wrap, + &RectangleTextureGLTest::wrapCreateIfNotAlready, &RectangleTextureGLTest::label, @@ -222,6 +224,29 @@ void RectangleTextureGLTest::wrap() { glDeleteTextures(1, &id); } +void RectangleTextureGLTest::wrapCreateIfNotAlready() { + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::texture_rectangle::string() << "is not supported."); + + /* Make an object and ensure it's created */ + RectangleTexture texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + RectangleTexture wrapped = RectangleTexture::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); +} + void RectangleTextureGLTest::label() { /* No-Op version is tested in AbstractObjectGLTest */ if(!Context::current().isExtensionSupported() && diff --git a/src/Magnum/GL/Test/RenderbufferGLTest.cpp b/src/Magnum/GL/Test/RenderbufferGLTest.cpp index aeb94054b..b78e2333f 100644 --- a/src/Magnum/GL/Test/RenderbufferGLTest.cpp +++ b/src/Magnum/GL/Test/RenderbufferGLTest.cpp @@ -45,6 +45,7 @@ struct RenderbufferGLTest: OpenGLTester { void construct(); void constructMove(); void wrap(); + void wrapCreateIfNotAlready(); #ifndef MAGNUM_TARGET_WEBGL void label(); @@ -60,6 +61,7 @@ RenderbufferGLTest::RenderbufferGLTest() { addTests({&RenderbufferGLTest::construct, &RenderbufferGLTest::constructMove, &RenderbufferGLTest::wrap, + &RenderbufferGLTest::wrapCreateIfNotAlready, #ifndef MAGNUM_TARGET_WEBGL &RenderbufferGLTest::label, @@ -146,6 +148,36 @@ void RenderbufferGLTest::wrap() { glDeleteRenderbuffers(1, &id); } +void RenderbufferGLTest::wrapCreateIfNotAlready() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::framebuffer_object::string() << "is not supported."); + #endif + + /* Make an object and ensure it's created */ + Renderbuffer renderbuffer; + renderbuffer.setStorage(RenderbufferFormat::RGBA4, {4, 4}); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(renderbuffer.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. Here the "already bound" case + only happens if GL_ARB_direct_state_access is disabled. */ + Renderbuffer wrapped = Renderbuffer::wrap(renderbuffer.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + #ifndef MAGNUM_TARGET_WEBGL void RenderbufferGLTest::label() { #ifndef MAGNUM_TARGET_GLES diff --git a/src/Magnum/GL/Test/TextureArrayGLTest.cpp b/src/Magnum/GL/Test/TextureArrayGLTest.cpp index 196ad7911..8d67ad862 100644 --- a/src/Magnum/GL/Test/TextureArrayGLTest.cpp +++ b/src/Magnum/GL/Test/TextureArrayGLTest.cpp @@ -70,6 +70,10 @@ struct TextureArrayGLTest: OpenGLTester { void wrap1D(); #endif void wrap2D(); + #ifndef MAGNUM_TARGET_GLES + void wrapCreateIfNotAlready1D(); + #endif + void wrapCreateIfNotAlready2D(); #ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_GLES @@ -309,6 +313,10 @@ TextureArrayGLTest::TextureArrayGLTest() { &TextureArrayGLTest::wrap1D, #endif &TextureArrayGLTest::wrap2D, + #ifndef MAGNUM_TARGET_GLES + &TextureArrayGLTest::wrapCreateIfNotAlready1D, + #endif + &TextureArrayGLTest::wrapCreateIfNotAlready2D, #ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_GLES @@ -567,6 +575,65 @@ void TextureArrayGLTest::wrap2D() { glDeleteTextures(1, &id); } +#ifndef MAGNUM_TARGET_GLES +void TextureArrayGLTest::wrapCreateIfNotAlready1D() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::texture_multisample::string() << "is not supported."); + #else + if(!Context::current().isVersionSupported(Version::GLES310)) + CORRADE_SKIP("OpenGL ES 3.1 is not supported."); + #endif + + /* Make an object and ensure it's created */ + Texture1DArray texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + Texture1DArray wrapped = Texture1DArray::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); +} +#endif + +void TextureArrayGLTest::wrapCreateIfNotAlready2D() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::EXT::texture_array::string() << "is not supported."); + #endif + + /* Make an object and ensure it's created */ + Texture2DArray texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + Texture2DArray wrapped = Texture2DArray::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + #ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_GLES void TextureArrayGLTest::label1D() { diff --git a/src/Magnum/GL/Test/TextureGLTest.cpp b/src/Magnum/GL/Test/TextureGLTest.cpp index 665d25bfa..f1330fe49 100644 --- a/src/Magnum/GL/Test/TextureGLTest.cpp +++ b/src/Magnum/GL/Test/TextureGLTest.cpp @@ -81,6 +81,13 @@ struct TextureGLTest: OpenGLTester { #if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL)) void wrap3D(); #endif + #ifndef MAGNUM_TARGET_GLES + void wrapCreateIfNotAlready1D(); + #endif + void wrapCreateIfNotAlready2D(); + #if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL)) + void wrapCreateIfNotAlready3D(); + #endif #ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_GLES @@ -533,6 +540,13 @@ TextureGLTest::TextureGLTest() { #if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL)) &TextureGLTest::wrap3D, #endif + #ifndef MAGNUM_TARGET_GLES + &TextureGLTest::wrapCreateIfNotAlready1D, + #endif + &TextureGLTest::wrapCreateIfNotAlready2D, + #if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL)) + &TextureGLTest::wrapCreateIfNotAlready3D, + #endif #ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_GLES @@ -925,6 +939,83 @@ void TextureGLTest::wrap3D() { } #endif +#ifndef MAGNUM_TARGET_GLES +void TextureGLTest::wrapCreateIfNotAlready1D() { + /* Make an object and ensure it's created */ + Texture1D texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + Texture1D wrapped = Texture1D::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); +} +#endif + +void TextureGLTest::wrapCreateIfNotAlready2D() { + /* Make an object and ensure it's created */ + Texture2D texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + Texture2D wrapped = Texture2D::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + +#if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL)) +void TextureGLTest::wrapCreateIfNotAlready3D() { + #ifdef MAGNUM_TARGET_GLES2 + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::OES::texture_3D::string() << "is not supported."); + #endif + + /* Make an object and ensure it's created */ + Texture3D texture; + texture.bind(0); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(texture.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. */ + Texture3D wrapped = Texture3D::wrap(texture.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} +#endif + #ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_GLES void TextureGLTest::label1D() { diff --git a/src/Magnum/GL/Test/TransformFeedbackGLTest.cpp b/src/Magnum/GL/Test/TransformFeedbackGLTest.cpp index 3b9bb09fe..edd9aa59b 100644 --- a/src/Magnum/GL/Test/TransformFeedbackGLTest.cpp +++ b/src/Magnum/GL/Test/TransformFeedbackGLTest.cpp @@ -57,6 +57,7 @@ struct TransformFeedbackGLTest: OpenGLTester { void construct(); void constructMove(); void wrap(); + void wrapCreateIfNotAlready(); #ifndef MAGNUM_TARGET_WEBGL void label(); @@ -126,6 +127,7 @@ TransformFeedbackGLTest::TransformFeedbackGLTest() { addTests({&TransformFeedbackGLTest::construct, &TransformFeedbackGLTest::constructMove, &TransformFeedbackGLTest::wrap, + &TransformFeedbackGLTest::wrapCreateIfNotAlready, #ifndef MAGNUM_TARGET_WEBGL &TransformFeedbackGLTest::label, @@ -218,6 +220,38 @@ void TransformFeedbackGLTest::wrap() { glDeleteTransformFeedbacks(1, &id); } +void TransformFeedbackGLTest::wrapCreateIfNotAlready() { + #ifndef MAGNUM_TARGET_GLES + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::ARB::framebuffer_object::string() << "is not supported."); + #endif + + Buffer buffer{Buffer::TargetHint::Array}; + + /* Make an object and ensure it's created */ + TransformFeedback xfb; + xfb.attachBuffer(0, buffer); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(xfb.flags(), ObjectFlag::Created|ObjectFlag::DeleteOnDestruction); + + /* Wrap into another object without ObjectFlag::Created being set, which is + a common usage pattern to make non-owning references. Then calling an + API that internally does createIfNotAlready() shouldn't assert just + because Created isn't set but the object is bound, instead it should + just mark it as such when it discovers it. Here the "already bound" case + only happens if GL_ARB_direct_state_access is disabled. */ + TransformFeedback wrapped = TransformFeedback::wrap(xfb.id()); + CORRADE_COMPARE(wrapped.flags(), ObjectFlags{}); + + #ifndef MAGNUM_TARGET_WEBGL + wrapped.label(); + MAGNUM_VERIFY_NO_GL_ERROR(); + CORRADE_COMPARE(wrapped.flags(), ObjectFlag::Created); + #else + CORRADE_SKIP("No API that would call createIfNotAlready() on WebGL, can't test."); + #endif +} + #ifndef MAGNUM_TARGET_WEBGL void TransformFeedbackGLTest::label() { /* No-Op version is tested in AbstractObjectGLTest */