diff --git a/doc/snippets/MagnumGL.cpp b/doc/snippets/MagnumGL.cpp index 06adbcf01..98755e21a 100644 --- a/doc/snippets/MagnumGL.cpp +++ b/doc/snippets/MagnumGL.cpp @@ -25,6 +25,7 @@ #include /* for std::tie() :( */ #include +#include #include #include @@ -300,7 +301,7 @@ MyShader::MyShader(NoInitT) {} MyShader::MyShader(CompileState&& state): MyShader{static_cast(std::move(state))} { - CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink()); + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag})); DOXYGEN_ELLIPSIS() } diff --git a/src/Magnum/GL/AbstractShaderProgram.cpp b/src/Magnum/GL/AbstractShaderProgram.cpp index 4b5e53be7..c26f0cab9 100644 --- a/src/Magnum/GL/AbstractShaderProgram.cpp +++ b/src/Magnum/GL/AbstractShaderProgram.cpp @@ -27,6 +27,7 @@ #include "AbstractShaderProgram.h" #include +#include #include #ifndef MAGNUM_TARGET_WEBGL #include @@ -591,7 +592,13 @@ void AbstractShaderProgram::submitLink() { glLinkProgram(_id); } -bool AbstractShaderProgram::checkLink() { +bool AbstractShaderProgram::checkLink(const Containers::Iterable shaders) { + /* If any compilation failed, abort without even checking the link status. + The checkCompile() API is called always, to print also compilation + warnings even in case everything still manages to link well. */ + for(Shader& shader: shaders) + if(!shader.checkCompile()) return false; + GLint success, logLength; glGetProgramiv(_id, GL_LINK_STATUS, &success); glGetProgramiv(_id, GL_INFO_LOG_LENGTH, &logLength); @@ -633,7 +640,7 @@ bool AbstractShaderProgram::checkLink() { bool AbstractShaderProgram::link(std::initializer_list> shaders) { for(AbstractShaderProgram& shader: shaders) shader.submitLink(); bool allSuccess = true; - for(AbstractShaderProgram& shader: shaders) allSuccess = allSuccess && shader.checkLink(); + for(AbstractShaderProgram& shader: shaders) allSuccess = allSuccess && shader.checkLink({}); return allSuccess; } diff --git a/src/Magnum/GL/AbstractShaderProgram.h b/src/Magnum/GL/AbstractShaderProgram.h index abba41a1e..10b804461 100644 --- a/src/Magnum/GL/AbstractShaderProgram.h +++ b/src/Magnum/GL/AbstractShaderProgram.h @@ -446,9 +446,9 @@ the application to schedule other work in the meantime. Async compilation and linking can be implemented by using @ref Shader::submitCompile() and @ref submitLink(), followed by -@ref checkLink() (and optionally @ref Shader::checkCompile()), instead of -@ref Shader::compile() and @ref link(). Calling the submit functions will -trigger a (potentially async) compilation and linking, calling the check +@ref checkLink() (which optionally delegates to @ref Shader::checkCompile()), +instead of @ref Shader::compile() and @ref link(). Calling the submit functions +will trigger a (potentially async) compilation and linking, calling the check functions will check the operation result, potentially stalling if the async operation isn't finished yet. @@ -476,9 +476,9 @@ creation capability while keeping also the simple constructor is the following: @cpp CompileState @ce instance. 4. A @cpp MyShader(CompileState&&) @ce constructor then takes over the base of @cpp CompileState @ce by delegating it into the move constructor. Then - it calls @ref checkLink() (and if that fails also @ref Shader::checkCompile() - to provide more context) and then performs any remaining post-link steps - such as uniform setup. + it calls @ref checkLink(), passing all input shaders to it for a complete + context in case of an error, and finally performs any remaining post-link + steps such as uniform setup. 5. The original @cpp MyShader(…) @ce constructor now only passes the result of @cpp compile() @ce to @cpp MyShader(CompileState&&) @ce. @@ -1551,16 +1551,31 @@ class MAGNUM_GL_EXPORT AbstractShaderProgram: public AbstractObject { * @m_since_latest * * Has to be called only if @ref submitLink() was called before. + * + * If @p shaders are not empty, first calls @ref Shader::checkCompile() + * on each. If a compilation failure is reached, returns @cpp false @ce + * without even checking link status. To have error messages with full + * context in case of a failed shader compilation or linking, an + * application is encouraged to pass all input @ref Shader instances to + * this function or, if not possible, explicitly call + * @ref Shader::checkCompile() on each. + * + * Then, link status is checked and a message (if any) is printed * Returns @cpp false @ce if linking failed, @cpp true @ce on success. - * Linker message (if any) is printed to error output. The function - * will stall until a (potentially async) linking operation finishes, - * you can use @ref isLinkFinished() to check the status instead. See - * @ref GL-AbstractShaderProgram-async for more information. + * If linking failed, it first goes through @p shaders and calls + * @ref Shader::checkCompile() on each until a failure is reached. If + * no compilation failed, a linker message is printed to error output. + * The function will stall until a (potentially async) linking + * operation finishes, you can use @ref isLinkFinished() to check the + * status instead. See @ref GL-AbstractShaderProgram-async for more + * information. * @see @ref Shader::checkCompile(), @fn_gl_keyword{GetProgram} with * @def_gl{LINK_STATUS} and @def_gl{INFO_LOG_LENGTH}, * @fn_gl_keyword{GetProgramInfoLog} */ - bool checkLink(); + /* No default argument is provided in order to *really* encourage apps + to pass the shaders here */ + bool checkLink(Containers::Iterable shaders); /** * @brief Get uniform location diff --git a/src/Magnum/GL/Shader.h b/src/Magnum/GL/Shader.h index 1ae65dcfc..5850e0ba6 100644 --- a/src/Magnum/GL/Shader.h +++ b/src/Magnum/GL/Shader.h @@ -645,13 +645,16 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject { * @m_since_latest * * You can call @ref isCompileFinished() or @ref checkCompile() after, - * but in most cases it's enough to defer that to after + * but it's recommended to instead immediately call * @ref AbstractShaderProgram::attachShader() and - * @relativeref{AbstractShaderProgram,submitLink()} were called, and - * then continuing with @relativeref{AbstractShaderProgram,isLinkFinished()} - * or @relativeref{AbstractShaderProgram,checkLink()} on the final - * program --- if compilation would fail, subsequent linking will as - * well. See @ref GL-AbstractShaderProgram-async for more information. + * @relativeref{AbstractShaderProgram,submitLink()}, then optionally + * continue with @relativeref{AbstractShaderProgram,isLinkFinished()} + * and pass all input shaders to + * @relativeref{AbstractShaderProgram,checkLink()} on the final program + * --- if compilation would fail, subsequent linking will as well, and + * @relativeref{AbstractShaderProgram,checkLink()} will print the + * compilation error if linking failed due to that. See + * @ref GL-AbstractShaderProgram-async for more information. * @see @fn_gl_keyword{ShaderSource}, @fn_gl_keyword{CompileShader} */ void submitCompile(); @@ -660,14 +663,17 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject { * @brief Check shader compilation status and await completion * @m_since_latest * - * Has to be called only if @ref submitCompile() was called before. In - * most cases it's enough to defer this check to after + * Has to be called only if @ref submitCompile() was called before. + * It's however recommended to instead immediately call * @ref AbstractShaderProgram::attachShader() and - * @relativeref{AbstractShaderProgram,submitLink()} were called, and - * then continuing with @relativeref{AbstractShaderProgram,isLinkFinished()} - * or @relativeref{AbstractShaderProgram,checkLink()} on the final - * program --- if compilation would fail, subsequent linking will as - * well. See @ref GL-AbstractShaderProgram-async for more information. + * @relativeref{AbstractShaderProgram,submitLink()}, then optionally + * continue with @relativeref{AbstractShaderProgram,isLinkFinished()} + * and pass all input shaders to + * @relativeref{AbstractShaderProgram,checkLink()} on the final program + * --- if compilation would fail, subsequent linking will as well, and + * @relativeref{AbstractShaderProgram,checkLink()} will print the + * compilation error if linking failed due to that. See + * @ref GL-AbstractShaderProgram-async for more information. * @see @fn_gl_keyword{GetShader} with @def_gl{COMPILE_STATUS} and * @def_gl{INFO_LOG_LENGTH}, @fn_gl_keyword{GetShaderInfoLog} */ @@ -684,7 +690,7 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject { * available, the function always returns @cpp true @ce --- i.e., as if * the compilation was done synchronously. * - * In most cases it's enough to only wait for the final link to finish, + * It's however recommended to wait only for the final link to finish, * and not for particular compilations --- i.e., right after * @ref submitCompile() continue with * @ref AbstractShaderProgram::attachShader() and diff --git a/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp b/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp index 20753b820..9b90f75da 100644 --- a/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp +++ b/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp @@ -24,6 +24,7 @@ */ #include +#include #include #include /** @todo remove when Shader is -free */ #include @@ -74,6 +75,7 @@ struct AbstractShaderProgramGLTest: OpenGLTester { void linkFailure(); void linkFailureAsync(); + void linkFailureAsyncShaderList(); void uniformNotFound(); @@ -116,6 +118,7 @@ AbstractShaderProgramGLTest::AbstractShaderProgramGLTest() { &AbstractShaderProgramGLTest::linkFailure, &AbstractShaderProgramGLTest::linkFailureAsync, + &AbstractShaderProgramGLTest::linkFailureAsyncShaderList, &AbstractShaderProgramGLTest::uniformNotFound, &AbstractShaderProgramGLTest::uniform, @@ -335,7 +338,7 @@ void AbstractShaderProgramGLTest::createAsync() { while(!program.isLinkFinished()) Utility::System::sleep(100); - CORRADE_VERIFY(program.checkLink()); + CORRADE_VERIFY(program.checkLink({vert, frag})); CORRADE_VERIFY(program.isLinkFinished()); const bool valid = program.validate().first; @@ -544,16 +547,76 @@ void AbstractShaderProgramGLTest::linkFailureAsync() { CORRADE_VERIFY(out.str().empty()); /* ... only the final check should. In this case it's "error: linking with - uncompiled/unspecialized shader" as well. */ + uncompiled/unspecialized shader" as well, but if the shaders would be + supplied like in linkFailureAsyncShaderList() below, it'd print the + shader failure instead. */ { Error redirectError{&out}; - CORRADE_VERIFY(!program.checkLink()); + 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); } +void AbstractShaderProgramGLTest::linkFailureAsyncShaderList() { + Shader vert( + #ifndef MAGNUM_TARGET_GLES + #ifndef CORRADE_TARGET_APPLE + Version::GL210 + #else + Version::GL310 + #endif + #else + Version::GLES200 + #endif + , Shader::Type::Vertex); + vert.addSource("void main() {}\n"); + + Shader frag( + #ifndef MAGNUM_TARGET_GLES + #ifndef CORRADE_TARGET_APPLE + Version::GL210 + #else + Version::GL310 + #endif + #else + Version::GLES200 + #endif + , Shader::Type::Fragment); + frag.addSource("[fu] bleh error #:! stuff\n"); + + vert.submitCompile(); + frag.submitCompile(); + + MyPublicShader program; + program.attachShaders({vert, frag}); + + /* The link submission should not print anything ... */ + { + std::ostringstream out; + Error redirectError{&out}; + program.submitLink(); + CORRADE_VERIFY(out.str().empty()); + } + + /* ... only the final check should. Vertex shader should be fine, but + fragment should fail. */ + std::ostringstream out; + { + Error redirectError{&out}; + CORRADE_VERIFY(!program.checkLink({vert, frag})); + } + CORRADE_COMPARE_AS(out.str(), "GL::Shader::compile(): compilation of fragment shader failed with the following message:", + TestSuite::Compare::StringHasPrefix); + + /* The linker error (which would most probably say something like "error: + linking with uncompiled/unspecialized shader") should not be even + printed */ + CORRADE_COMPARE_AS(out.str(), "GL::AbstractShaderProgram::link(): linking failed with the following message:", + TestSuite::Compare::StringNotContains); +} + void AbstractShaderProgramGLTest::uniformNotFound() { MyPublicShader program; diff --git a/src/Magnum/Shaders/DistanceFieldVectorGL.cpp b/src/Magnum/Shaders/DistanceFieldVectorGL.cpp index be4dd12f5..648330122 100644 --- a/src/Magnum/Shaders/DistanceFieldVectorGL.cpp +++ b/src/Magnum/Shaders/DistanceFieldVectorGL.cpp @@ -27,6 +27,7 @@ #include "DistanceFieldVectorGL.h" #include +#include #include #include @@ -170,7 +171,7 @@ template DistanceFieldVectorGL::DistanceFiel if(!id()) return; #endif - CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink()); + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag})); const GL::Context& context = GL::Context::current(); const GL::Version version = state._version; diff --git a/src/Magnum/Shaders/FlatGL.cpp b/src/Magnum/Shaders/FlatGL.cpp index af12286a3..1d61fed6c 100644 --- a/src/Magnum/Shaders/FlatGL.cpp +++ b/src/Magnum/Shaders/FlatGL.cpp @@ -27,6 +27,7 @@ #include "FlatGL.h" #include +#include #include #include @@ -245,7 +246,7 @@ template FlatGL::FlatGL(CompileState&& state if(!id()) return; #endif - CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink()); + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag})); const GL::Context& context = GL::Context::current(); const GL::Version version = state._version; diff --git a/src/Magnum/Shaders/MeshVisualizerGL.cpp b/src/Magnum/Shaders/MeshVisualizerGL.cpp index 80cde4f35..6c079655b 100644 --- a/src/Magnum/Shaders/MeshVisualizerGL.cpp +++ b/src/Magnum/Shaders/MeshVisualizerGL.cpp @@ -27,6 +27,7 @@ #include "MeshVisualizerGL.h" #include +#include #include #include #include @@ -536,7 +537,10 @@ MeshVisualizerGL2D::MeshVisualizerGL2D(CompileState&& state): MeshVisualizerGL2D if(!id()) return; #endif - CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink()); + if(state._geom) + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag, *state._geom})); + else + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag})); const GL::Context& context = GL::Context::current(); const GL::Version version = state._version; @@ -912,7 +916,10 @@ MeshVisualizerGL3D::MeshVisualizerGL3D(CompileState&& state): MeshVisualizerGL3D if(!id()) return; #endif - CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink()); + if(state._geom) + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag, *state._geom})); + else + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag})); const GL::Context& context = GL::Context::current(); const GL::Version version = state._version; diff --git a/src/Magnum/Shaders/PhongGL.cpp b/src/Magnum/Shaders/PhongGL.cpp index f68258dc8..6e511a0ba 100644 --- a/src/Magnum/Shaders/PhongGL.cpp +++ b/src/Magnum/Shaders/PhongGL.cpp @@ -30,6 +30,7 @@ #include #endif #include +#include #include #include #include @@ -355,7 +356,7 @@ PhongGL::PhongGL(CompileState&& state): PhongGL{static_cast(std::move if(!id()) return; #endif - CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink()); + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag})); const GL::Context& context = GL::Context::current(); const GL::Version version = state._version; diff --git a/src/Magnum/Shaders/VectorGL.cpp b/src/Magnum/Shaders/VectorGL.cpp index b9f1b597b..89b5353e3 100644 --- a/src/Magnum/Shaders/VectorGL.cpp +++ b/src/Magnum/Shaders/VectorGL.cpp @@ -27,6 +27,7 @@ #include "VectorGL.h" #include +#include #include #include @@ -172,7 +173,7 @@ template VectorGL::VectorGL(CompileState&& s if(!id()) return; #endif - CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink()); + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag})); const GL::Context& context = GL::Context::current(); const GL::Version version = state._version; diff --git a/src/Magnum/Shaders/VertexColorGL.cpp b/src/Magnum/Shaders/VertexColorGL.cpp index 0853e52f2..048e95653 100644 --- a/src/Magnum/Shaders/VertexColorGL.cpp +++ b/src/Magnum/Shaders/VertexColorGL.cpp @@ -27,6 +27,7 @@ #include "VertexColorGL.h" #include +#include #include #include @@ -150,7 +151,7 @@ template VertexColorGL::VertexColorGL(Compil if(!id()) return; #endif - CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink()); + CORRADE_INTERNAL_ASSERT_OUTPUT(checkLink({state._vert, state._frag})); const GL::Context& context = GL::Context::current(); const GL::Version version = state._version;