Browse Source

GL: deprecate list-taking Shader::compile() and ShaderProgram::link().

There's no reason for those to exist anymore -- origiinally they were
added in a hopeful attempt to make use of parallel shader compilation,
but in practice that meant compiling at most two or three shaders at
once and still stalling until that was done, so not that great at all.
The new APIs provide much better opportunities for parallelism.

Fun fact:

    CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());

is actually one character shorter than

    CORRADE_INTERNAL_ASSERT_OUTPUT(GL::Shader::compile({vert, frag}));

so not even typing convenience would be a reason to keep these.
pull/589/head
Vladimír Vondruš 4 years ago
parent
commit
c0c712cc33
  1. 10
      doc/changelog.dox
  2. 4
      doc/snippets/MagnumGL.cpp
  3. 2
      src/Magnum/DebugTools/TextureImage.cpp
  4. 7
      src/Magnum/GL/AbstractShaderProgram.cpp
  5. 18
      src/Magnum/GL/AbstractShaderProgram.h
  6. 9
      src/Magnum/GL/Shader.cpp
  7. 23
      src/Magnum/GL/Shader.h
  8. 28
      src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp
  9. 12
      src/Magnum/GL/Test/MeshGLTest.cpp
  10. 2
      src/Magnum/GL/Test/RendererGLTest.cpp
  11. 2
      src/Magnum/GL/Test/SampleQueryGLTest.cpp
  12. 5
      src/Magnum/GL/Test/TransformFeedbackGLTest.cpp
  13. 2
      src/Magnum/MeshTools/Test/FullScreenTriangleGLTest.cpp
  14. 2
      src/Magnum/TextureTools/DistanceField.cpp

10
doc/changelog.dox

@ -821,6 +821,16 @@ See also:
@ref DebugTools::FrameProfilerGL. The new name plays better with IDE @ref DebugTools::FrameProfilerGL. The new name plays better with IDE
autocompletion and makes the GL-specific class appear next to the autocompletion and makes the GL-specific class appear next to the
API-independent base in alphabetically sorted lists. API-independent base in alphabetically sorted lists.
- List-taking @cpp GL::Shader::compile() @ce and
@cpp GL::AbstractShaderProgram::link() @ce functions are deprecated in
favor of new @ref GL::Shader::submitCompile(),
@relativeref{GL::Shader,checkCompile()},
@ref GL::AbstractShaderProgram::submitLink() and
@relativeref{GL::AbstractShaderProgram,checkLink()} APIs. These were
originally meant to make use of parallel shader compilation, but in
practice that meant compiling at most two or three shaders at once. The new
API allows for much larger parallelism as well as an ability to query
completion status.
- @cpp Shaders::DistanceFieldVector @ce, @cpp Shaders::Flat @ce, - @cpp Shaders::DistanceFieldVector @ce, @cpp Shaders::Flat @ce,
@cpp Shaders::Generic @ce, @cpp Shaders::MeshVisualizer2D @ce, @cpp Shaders::Generic @ce, @cpp Shaders::MeshVisualizer2D @ce,
@cpp Shaders::MeshVisualizer3D @ce, @cpp Shaders::Phong @ce, @cpp Shaders::MeshVisualizer3D @ce, @cpp Shaders::Phong @ce,

4
doc/snippets/MagnumGL.cpp

@ -147,8 +147,8 @@ explicit MyShader() {
vert.addFile("MyShader.vert"); vert.addFile("MyShader.vert");
frag.addFile("MyShader.frag"); frag.addFile("MyShader.frag");
/* Invoke parallel compilation for best performance */ /* Compile them */
CORRADE_INTERNAL_ASSERT_OUTPUT(GL::Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
/* Attach the shaders */ /* Attach the shaders */
attachShaders({vert, frag}); attachShaders({vert, frag});

2
src/Magnum/DebugTools/TextureImage.cpp

@ -87,7 +87,7 @@ FloatReinterpretShader::FloatReinterpretShader() {
vert.addSource(rs.getString("TextureImage.vert")); vert.addSource(rs.getString("TextureImage.vert"));
frag.addSource(rs.getString("TextureImage.frag")); frag.addSource(rs.getString("TextureImage.frag"));
CORRADE_INTERNAL_ASSERT_OUTPUT(GL::Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});
if(!GL::Context::current().isExtensionSupported<GL::Extensions::MAGNUM::shader_vertex_id>()) { if(!GL::Context::current().isExtensionSupported<GL::Extensions::MAGNUM::shader_vertex_id>()) {

7
src/Magnum/GL/AbstractShaderProgram.cpp

@ -586,7 +586,10 @@ void AbstractShaderProgram::transformFeedbackVaryingsImplementationDanglingWorka
#endif #endif
#endif #endif
bool AbstractShaderProgram::link() { return link({*this}); } bool AbstractShaderProgram::link() {
submitLink();
return checkLink({});
}
void AbstractShaderProgram::submitLink() { void AbstractShaderProgram::submitLink() {
glLinkProgram(_id); glLinkProgram(_id);
@ -637,12 +640,14 @@ bool AbstractShaderProgram::checkLink(const Containers::Iterable<Shader> shaders
return success; return success;
} }
#ifdef MAGNUM_BUILD_DEPRECATED
bool AbstractShaderProgram::link(std::initializer_list<Containers::Reference<AbstractShaderProgram>> shaders) { bool AbstractShaderProgram::link(std::initializer_list<Containers::Reference<AbstractShaderProgram>> shaders) {
for(AbstractShaderProgram& shader: shaders) shader.submitLink(); for(AbstractShaderProgram& shader: shaders) shader.submitLink();
bool allSuccess = true; bool allSuccess = true;
for(AbstractShaderProgram& shader: shaders) allSuccess = allSuccess && shader.checkLink({}); for(AbstractShaderProgram& shader: shaders) allSuccess = allSuccess && shader.checkLink({});
return allSuccess; return allSuccess;
} }
#endif
bool AbstractShaderProgram::isLinkFinished() { bool AbstractShaderProgram::isLinkFinished() {
GLint success; GLint success;

18
src/Magnum/GL/AbstractShaderProgram.h

@ -43,6 +43,7 @@
#endif #endif
#ifdef MAGNUM_BUILD_DEPRECATED #ifdef MAGNUM_BUILD_DEPRECATED
#include <Corrade/Utility/Macros.h>
/* For label() / setLabel(), which used to be a std::string */ /* For label() / setLabel(), which used to be a std::string */
#include <Corrade/Containers/StringStl.h> #include <Corrade/Containers/StringStl.h>
#endif #endif
@ -1345,15 +1346,22 @@ class MAGNUM_GL_EXPORT AbstractShaderProgram: public AbstractObject {
bool isLinkFinished(); bool isLinkFinished();
protected: protected:
/** @brief Link the shaders #ifdef MAGNUM_BUILD_DEPRECATED
/**
* @brief Link multiple shaders simultaenously
* @m_deprecated_since_latest Originally meant to batch multiple link
* operations together in a way that allowed the driver to perform
* the linking in multiple threads. Superseded by @ref submitLink()
* and @ref checkLink(), use either those or the zero-argument
* @ref link() instead. See @ref GL-AbstractShaderProgram-async
* for more information.
* *
* Calls @ref submitLink() on all shaders first, then @ref checkLink(). * Calls @ref submitLink() on all shaders first, then @ref checkLink().
* Returns @cpp false @ce if linking of any shader failed, @cpp true @ce * Returns @cpp false @ce if linking of any shader failed, @cpp true @ce
* if everything succeeded. The operation is batched in a * if everything succeeded.
* way that allows the driver to link multiple shaders simultaneously
* (i.e. in multiple threads).
*/ */
static bool link(std::initializer_list<Containers::Reference<AbstractShaderProgram>> shaders); static CORRADE_DEPRECATED("use either submitLink() and checkLink() or the zero-argument link() instead") bool link(std::initializer_list<Containers::Reference<AbstractShaderProgram>> shaders);
#endif
#if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL) #if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL)
/** /**

9
src/Magnum/GL/Shader.cpp

@ -27,7 +27,9 @@
#include "Shader.h" #include "Shader.h"
#include <Corrade/Containers/Array.h> #include <Corrade/Containers/Array.h>
#ifdef MAGNUM_BUILD_DEPRECATED
#include <Corrade/Containers/Reference.h> #include <Corrade/Containers/Reference.h>
#endif
#ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_WEBGL
#include <Corrade/Containers/String.h> #include <Corrade/Containers/String.h>
#endif #endif
@ -748,7 +750,10 @@ Shader& Shader::addFile(const std::string& filename) {
return *this; return *this;
} }
bool Shader::compile() { return compile({*this}); } bool Shader::compile() {
submitCompile();
return checkCompile();
}
void Shader::submitCompile() { void Shader::submitCompile() {
CORRADE_ASSERT(_sources.size() > 1, "GL::Shader::compile(): no files added", ); CORRADE_ASSERT(_sources.size() > 1, "GL::Shader::compile(): no files added", );
@ -808,6 +813,7 @@ bool Shader::checkCompile() {
return success; return success;
} }
#ifdef MAGNUM_BUILD_DEPRECATED
bool Shader::compile(std::initializer_list<Containers::Reference<Shader>> shaders) { bool Shader::compile(std::initializer_list<Containers::Reference<Shader>> shaders) {
/* Invoke (possibly parallel) compilation on all shaders */ /* Invoke (possibly parallel) compilation on all shaders */
for(Shader& shader: shaders) shader.submitCompile(); for(Shader& shader: shaders) shader.submitCompile();
@ -815,6 +821,7 @@ bool Shader::compile(std::initializer_list<Containers::Reference<Shader>> shader
for(Shader& shader: shaders) allSuccess = allSuccess && shader.checkCompile(); for(Shader& shader: shaders) allSuccess = allSuccess && shader.checkCompile();
return allSuccess; return allSuccess;
} }
#endif
bool Shader::isCompileFinished() { bool Shader::isCompileFinished() {
GLint success; GLint success;

23
src/Magnum/GL/Shader.h

@ -39,6 +39,7 @@
#include "Magnum/GL/GL.h" #include "Magnum/GL/GL.h"
#ifdef MAGNUM_BUILD_DEPRECATED #ifdef MAGNUM_BUILD_DEPRECATED
#include <Corrade/Utility/Macros.h>
/* For label() / setLabel(), which used to be a std::string. Not ideal for the /* For label() / setLabel(), which used to be a std::string. Not ideal for the
return type, but at least something. */ return type, but at least something. */
#include <Corrade/Containers/StringStl.h> #include <Corrade/Containers/StringStl.h>
@ -508,16 +509,22 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject {
static Int maxCombinedUniformComponents(Type type); static Int maxCombinedUniformComponents(Type type);
#endif #endif
#ifdef MAGNUM_BUILD_DEPRECATED
/** /**
* @brief Compile multiple shaders simultaneously * @brief Compile multiple shaders simultaneously
* * @m_deprecated_since_latest Originally meant to batch multiple
* Calls @ref submitCompile() on all shaders first, then @ref checkCompile(). * compile operations together in a way that allowed the driver to
* Returns @cpp false @ce if compilation of any shader failed, * perform the compilation in multiple threads. Superseded by
* @cpp true @ce if everything succeeded. The operation is batched in a way that * @ref submitCompile() and @ref checkCompile(), use either those
* allows the driver to perform multiple compilations simultaneously * or the zero-argument @ref compile() instead. See
* (i.e. in multiple threads). * @ref GL-AbstractShaderProgram-async for more information.
*/ *
static bool compile(std::initializer_list<Containers::Reference<Shader>> shaders); * Calls @ref submitCompile() on all shaders first, then
* @ref checkCompile(). Returns @cpp false @ce if compilation of any
* shader failed, @cpp true @ce if everything succeeded.
*/
static CORRADE_DEPRECATED("use either submitCompile() and checkCompile() or the zero-argument compile() instead") bool compile(std::initializer_list<Containers::Reference<Shader>> shaders);
#endif
/** /**
* @brief Constructor * @brief Constructor

28
src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp

@ -652,7 +652,8 @@ void AbstractShaderProgramGLTest::uniformNotFound() {
#endif #endif
); );
CORRADE_VERIFY(Shader::compile({vert, frag})); CORRADE_VERIFY(vert.compile() && frag.compile());
program.attachShaders({vert, frag}); program.attachShaders({vert, frag});
CORRADE_VERIFY(program.link()); CORRADE_VERIFY(program.link());
@ -702,10 +703,10 @@ MyShader::MyShader() {
Version::GLES200 Version::GLES200
#endif #endif
, Shader::Type::Fragment); , Shader::Type::Fragment);
vert.addSource(rs.getString("MyShader.vert")); vert.addSource(rs.getString("MyShader.vert"))
frag.addSource(rs.getString("MyShader.frag")); .compile();
frag.addSource(rs.getString("MyShader.frag"))
Shader::compile({vert, frag}); .compile();
attachShaders({vert, frag}); attachShaders({vert, frag});
@ -786,10 +787,10 @@ MyDoubleShader::MyDoubleShader() {
Shader vert(Version::GL320, Shader::Type::Vertex); Shader vert(Version::GL320, Shader::Type::Vertex);
Shader frag(Version::GL320, Shader::Type::Fragment); Shader frag(Version::GL320, Shader::Type::Fragment);
vert.addSource(rs.getString("MyDoubleShader.vert")); vert.addSource(rs.getString("MyDoubleShader.vert"))
frag.addSource(rs.getString("MyDoubleShader.frag")); .compile();
frag.addSource(rs.getString("MyDoubleShader.frag"))
Shader::compile({vert, frag}); .compile();
attachShaders({vert, frag}); attachShaders({vert, frag});
@ -948,8 +949,8 @@ void AbstractShaderProgramGLTest::uniformBlockIndexNotFound() {
vert.addSource("void main() { gl_Position = vec4(0.0); }"); vert.addSource("void main() { gl_Position = vec4(0.0); }");
frag.addSource("out lowp vec4 color;\n" frag.addSource("out lowp vec4 color;\n"
"void main() { color = vec4(1.0); }"); "void main() { color = vec4(1.0); }");
CORRADE_VERIFY(vert.compile() && frag.compile());
CORRADE_VERIFY(Shader::compile({vert, frag}));
program.attachShaders({vert, frag}); program.attachShaders({vert, frag});
CORRADE_VERIFY(program.link()); CORRADE_VERIFY(program.link());
@ -989,10 +990,11 @@ UniformBlockShader::UniformBlockShader() {
Version::GLES300 Version::GLES300
#endif #endif
, Shader::Type::Fragment); , Shader::Type::Fragment);
vert.addSource(rs.getString("UniformBlockShader.vert")); vert.addSource(rs.getString("UniformBlockShader.vert"))
frag.addSource(rs.getString("UniformBlockShader.frag")); .compile();
frag.addSource(rs.getString("UniformBlockShader.frag"))
.compile();
Shader::compile({vert, frag});
attachShaders({vert, frag}); attachShaders({vert, frag});
link(); link();

12
src/Magnum/GL/Test/MeshGLTest.cpp

@ -1023,7 +1023,7 @@ FloatShader::FloatShader(const std::string& type, const std::string& conversion)
"#endif\n" "#endif\n"
"void main() { result = " + conversion + "; }\n"); "void main() { result = " + conversion + "; }\n");
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});
@ -1065,7 +1065,7 @@ IntegerShader::IntegerShader(const std::string& type) {
"out mediump " + type + " result;\n" "out mediump " + type + " result;\n"
"void main() { result = valueInterpolated; }\n"); "void main() { result = valueInterpolated; }\n");
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});
@ -1100,7 +1100,7 @@ DoubleShader::DoubleShader(const std::string& type, const std::string& outputTyp
"out " + outputType + " result;\n" "out " + outputType + " result;\n"
"void main() { result = valueInterpolated; }\n"); "void main() { result = valueInterpolated; }\n");
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});
@ -2147,7 +2147,7 @@ MultipleShader::MultipleShader() {
"#endif\n" "#endif\n"
"void main() { result = valueInterpolated; }\n"); "void main() { result = valueInterpolated; }\n");
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});
@ -3836,7 +3836,7 @@ MultiDrawShader::MultiDrawShader(bool vertexId, bool drawId) {
"#endif\n" "#endif\n"
"void main() { result.r = valueInterpolated; }\n"); "void main() { result.r = valueInterpolated; }\n");
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});
@ -4641,7 +4641,7 @@ MultiDrawInstancedShader::MultiDrawInstancedShader(bool vertexId, bool drawId
"#endif\n" "#endif\n"
"void main() { result.r = valueInterpolated; }\n"); "void main() { result.r = valueInterpolated; }\n");
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});

2
src/Magnum/GL/Test/RendererGLTest.cpp

@ -203,7 +203,7 @@ void RendererGLTest::pointCoord() {
color = vec4(gl_PointCoord.x, gl_PointCoord.y, 0.0, 1.0); color = vec4(gl_PointCoord.x, gl_PointCoord.y, 0.0, 1.0);
})GLSL"); })GLSL");
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});

2
src/Magnum/GL/Test/SampleQueryGLTest.cpp

@ -207,7 +207,7 @@ MyShader::MyShader() {
" color = vec4(1.0, 1.0, 1.0, 1.0);\n" " color = vec4(1.0, 1.0, 1.0, 1.0);\n"
"}\n"); "}\n");
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});

5
src/Magnum/GL/Test/TransformFeedbackGLTest.cpp

@ -635,7 +635,8 @@ void TransformFeedbackGLTest::draw() {
else geom.addSource( else geom.addSource(
" EmitVertex();\n"); " EmitVertex();\n");
geom.addSource("}\n"); geom.addSource("}\n");
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, geom})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && geom.compile());
attachShaders({vert, geom}); attachShaders({vert, geom});
setTransformFeedbackOutputs({"geomOutput"}, TransformFeedbackBufferMode::SeparateAttributes); setTransformFeedbackOutputs({"geomOutput"}, TransformFeedbackBufferMode::SeparateAttributes);
CORRADE_INTERNAL_ASSERT_OUTPUT(link()); CORRADE_INTERNAL_ASSERT_OUTPUT(link());
@ -696,8 +697,8 @@ void TransformFeedbackGLTest::draw() {
"void main() {\n" "void main() {\n"
" outputData = interleaved.x + 2*interleaved.y;\n" " outputData = interleaved.x + 2*interleaved.y;\n"
"}\n"); "}\n");
CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag}));
attachShaders({vert, frag}); attachShaders({vert, frag});
bindAttributeLocation(Input::Location, "inputData"); bindAttributeLocation(Input::Location, "inputData");
CORRADE_INTERNAL_ASSERT_OUTPUT(link()); CORRADE_INTERNAL_ASSERT_OUTPUT(link());

2
src/Magnum/MeshTools/Test/FullScreenTriangleGLTest.cpp

@ -102,7 +102,7 @@ void main() {
} }
)"); )");
CORRADE_INTERNAL_ASSERT_OUTPUT(GL::Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});

2
src/Magnum/TextureTools/DistanceField.cpp

@ -104,7 +104,7 @@ DistanceFieldShader::DistanceFieldShader(const UnsignedInt radius) {
frag.addSource(Utility::formatString("#define RADIUS {}\n", radius)) frag.addSource(Utility::formatString("#define RADIUS {}\n", radius))
.addSource(rs.getString("DistanceFieldShader.frag")); .addSource(rs.getString("DistanceFieldShader.frag"));
CORRADE_INTERNAL_ASSERT_OUTPUT(GL::Shader::compile({vert, frag})); CORRADE_INTERNAL_ASSERT_OUTPUT(vert.compile() && frag.compile());
attachShaders({vert, frag}); attachShaders({vert, frag});

Loading…
Cancel
Save