From 1a1c045ef3093b322b24d0d124ee13f980b3d2f4 Mon Sep 17 00:00:00 2001 From: Vladislav Oleshko Date: Sat, 6 Aug 2022 13:04:04 +0300 Subject: [PATCH] Fixes: docs, tests --- src/Magnum/GL/AbstractShaderProgram.h | 6 ++ src/Magnum/GL/Shader.h | 4 + .../GL/Test/AbstractShaderProgramGLTest.cpp | 20 ++++- src/Magnum/GL/Test/ShaderGLTest.cpp | 80 +++++++++++++++---- src/Magnum/Shaders/Test/FlatGLTest.cpp | 45 ++++++++--- 5 files changed, 125 insertions(+), 30 deletions(-) diff --git a/src/Magnum/GL/AbstractShaderProgram.h b/src/Magnum/GL/AbstractShaderProgram.h index 4d18951b5..f72621eca 100644 --- a/src/Magnum/GL/AbstractShaderProgram.h +++ b/src/Magnum/GL/AbstractShaderProgram.h @@ -1260,7 +1260,13 @@ class MAGNUM_GL_EXPORT AbstractShaderProgram: public AbstractObject { /** * @brief Non-blocking linking status check + * @return @cpp true @ce if linking finished, @cpp false @ce otherwise * + * On some drivers this might return false even after + * @ref checkLink() reported successful linking. + * + * @see @fn_gl_keyword{GetProgram} with + * @def_gl_extension{COMPLETION_STATUS,KHR,parallel_shader_compile} */ bool isLinkFinished(); diff --git a/src/Magnum/GL/Shader.h b/src/Magnum/GL/Shader.h index ccc3105db..045d822bd 100644 --- a/src/Magnum/GL/Shader.h +++ b/src/Magnum/GL/Shader.h @@ -661,6 +661,10 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject { /** * @brief Non-blocking compilation status check + * @return @cpp true @ce if shader compilation finished, @cpp false @ce otherwise + * + * @see @fn_gl_keyword{GetProgram} with + * @def_gl_extension{COMPLETION_STATUS,KHR,parallel_shader_compile} * */ bool isCompileFinished(); diff --git a/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp b/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp index 18ec7b602..e378aca0e 100644 --- a/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp +++ b/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include "Magnum/Image.h" #include "Magnum/ImageView.h" @@ -265,8 +266,10 @@ void AbstractShaderProgramGLTest::create() { MAGNUM_VERIFY_NO_GL_ERROR(); CORRADE_VERIFY(linked); - // TODO: decide on this. - //CORRADE_VERIFY(program.isLinkFinished()); + + // Some drivers need a bit of time to update this result + Utility::System::sleep(200); + CORRADE_VERIFY(program.isLinkFinished()); { #if defined(CORRADE_TARGET_APPLE) && !defined(MAGNUM_TARGET_GLES) CORRADE_EXPECT_FAIL("macOS drivers need insane amount of state to validate properly."); @@ -329,7 +332,11 @@ void AbstractShaderProgramGLTest::createAsync() { program.bindAttributeLocation(0, "position"); program.submitLink(); + while(!program.isLinkFinished()) + Utility::System::sleep(100); + CORRADE_VERIFY(program.checkLink()); + CORRADE_VERIFY(program.isLinkFinished()); const bool valid = program.validate().first; MAGNUM_VERIFY_NO_GL_ERROR(); @@ -486,6 +493,9 @@ void AbstractShaderProgramGLTest::linkFailure() { MyPublicShader program; program.attachShaders({shader}); CORRADE_VERIFY(!program.link()); + + Utility::System::sleep(200); + CORRADE_VERIFY(program.isLinkFinished()); } void AbstractShaderProgramGLTest::linkFailureAsync() { @@ -512,11 +522,15 @@ void AbstractShaderProgramGLTest::linkFailureAsync() { std::ostringstream out; Error redirectError{&out}; - program.submitLink(); + + while(!program.isLinkFinished()) + Utility::System::sleep(100); + CORRADE_VERIFY(out.str().empty()); CORRADE_VERIFY(!program.checkLink()); + CORRADE_VERIFY(program.isLinkFinished()); CORRADE_COMPARE_AS(out.str(), "GL::AbstractShaderProgram::link(): linking failed with the following message:", TestSuite::Compare::StringHasPrefix); } diff --git a/src/Magnum/GL/Test/ShaderGLTest.cpp b/src/Magnum/GL/Test/ShaderGLTest.cpp index 32ad1a28b..49ef6cc66 100644 --- a/src/Magnum/GL/Test/ShaderGLTest.cpp +++ b/src/Magnum/GL/Test/ShaderGLTest.cpp @@ -27,6 +27,7 @@ #include /** @todo remove once Shader is -free */ #include #include +#include #include #include "Magnum/GL/Context.h" @@ -42,18 +43,6 @@ namespace Magnum { namespace GL { namespace Test { namespace { -constexpr GL::Version shaderCompileGLVersion = -#ifndef MAGNUM_TARGET_GLES - #ifndef CORRADE_TARGET_APPLE - Version::GL210 - #else - Version::GL310 - #endif - ; -#else -Version::GLES200; -#endif - struct ShaderGLTest: OpenGLTester { explicit ShaderGLTest(); @@ -282,7 +271,19 @@ void ShaderGLTest::addFile() { } void ShaderGLTest::compile() { - Shader shader(shaderCompileGLVersion, Shader::Type::Fragment); + #ifndef MAGNUM_TARGET_GLES + constexpr Version v = + #ifndef CORRADE_TARGET_APPLE + Version::GL210 + #else + Version::GL310 + #endif + ; + #else + constexpr Version v = Version::GLES200; + #endif + + Shader shader(v, Shader::Type::Fragment); shader.addSource("void main() {}\n"); CORRADE_VERIFY(shader.compile()); @@ -290,7 +291,19 @@ void ShaderGLTest::compile() { } void ShaderGLTest::compileFailure() { - Shader shader(shaderCompileGLVersion, Shader::Type::Fragment); + #ifndef MAGNUM_TARGET_GLES + constexpr Version v = + #ifndef CORRADE_TARGET_APPLE + Version::GL210 + #else + Version::GL310 + #endif + ; + #else + constexpr Version v = Version::GLES200; + #endif + + Shader shader(v, Shader::Type::Fragment); shader.addSource("[fu] bleh error #:! stuff\n"); CORRADE_VERIFY(!shader.compile()); @@ -298,23 +311,56 @@ void ShaderGLTest::compileFailure() { } void ShaderGLTest::compileAsync() { - Shader shader(shaderCompileGLVersion, Shader::Type::Fragment); + #ifndef MAGNUM_TARGET_GLES + constexpr Version v = + #ifndef CORRADE_TARGET_APPLE + Version::GL210 + #else + Version::GL310 + #endif + ; + #else + constexpr Version v = Version::GLES200; + #endif + + Shader shader(v, Shader::Type::Fragment); shader.addSource("void main() {}\n"); shader.submitCompile(); + while(!shader.isCompileFinished()) + Utility::System::sleep(100); + CORRADE_VERIFY(shader.checkCompile()); + CORRADE_VERIFY(shader.isCompileFinished()); } void ShaderGLTest::compileAsyncFailure() { - Shader shader(shaderCompileGLVersion, Shader::Type::Fragment); + #ifndef MAGNUM_TARGET_GLES + constexpr Version v = + #ifndef CORRADE_TARGET_APPLE + Version::GL210 + #else + Version::GL310 + #endif + ; + #else + constexpr Version v = Version::GLES200; + #endif + + Shader shader(v, Shader::Type::Fragment); shader.addSource("[fu] bleh error #:! stuff\n"); - shader.submitCompile(); std::ostringstream out; Error redirectError{&out}; + shader.submitCompile(); + + while(!shader.isCompileFinished()) + Utility::System::sleep(100); + CORRADE_VERIFY(out.str().empty()); CORRADE_VERIFY(!shader.checkCompile()); + CORRADE_VERIFY(shader.isCompileFinished()); CORRADE_COMPARE_AS(out.str(), "GL::Shader::compile(): compilation of fragment shader failed with the following message:", TestSuite::Compare::StringHasPrefix); } diff --git a/src/Magnum/Shaders/Test/FlatGLTest.cpp b/src/Magnum/Shaders/Test/FlatGLTest.cpp index 003ab2eb8..2828e2dbf 100644 --- a/src/Magnum/Shaders/Test/FlatGLTest.cpp +++ b/src/Magnum/Shaders/Test/FlatGLTest.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #ifdef CORRADE_TARGET_APPLE @@ -599,19 +600,22 @@ constexpr struct { FlatGLTest::FlatGLTest() { addInstancedTests({ &FlatGLTest::construct<2>, - &FlatGLTest::construct<3>, - &FlatGLTest::constructAsync<2>, - &FlatGLTest::constructAsync<3>}, + &FlatGLTest::construct<3>}, Containers::arraySize(ConstructData)); + addTests({ + &FlatGLTest::constructAsync<2>, + &FlatGLTest::constructAsync<3>}); #ifndef MAGNUM_TARGET_GLES2 addInstancedTests({ &FlatGLTest::constructUniformBuffers<2>, - &FlatGLTest::constructUniformBuffers<3>, - &FlatGLTest::constructUniformBuffersAsync<2>, - &FlatGLTest::constructUniformBuffersAsync<3>}, + &FlatGLTest::constructUniformBuffers<3>}, Containers::arraySize(ConstructUniformBuffersData)); + + addTests({ + &FlatGLTest::constructUniformBuffersAsync<2>, + &FlatGLTest::constructUniformBuffersAsync<3>}); #endif addTests({ @@ -863,7 +867,14 @@ template void FlatGLTest::construct() { template void FlatGLTest::constructAsync() { setTestCaseTemplateName(Utility::format("{}", dimensions)); - auto&& data = ConstructData[testCaseInstanceId()]; + constexpr struct { + const char* name; + FlatGL2D::Flags flags; + } data { + "textured + texture transformation", + FlatGL2D::Flag::Textured|FlatGL2D::Flag::TextureTransformation + }; + setTestCaseDescription(data.name); #ifndef MAGNUM_TARGET_GLES @@ -876,10 +887,12 @@ template void FlatGLTest::constructAsync() { auto compileState = FlatGL::compile(data.flags); CORRADE_COMPARE(compileState.flags(), data.flags); + while(!compileState.isLinkFinished()) + Utility::System::sleep(100); + FlatGL shader{std::move(compileState)}; CORRADE_COMPARE(shader.flags(), data.flags); - // TODO: decide on this. - // CORRADE_VERIFY(shader.isLinkFinished()); + CORRADE_VERIFY(shader.id()); { #if defined(CORRADE_TARGET_APPLE) && !defined(MAGNUM_TARGET_GLES) @@ -938,7 +951,16 @@ template void FlatGLTest::constructUniformBuffers() { template void FlatGLTest::constructUniformBuffersAsync() { setTestCaseTemplateName(Utility::format("{}", dimensions)); - auto&& data = ConstructUniformBuffersData[testCaseInstanceId()]; + constexpr struct { + const char* name; + FlatGL2D::Flags flags; + UnsignedInt materialCount, drawCount; + } data { + "alpha mask", + FlatGL2D::Flag::UniformBuffers|FlatGL2D::Flag::AlphaMask, + 1, 1 + }; + setTestCaseDescription(data.name); #ifndef MAGNUM_TARGET_GLES @@ -968,6 +990,9 @@ template void FlatGLTest::constructUniformBuffersAsync() CORRADE_COMPARE(compileState.materialCount(), data.materialCount); CORRADE_COMPARE(compileState.drawCount(), data.drawCount); + while(!compileState.isLinkFinished()) + Utility::System::sleep(100); + FlatGL shader{std::move(compileState)}; CORRADE_COMPARE(shader.flags(), data.flags); CORRADE_COMPARE(shader.materialCount(), data.materialCount);