From fd4ada4fc1999f920816b28f9f1d80d53d01a337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 29 Apr 2013 20:23:28 +0200 Subject: [PATCH] Method chaining in Shader. The methods return reference instead of pointer, as the class is commonly created on the stack. Removed static functions Shader::fromFile() and Shader::fromData(), as they are not needed now. Also asserting that the file exists and is readable in addFile(). --- doc/method-chaining.dox | 5 +++ src/AbstractShaderProgram.h | 4 +-- src/Shader.cpp | 11 ++---- src/Shader.h | 53 ++++------------------------- src/Shaders/DistanceFieldVector.cpp | 16 ++++----- src/Shaders/Flat.cpp | 16 ++++----- src/Shaders/Phong.cpp | 16 ++++----- src/Shaders/Vector.cpp | 16 ++++----- src/Shaders/VertexColor.cpp | 16 ++++----- 9 files changed, 52 insertions(+), 101 deletions(-) diff --git a/doc/method-chaining.dox b/doc/method-chaining.dox index 798830984..df77cfd54 100644 --- a/doc/method-chaining.dox +++ b/doc/method-chaining.dox @@ -86,5 +86,10 @@ Scene3D scene; ->rotateX(90.0_degf) ->translate({-1.5f, 0.5f, 7.0f}); @endcode + +In most cases method chaining methods return pointer to the object, because +most of the objects are commonly created on the heap. The only exception are +Shader methods, which return reference, because the class is commonly created +as local variable in shader constructors. */ } diff --git a/src/AbstractShaderProgram.h b/src/AbstractShaderProgram.h index bae79bb8b..72136ea37 100644 --- a/src/AbstractShaderProgram.h +++ b/src/AbstractShaderProgram.h @@ -85,8 +85,8 @@ Int TransformationUniform = 0, @code MyShader() { // Load shaders from file and attach them to the program - attachShader(Shader::fromFile(Version::430, Shader::Type::Vertex, "PhongShader.vert")); - attachShader(Shader::fromFile(Version::430, Shader::Type::Fragment, "PhongShader.frag")); + attachShader(Shader(Version::GL430, Shader::Type::Vertex).attachFile("PhongShader.vert")); + attachShader(Shader(Version::GL430, Shader::Type::Fragment).attachFile("PhongShader.frag")); // Link link(); diff --git a/src/Shader.cpp b/src/Shader.cpp index 64e3679e5..72777fbae 100644 --- a/src/Shader.cpp +++ b/src/Shader.cpp @@ -79,15 +79,10 @@ Shader& Shader::operator=(Shader&& other) { return *this; } -bool Shader::addFile(const std::string& filename) { +Shader& Shader::addFile(const std::string& filename) { /* Open file */ std::ifstream file(filename.c_str()); - if(!file.good()) { - file.close(); - - Error() << "Shader file " << '\'' + filename + '\'' << " cannot be opened."; - return false; - } + CORRADE_ASSERT(file.good(), "Shader file " << '\'' + filename + '\'' << " cannot be opened.", *this); /* Get size of shader and initialize buffer */ file.seekg(0, std::ios::end); @@ -104,7 +99,7 @@ bool Shader::addFile(const std::string& filename) { addSource(source); delete[] source; - return true; + return *this; } GLuint Shader::compile() { diff --git a/src/Shader.h b/src/Shader.h index 944ca2ff2..2b5e9936c 100644 --- a/src/Shader.h +++ b/src/Shader.h @@ -93,48 +93,6 @@ class MAGNUM_EXPORT Shader { Failed /**< Compilation failed */ }; - /** - * @brief Load shader from source - * @param version Target version - * @param type %Shader type - * @param source %Shader source - * @return Shader instance - * - * Loads the shader from one source. Shorthand for - * @code - * Shader s(version, type); - * s.addData(data); - * @endcode - * Note that it is also possible to create shader from more than one - * source. - */ - inline static Shader fromData(Version version, Type type, const std::string& source) { - Shader s(version, type); - s.addSource(source); - return s; - } - - /** - * @brief Load shader from file - * @param version Target version - * @param type %Shader type - * @param filename Source filename - * @return Shader instance - * - * Loads the shader from from one file. Shorthand for - * @code - * Shader s(version, type); - * s.addFile(filename); - * @endcode - * Note that it is also possible to create shader from more than one - * source. - */ - inline static Shader fromFile(Version version, Type type, const char* filename) { - Shader s(version, type); - s.addFile(filename); - return s; - } - /** * @brief Constructor * @param version Target version @@ -142,7 +100,7 @@ class MAGNUM_EXPORT Shader { * * Creates empty OpenGL shader and adds @c \#version directive at the * beginning. Sources can be added with addSource() or addFile(). - * @see fromData(), fromFile(), @fn_gl{CreateShader} + * @see @fn_gl{CreateShader} */ explicit Shader(Version version, Type type); @@ -177,24 +135,27 @@ class MAGNUM_EXPORT Shader { /** * @brief Add shader source * @param source String with shader source + * @return Reference to self (for method chaining) * * If the shader is not compiled already, adds given source to source * list. Note that it is possible to compile shader from more than * one source. * @see addFile() */ - inline void addSource(const std::string& source) { + inline Shader& addSource(const std::string& source) { if(_state == State::Initialized) sources.push_back(source); + return *this; } /** * @brief Add source file * @param filename Name of source file to read from - * @return False if reading the file fails, true otherwise. + * @return Reference to self (for method chaining) * + * The file must exist and must be readable. * @see addSource() */ - bool addFile(const std::string& filename); + Shader& addFile(const std::string& filename); /** * @brief Compile shader diff --git a/src/Shaders/DistanceFieldVector.cpp b/src/Shaders/DistanceFieldVector.cpp index 94ab352dc..09b140cea 100644 --- a/src/Shaders/DistanceFieldVector.cpp +++ b/src/Shaders/DistanceFieldVector.cpp @@ -47,15 +47,13 @@ template DistanceFieldVector::DistanceFieldV Version v = Context::current()->supportedVersion({Version::GLES300, Version::GLES200}); #endif - Shader vertexShader(v, Shader::Type::Vertex); - vertexShader.addSource(rs.get("compatibility.glsl")); - vertexShader.addSource(rs.get(vertexShaderName())); - AbstractShaderProgram::attachShader(vertexShader); - - Shader fragmentShader(v, Shader::Type::Fragment); - fragmentShader.addSource(rs.get("compatibility.glsl")); - fragmentShader.addSource(rs.get("DistanceFieldVector.frag")); - AbstractShaderProgram::attachShader(fragmentShader); + AbstractShaderProgram::attachShader(Shader(v, Shader::Type::Vertex) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get(vertexShaderName()))); + + AbstractShaderProgram::attachShader(Shader(v, Shader::Type::Fragment) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get("DistanceFieldVector.frag"))); #ifndef MAGNUM_TARGET_GLES if(!Context::current()->isExtensionSupported() || diff --git a/src/Shaders/Flat.cpp b/src/Shaders/Flat.cpp index 1eebdd955..69d1371c2 100644 --- a/src/Shaders/Flat.cpp +++ b/src/Shaders/Flat.cpp @@ -46,15 +46,13 @@ template Flat::Flat(): transformationProject Version v = Context::current()->supportedVersion({Version::GLES300, Version::GLES200}); #endif - Shader vertexShader(v, Shader::Type::Vertex); - vertexShader.addSource(rs.get("compatibility.glsl")); - vertexShader.addSource(rs.get(vertexShaderName())); - attachShader(vertexShader); - - Shader fragmentShader(v, Shader::Type::Fragment); - fragmentShader.addSource(rs.get("compatibility.glsl")); - fragmentShader.addSource(rs.get("Flat.frag")); - attachShader(fragmentShader); + attachShader(Shader(v, Shader::Type::Vertex) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get(vertexShaderName()))); + + attachShader(Shader(v, Shader::Type::Fragment) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get("Flat.frag"))); #ifndef MAGNUM_TARGET_GLES if(!Context::current()->isExtensionSupported() || diff --git a/src/Shaders/Phong.cpp b/src/Shaders/Phong.cpp index 5a1c8c24e..b57723910 100644 --- a/src/Shaders/Phong.cpp +++ b/src/Shaders/Phong.cpp @@ -40,15 +40,13 @@ Phong::Phong(): transformationMatrixUniform(0), projectionMatrixUniform(1), norm Version v = Context::current()->supportedVersion({Version::GLES300, Version::GLES200}); #endif - Shader vertexShader(v, Shader::Type::Vertex); - vertexShader.addSource(rs.get("compatibility.glsl")); - vertexShader.addSource(rs.get("Phong.vert")); - attachShader(vertexShader); - - Shader fragmentShader(v, Shader::Type::Fragment); - fragmentShader.addSource(rs.get("compatibility.glsl")); - fragmentShader.addSource(rs.get("Phong.frag")); - attachShader(fragmentShader); + attachShader(Shader(v, Shader::Type::Vertex) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get("Phong.vert"))); + + attachShader(Shader(v, Shader::Type::Fragment) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get("Phong.frag"))); #ifndef MAGNUM_TARGET_GLES if(!Context::current()->isExtensionSupported() || diff --git a/src/Shaders/Vector.cpp b/src/Shaders/Vector.cpp index 461015fe4..48b3463cc 100644 --- a/src/Shaders/Vector.cpp +++ b/src/Shaders/Vector.cpp @@ -47,15 +47,13 @@ template Vector::Vector(): transformationPro Version v = Context::current()->supportedVersion({Version::GLES300, Version::GLES200}); #endif - Shader vertexShader(v, Shader::Type::Vertex); - vertexShader.addSource(rs.get("compatibility.glsl")); - vertexShader.addSource(rs.get(vertexShaderName())); - AbstractShaderProgram::attachShader(vertexShader); - - Shader fragmentShader(v, Shader::Type::Fragment); - fragmentShader.addSource(rs.get("compatibility.glsl")); - fragmentShader.addSource(rs.get("Vector.frag")); - AbstractShaderProgram::attachShader(fragmentShader); + AbstractShaderProgram::attachShader(Shader(v, Shader::Type::Vertex) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get(vertexShaderName()))); + + AbstractShaderProgram::attachShader(Shader(v, Shader::Type::Fragment) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get("Vector.frag"))); #ifndef MAGNUM_TARGET_GLES if(!Context::current()->isExtensionSupported() || diff --git a/src/Shaders/VertexColor.cpp b/src/Shaders/VertexColor.cpp index 444932bd5..910285f3d 100644 --- a/src/Shaders/VertexColor.cpp +++ b/src/Shaders/VertexColor.cpp @@ -46,15 +46,13 @@ template VertexColor::VertexColor(): transfo Version v = Context::current()->supportedVersion({Version::GLES300, Version::GLES200}); #endif - Shader vertexShader(v, Shader::Type::Vertex); - vertexShader.addSource(rs.get("compatibility.glsl")); - vertexShader.addSource(rs.get(vertexShaderName())); - attachShader(vertexShader); - - Shader fragmentShader(v, Shader::Type::Fragment); - fragmentShader.addSource(rs.get("compatibility.glsl")); - fragmentShader.addSource(rs.get("VertexColor.frag")); - attachShader(fragmentShader); + attachShader(Shader(v, Shader::Type::Vertex) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get(vertexShaderName()))); + + attachShader(Shader(v, Shader::Type::Fragment) + .addSource(rs.get("compatibility.glsl")) + .addSource(rs.get("VertexColor.frag"))); #ifndef MAGNUM_TARGET_GLES if(!Context::current()->isExtensionSupported() ||