From 43c0016ed3e6d4898444735c915440c24e96b505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 9 May 2012 01:21:46 +0200 Subject: [PATCH] Move semantic for Shader class. The class is now created always on the stack, so the user doesn't have to delete it explicitly. It's now possible to write less verbose shader code, instead of three lines before: Shader* s = Shader::fromFile(Shader::Fragment, "Shader.frag"); attachShader(s); // ... delete s; It's now only one: attachShader(Shader::fromFile(Shader::Fragment, "Shader.frag")); --- src/AbstractShaderProgram.cpp | 6 ++--- src/AbstractShaderProgram.h | 18 +++++++------- src/Shader.cpp | 17 +++++++++++++ src/Shader.h | 45 ++++++++++++++++++----------------- src/Shaders/PhongShader.cpp | 8 ++----- 5 files changed, 53 insertions(+), 41 deletions(-) diff --git a/src/AbstractShaderProgram.cpp b/src/AbstractShaderProgram.cpp index d7c16d41a..aaeb0d3ad 100644 --- a/src/AbstractShaderProgram.cpp +++ b/src/AbstractShaderProgram.cpp @@ -39,10 +39,8 @@ bool AbstractShaderProgram::use() { return true; } -bool AbstractShaderProgram::attachShader(Shader* shader) { - if(!shader) return false; - - GLuint _shader = shader->compile(); +bool AbstractShaderProgram::attachShader(Shader& shader) { + GLuint _shader = shader.compile(); if(!_shader) return false; glAttachShader(program, _shader); diff --git a/src/AbstractShaderProgram.h b/src/AbstractShaderProgram.h index c57caaf44..199dbddc2 100644 --- a/src/AbstractShaderProgram.h +++ b/src/AbstractShaderProgram.h @@ -44,20 +44,16 @@ typedef Attribute<2, Vector2> TextureCoords; binds attribute locations and gets uniform locations, for example: @code // Load shaders from file and attach them to the program -Shader* vertexShader = Shader::fromFile(Shader::Vertex, "PhongShader.vert"); -Shader* fragmentShader = Shader::fromFile(Shader::Fragment, "PhongShader.frag"); -attachShader(vertexShader); -attachShader(fragmentShader); +attachShader(Shader::fromFile(Shader::Vertex, "PhongShader.vert")); +attachShader(Shader::fromFile(Shader::Fragment, "PhongShader.frag")); // Bind attribute names to IDs bindAttribute(Vertex::Location, "vertex"); bindAttribute(Normal::Location, "normal"); bindAttribute(TextureCoords::Location, "textureCoords"); -// Link, then delete now uneeded shaders +// Link link(); -delete vertexShader; -delete fragmentShader; // Get locations of uniforms transformationMatrixUniform = uniformLocation("transformationMatrix"); @@ -133,9 +129,13 @@ class MAGNUM_EXPORT AbstractShaderProgram { * * Compiles the shader, if it is not already, and prepares it for * linking. - * @note The shader should be deleted by caller after linking. */ - bool attachShader(Shader* shader); + bool attachShader(Shader& shader); + + /** @copydoc attachShader(Shader&) */ + inline bool attachShader(Shader&& shader) { + return attachShader(shader); + } /** * @brief Bind attribute to given location diff --git a/src/Shader.cpp b/src/Shader.cpp index fa8f627bf..f87b81df0 100644 --- a/src/Shader.cpp +++ b/src/Shader.cpp @@ -24,6 +24,23 @@ using namespace Corrade::Utility; namespace Magnum { +Shader::Shader(Shader&& other): _type(other._type), _state(other._state), sources(other.sources), shader(other.shader) { + other.shader = 0; +} + +Shader& Shader::operator=(Shader&& other) { + if(shader) glDeleteShader(shader); + + _type = other._type; + _state = other._state; + sources = other.sources; + shader = other.shader; + + other.shader = 0; + + return *this; +} + bool Shader::addFile(const std::string& filename) { /* Open file */ ifstream file(filename.c_str()); diff --git a/src/Shader.h b/src/Shader.h index a3838558b..2511d32d6 100644 --- a/src/Shader.h +++ b/src/Shader.h @@ -35,9 +35,7 @@ namespace Magnum { */ class MAGNUM_EXPORT Shader { Shader(const Shader& other) = delete; - Shader(Shader&& other) = delete; Shader& operator=(const Shader& other) = delete; - Shader& operator=(Shader&& other) = delete; public: /** @brief %Shader type */ @@ -79,19 +77,19 @@ class MAGNUM_EXPORT Shader { * @brief Load shader from source * @param type %Shader type * @param source %Shader source - * @return Pointer to compiled shader + * @return Shader instance * * Loads the shader from one source. Shorthand for * @code - * Shader* s = new Shader(type); - * s->addData(data); + * Shader s(type); + * s.addData(data); * @endcode - * Note that it is also possible to compile shader from more than one + * Note that it is also possible to create shader from more than one * source. */ - inline static Shader* fromData(Type type, const std::string& source) { - Shader* s = new Shader(type); - s->addSource(source); + inline static Shader fromData(Type type, const std::string& source) { + Shader s(type); + s.addSource(source); return s; } @@ -99,38 +97,41 @@ class MAGNUM_EXPORT Shader { * @brief Load shader from file * @param type %Shader type * @param filename %Source filename - * @return Pointer to compiled shader or zero, if file cannot be opened. + * @return Shader instance * * Loads the shader from from one file. Shorthand for * @code - * Shader* s = new Shader(type); - * if(!s->addFile(filename)) delete s; + * Shader s(type); + * s.addFile(filename); * @endcode - * Note that it is also possible to compile shader from more than one + * Note that it is also possible to create shader from more than one * source. */ - inline static Shader* fromFile(Type type, const char* filename) { - Shader* s = new Shader(type); - if(!s->addFile(filename)) { - delete s; - return nullptr; - } - + inline static Shader fromFile(Type type, const char* filename) { + Shader s(type); + s.addFile(filename); return s; } /** * @brief Constructor * - * Creates empty shader. Sources can be added with addData() or addFile(). + * Creates empty OpenGL shader. Sources can be added with addData() + * or addFile(). * @see fromData(), fromFile() */ inline Shader(Type type): _type(type), _state(Initialized), shader(0) {} + /** @brief Move constructor */ + Shader(Shader&& other); + + /** @brief Move assignment operator */ + Shader& operator=(Shader&& other); + /** * @brief Destructor * - * If the shader is compiled, deletes it. + * Deletes associated OpenGL shader. */ inline ~Shader() { if(shader) glDeleteShader(shader); } diff --git a/src/Shaders/PhongShader.cpp b/src/Shaders/PhongShader.cpp index 299f1bc4b..2877d8463 100644 --- a/src/Shaders/PhongShader.cpp +++ b/src/Shaders/PhongShader.cpp @@ -23,17 +23,13 @@ namespace Magnum { namespace Shaders { PhongShader::PhongShader() { Resource rs("shaders"); - Shader* vertexShader = Shader::fromData(Shader::Vertex, rs.get("PhongShader.vert")); - Shader* fragmentShader = Shader::fromData(Shader::Fragment, rs.get("PhongShader.frag")); - attachShader(vertexShader); - attachShader(fragmentShader); + attachShader(Shader::fromData(Shader::Vertex, rs.get("PhongShader.vert"))); + attachShader(Shader::fromData(Shader::Fragment, rs.get("PhongShader.frag"))); bindAttribute(Vertex::Location, "vertex"); bindAttribute(Normal::Location, "normal"); link(); - delete vertexShader; - delete fragmentShader; ambientColorUniform = uniformLocation("ambientColor"); diffuseColorUniform = uniformLocation("diffuseColor");