Browse Source

GL: add Shader::wrap() and release().

The whole time I thought this class doesn't need such APIs due to being
rather short-lived. But now with async shader compilation it's no longer
so short-lived.
pull/589/head
Vladimír Vondruš 4 years ago
parent
commit
a494b47d4b
  1. 1
      doc/changelog.dox
  2. 4
      doc/opengl-wrapping.dox
  3. 8
      src/Magnum/GL/Shader.cpp
  4. 46
      src/Magnum/GL/Shader.h
  5. 16
      src/Magnum/GL/Test/ShaderGLTest.cpp

1
doc/changelog.dox

@ -412,6 +412,7 @@ See also:
complement @relativeref{GL::Framebuffer::InvalidationAttachment,Depth} and complement @relativeref{GL::Framebuffer::InvalidationAttachment,Depth} and
@relativeref{GL::Framebuffer::InvalidationAttachment,Stencil} (see @relativeref{GL::Framebuffer::InvalidationAttachment,Stencil} (see
[mosra/magnum#554](https://github.com/mosra/magnum/pull/554)) [mosra/magnum#554](https://github.com/mosra/magnum/pull/554))
- Added @ref GL::Shader::wrap() and @relativeref{GL::Shader,release()}
@subsubsection changelog-latest-changes-math Math library @subsubsection changelog-latest-changes-math Math library

4
doc/opengl-wrapping.dox

@ -61,8 +61,8 @@ Magnum object instance using @cpp wrap() @ce:
@snippet MagnumGL.cpp opengl-wrapping-transfer @snippet MagnumGL.cpp opengl-wrapping-transfer
The @cpp wrap() @ce and @cpp release() @ce functions are available for all The @cpp wrap() @ce and @cpp release() @ce functions are available for all
OpenGL classes except for @ref GL::Shader, instances of which are rather OpenGL classes except for @ref GL::AbstractShaderProgram, where the desired
short-lived and thus wrapping external instances makes less sense. usage via subclassing isn't really suited for wrapping external objects.
@attention @attention
Note that interaction with third-party OpenGL code as shown above usually Note that interaction with third-party OpenGL code as shown above usually

8
src/Magnum/GL/Shader.cpp

@ -645,7 +645,7 @@ Int Shader::maxCombinedUniformComponents(const Type type) {
} }
#endif #endif
Shader::Shader(const Version version, const Type type): _type(type), _id(0) { Shader::Shader(const Version version, const Type type): _type{type}, _flags{ObjectFlag::DeleteOnDestruction|ObjectFlag::Created} {
_id = glCreateShader(GLenum(_type)); _id = glCreateShader(GLenum(_type));
switch(version) { switch(version) {
@ -678,9 +678,11 @@ Shader::Shader(const Version version, const Type type): _type(type), _id(0) {
CORRADE_ASSERT_UNREACHABLE("GL::Shader::Shader(): unsupported version" << version, ); CORRADE_ASSERT_UNREACHABLE("GL::Shader::Shader(): unsupported version" << version, );
} }
Shader::Shader(const Type type, const GLuint id, ObjectFlags flags) noexcept: _type{type}, _id{id}, _flags{flags} {}
Shader::~Shader() { Shader::~Shader() {
/* Moved out, nothing to do */ /* Moved out or not deleting on destruction, nothing to do */
if(!_id) return; if(!_id || !(_flags & ObjectFlag::DeleteOnDestruction)) return;
glDeleteShader(_id); glDeleteShader(_id);
} }

46
src/Magnum/GL/Shader.h

@ -526,6 +526,23 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject {
static CORRADE_DEPRECATED("use either submitCompile() and checkCompile() or the zero-argument compile() instead") bool compile(std::initializer_list<Containers::Reference<Shader>> shaders); static CORRADE_DEPRECATED("use either submitCompile() and checkCompile() or the zero-argument compile() instead") bool compile(std::initializer_list<Containers::Reference<Shader>> shaders);
#endif #endif
/**
* @brief Wrap existing OpenGL shader object
* @param type Shader type
* @param id OpenGL shader ID
* @param flags Object creation flags
* @m_since_latest
*
* The @p id is expected to be of an existing OpenGL shader object.
* Unlike a shader created using a constructor, the OpenGL object is by
* default not deleted on destruction, use @p flags for different
* behavior.
* @see @ref release()
*/
static Shader wrap(Type type, GLuint id, ObjectFlags flags = {}) {
return Shader{type, id, flags};
}
/** /**
* @brief Constructor * @brief Constructor
* @param version Target version * @param version Target version
@ -535,7 +552,8 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject {
* corresponding to @p version parameter at the beginning. If * corresponding to @p version parameter at the beginning. If
* @ref Version::None is specified, (not) adding the @glsl #version @ce * @ref Version::None is specified, (not) adding the @glsl #version @ce
* directive is left to the user. * directive is left to the user.
* @see @fn_gl_keyword{CreateShader} * @see @ref Shader(NoCreateT), @ref wrap(),
* @fn_gl_keyword{CreateShader}
*/ */
explicit Shader(Version version, Type type); explicit Shader(Version version, Type type);
@ -564,7 +582,7 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject {
* @brief Destructor * @brief Destructor
* *
* Deletes associated OpenGL shader. * Deletes associated OpenGL shader.
* @see @fn_gl_keyword{DeleteShader} * @see @ref wrap(), @ref release(), @fn_gl_keyword{DeleteShader}
*/ */
~Shader(); ~Shader();
@ -577,6 +595,18 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject {
/** @brief OpenGL shader ID */ /** @brief OpenGL shader ID */
GLuint id() const { return _id; } GLuint id() const { return _id; }
/**
* @brief Release the underlying OpenGL object
* @m_since_latest
*
* Releases ownership of the OpenGL shader object and returns its ID so
* it's not deleted on destruction. The internal state is then
* equivalent to a moved-from state.
* @see @ref wrap()
*/
/* MinGW complains loudly if the declaration doesn't also have inline */
inline GLuint release();
#ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_WEBGL
/** /**
* @brief Shader label * @brief Shader label
@ -711,6 +741,8 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject {
bool isCompileFinished(); bool isCompileFinished();
private: private:
explicit Shader(Type type, GLuint id, ObjectFlags flags) noexcept;
void MAGNUM_GL_LOCAL addSourceImplementationDefault(std::string source); void MAGNUM_GL_LOCAL addSourceImplementationDefault(std::string source);
#if defined(CORRADE_TARGET_EMSCRIPTEN) && defined(__EMSCRIPTEN_PTHREADS__) #if defined(CORRADE_TARGET_EMSCRIPTEN) && defined(__EMSCRIPTEN_PTHREADS__)
void MAGNUM_GL_LOCAL addSourceImplementationEmscriptenPthread(std::string source); void MAGNUM_GL_LOCAL addSourceImplementationEmscriptenPthread(std::string source);
@ -725,6 +757,7 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject {
Type _type; Type _type;
GLuint _id; GLuint _id;
ObjectFlags _flags;
std::vector<std::string> _sources; std::vector<std::string> _sources;
}; };
@ -732,7 +765,7 @@ class MAGNUM_GL_EXPORT Shader: public AbstractObject {
/** @debugoperatorclassenum{Shader,Shader::Type} */ /** @debugoperatorclassenum{Shader,Shader::Type} */
MAGNUM_GL_EXPORT Debug& operator<<(Debug& debug, Shader::Type value); MAGNUM_GL_EXPORT Debug& operator<<(Debug& debug, Shader::Type value);
inline Shader::Shader(Shader&& other) noexcept: _type(other._type), _id(other._id), _sources(std::move(other._sources)) { inline Shader::Shader(Shader&& other) noexcept: _type{other._type}, _id{other._id}, _flags{other._flags}, _sources{std::move(other._sources)} {
other._id = 0; other._id = 0;
} }
@ -740,10 +773,17 @@ inline Shader& Shader::operator=(Shader&& other) noexcept {
using std::swap; using std::swap;
swap(_type, other._type); swap(_type, other._type);
swap(_id, other._id); swap(_id, other._id);
swap(_flags, other._flags);
swap(_sources, other._sources); swap(_sources, other._sources);
return *this; return *this;
} }
inline GLuint Shader::release() {
const GLuint id = _id;
_id = 0;
return id;
}
}} }}
#endif #endif

16
src/Magnum/GL/Test/ShaderGLTest.cpp

@ -49,6 +49,7 @@ struct ShaderGLTest: OpenGLTester {
void construct(); void construct();
void constructNoVersion(); void constructNoVersion();
void constructMove(); void constructMove();
void wrap();
#ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_WEBGL
void label(); void label();
@ -69,6 +70,7 @@ ShaderGLTest::ShaderGLTest() {
addTests({&ShaderGLTest::construct, addTests({&ShaderGLTest::construct,
&ShaderGLTest::constructNoVersion, &ShaderGLTest::constructNoVersion,
&ShaderGLTest::constructMove, &ShaderGLTest::constructMove,
&ShaderGLTest::wrap,
#ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_WEBGL
&ShaderGLTest::label, &ShaderGLTest::label,
@ -160,6 +162,20 @@ void ShaderGLTest::constructMove() {
CORRADE_VERIFY(std::is_nothrow_move_assignable<Shader>::value); CORRADE_VERIFY(std::is_nothrow_move_assignable<Shader>::value);
} }
void ShaderGLTest::wrap() {
GLuint id = glCreateShader(GL_FRAGMENT_SHADER);
/* Releasing won't delete anything */
{
auto shader = Shader::wrap(Shader::Type::Fragment, id, ObjectFlag::DeleteOnDestruction);
CORRADE_COMPARE(shader.release(), id);
}
/* ...so we can wrap it again */
Shader::wrap(Shader::Type::Fragment, id);
glDeleteShader(id);
}
#ifndef MAGNUM_TARGET_WEBGL #ifndef MAGNUM_TARGET_WEBGL
void ShaderGLTest::label() { void ShaderGLTest::label() {
/* No-Op version is tested in AbstractObjectGLTest */ /* No-Op version is tested in AbstractObjectGLTest */

Loading…
Cancel
Save