[gl] Added mbgl::gl::ShaderHolder

4 files changed, 53 insertions, 42 deletions

diff --git a/src/mbgl/gl/gl_object_store.cpp b/src/mbgl/gl/gl_object_store.cpp
index 52cf03d04b..7a6037d505 100644
--- a/src/mbgl/gl/gl_object_store.cpp
+++ b/src/mbgl/gl/gl_object_store.cpp
@@ -22,6 +22,20 @@ void ProgramHolder::reset() {
     id = 0;
 }
 
+void ShaderHolder::create() {
+    if (id) return;
+    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
+    id = MBGL_CHECK_ERROR(glCreateShader(type));
+}
+
+void ShaderHolder::reset() {
+    if (!id) return;
+    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
+    MBGL_CHECK_ERROR(glDeleteShader(id));
+    id = 0;
+}
+
+
 void GLObjectStore::abandonVAO(GLuint vao) {
     assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
     abandonedVAOs.emplace_back(vao);
diff --git a/src/mbgl/gl/gl_object_store.hpp b/src/mbgl/gl/gl_object_store.hpp
index be25728660..6c7f3c92f2 100644
--- a/src/mbgl/gl/gl_object_store.hpp
+++ b/src/mbgl/gl/gl_object_store.hpp
@@ -36,6 +36,21 @@ public:
     void reset();
 };
 
+class ShaderHolder : public GLHolder {
+public:
+    ShaderHolder(GLenum type_) : type(type_) {}
+    ~ShaderHolder() { reset(); }
+
+    ShaderHolder(ShaderHolder&& o) noexcept : GLHolder(std::move(o)), type(o.type) {}
+    ShaderHolder& operator=(ShaderHolder&& o) noexcept { GLHolder::operator=(std::move(o)); type = o.type; return *this; }
+
+    void create();
+    void reset();
+
+private:
+    GLenum type = 0;
+};
+
 class GLObjectStore : private util::noncopyable {
 public:
     // Mark OpenGL objects for deletion
diff --git a/src/mbgl/shader/shader.cpp b/src/mbgl/shader/shader.cpp
index c0757b749a..998edfb970 100644
--- a/src/mbgl/shader/shader.cpp
+++ b/src/mbgl/shader/shader.cpp
@@ -11,7 +11,7 @@
 #include <fstream>
 #include <cstdio>
 
-using namespace mbgl;
+namespace mbgl {
 
 Shader::Shader(const char *name_, const GLchar *vertSource, const GLchar *fragSource)
     : name(name_)
@@ -19,24 +19,21 @@ Shader::Shader(const char *name_, const GLchar *vertSource, const GLchar *fragSo
     util::stopwatch stopwatch("shader compilation", Event::Shader);
 
     program.create();
-
-    if (!compileShader(&vertShader, GL_VERTEX_SHADER, &vertSource)) {
+    vertexShader.create();
+    if (!compileShader(vertexShader, &vertSource)) {
         Log::Error(Event::Shader, "Vertex shader %s failed to compile: %s", name, vertSource);
-        program.reset();
         throw util::ShaderException(std::string { "Vertex shader " } + name + " failed to compile");
     }
 
-    if (!compileShader(&fragShader, GL_FRAGMENT_SHADER, &fragSource)) {
+    fragmentShader.create();
+    if (!compileShader(fragmentShader, &fragSource)) {
         Log::Error(Event::Shader, "Fragment shader %s failed to compile: %s", name, fragSource);
-        MBGL_CHECK_ERROR(glDeleteShader(vertShader));
-        vertShader = 0;
-        program.reset();
         throw util::ShaderException(std::string { "Fragment shader " } + name + " failed to compile");
     }
 
     // Attach shaders
-    MBGL_CHECK_ERROR(glAttachShader(program.getID(), vertexShader));
-    MBGL_CHECK_ERROR(glAttachShader(program.getID(), fragmentShader));
+    MBGL_CHECK_ERROR(glAttachShader(program.getID(), vertexShader.getID()));
+    MBGL_CHECK_ERROR(glAttachShader(program.getID(), fragmentShader.getID()));
 
     {
         // Link program
@@ -52,49 +49,36 @@ Shader::Shader(const char *name_, const GLchar *vertSource, const GLchar *fragSo
                 MBGL_CHECK_ERROR(glGetProgramInfoLog(program.getID(), logLength, &logLength, log.get()));
                 Log::Error(Event::Shader, "Program failed to link: %s", log.get());
             }
-
-            MBGL_CHECK_ERROR(glDeleteShader(vertShader));
-            vertShader = 0;
-            MBGL_CHECK_ERROR(glDeleteShader(fragShader));
-            fragShader = 0;
-            program.reset();
-        throw util::ShaderException(std::string { "Program " } + name + " failed to link: " + log.get());
+            throw util::ShaderException(std::string { "Program " } + name + " failed to link: " + log.get());
         }
     }
 
     a_pos = MBGL_CHECK_ERROR(glGetAttribLocation(program.getID(), "a_pos"));
 }
 
+bool Shader::compileShader(gl::ShaderHolder& shader, const GLchar *source[]) {
+    GLint status = 0;
 
-bool Shader::compileShader(GLuint *shader, GLenum type, const GLchar *source[]) {
-    GLint status;
-
-    *shader = MBGL_CHECK_ERROR(glCreateShader(type));
     const GLsizei lengths = static_cast<GLsizei>(std::strlen(*source));
-    MBGL_CHECK_ERROR(glShaderSource(*shader, 1, source, &lengths));
+    MBGL_CHECK_ERROR(glShaderSource(shader.getID(), 1, source, &lengths));
 
-    MBGL_CHECK_ERROR(glCompileShader(*shader));
+    MBGL_CHECK_ERROR(glCompileShader(shader.getID()));
 
-    MBGL_CHECK_ERROR(glGetShaderiv(*shader, GL_COMPILE_STATUS, &status));
+    MBGL_CHECK_ERROR(glGetShaderiv(shader.getID(), GL_COMPILE_STATUS, &status));
     if (status == 0) {
         GLint logLength;
-        MBGL_CHECK_ERROR(glGetShaderiv(*shader, GL_INFO_LOG_LENGTH, &logLength));
+        MBGL_CHECK_ERROR(glGetShaderiv(shader.getID(), GL_INFO_LOG_LENGTH, &logLength));
         if (logLength > 0) {
             const auto log = std::make_unique<GLchar[]>(logLength);
-            MBGL_CHECK_ERROR(glGetShaderInfoLog(*shader, logLength, &logLength, log.get()));
+            MBGL_CHECK_ERROR(glGetShaderInfoLog(shader.getID(), logLength, &logLength, log.get()));
             Log::Error(Event::Shader, "Shader failed to compile: %s", log.get());
         }
-
-        MBGL_CHECK_ERROR(glDeleteShader(*shader));
-        *shader = 0;
         return false;
     }
 
-    MBGL_CHECK_ERROR(glGetShaderiv(*shader, GL_COMPILE_STATUS, &status));
+    MBGL_CHECK_ERROR(glGetShaderiv(shader.getID(), GL_COMPILE_STATUS, &status));
     if (status == GL_FALSE) {
         Log::Error(Event::Shader, "Shader %s failed to compile.", name);
-        MBGL_CHECK_ERROR(glDeleteShader(*shader));
-        *shader = 0;
         return false;
     }
 
@@ -103,11 +87,9 @@ bool Shader::compileShader(GLuint *shader, GLenum type, const GLchar *source[])
 
 Shader::~Shader() {
     if (program) {
-        MBGL_CHECK_ERROR(glDetachShader(program.getID(), vertexShader));
-        MBGL_CHECK_ERROR(glDetachShader(program.getID(), fragmentShader));
-        MBGL_CHECK_ERROR(glDeleteShader(vertShader));
-        vertShader = 0;
-        MBGL_CHECK_ERROR(glDeleteShader(fragShader));
-        fragShader = 0;
+        MBGL_CHECK_ERROR(glDetachShader(program.getID(), vertexShader.getID()));
+        MBGL_CHECK_ERROR(glDetachShader(program.getID(), fragmentShader.getID()));
     }
 }
+
+} // namespace mbgl
diff --git a/src/mbgl/shader/shader.hpp b/src/mbgl/shader/shader.hpp
index b3d694a04f..c63aa6714c 100644
--- a/src/mbgl/shader/shader.hpp
+++ b/src/mbgl/shader/shader.hpp
@@ -14,7 +14,7 @@ public:
     ~Shader();
     const GLchar *name;
 
-    inline const GLuint& getID() const {
+    GLuint getID() const {
         return program.getID();
     }
 
@@ -24,11 +24,11 @@ protected:
     GLint a_pos = -1;
 
 private:
-    bool compileShader(GLuint *shader, GLenum type, const GLchar *source[]);
+    bool compileShader(gl::ShaderHolder&, const GLchar *source[]);
 
     gl::ProgramHolder program;
-    GLuint vertShader = 0;
-    GLuint fragShader = 0;
+    gl::ShaderHolder vertexShader = { GL_VERTEX_SHADER };
+    gl::ShaderHolder fragmentShader = { GL_FRAGMENT_SHADER };
 };
 
 } // namespace mbgl