[core] move GL value accessors to gl::value namespace

11 files changed, 161 insertions, 140 deletions

diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake
index a885e182a1..a809eb2a6f 100644
--- a/cmake/core-files.cmake
+++ b/cmake/core-files.cmake
@@ -78,14 +78,15 @@ set(MBGL_CORE_FILES
     # gl
     include/mbgl/gl/gl.hpp
     include/mbgl/gl/gl_helper.hpp
-    include/mbgl/gl/gl_values.hpp
     src/mbgl/gl/debugging.cpp
     src/mbgl/gl/debugging.hpp
     src/mbgl/gl/gl.cpp
     src/mbgl/gl/gl_config.hpp
-    src/mbgl/gl/gl_values.cpp
     src/mbgl/gl/object_store.cpp
     src/mbgl/gl/object_store.hpp
+    src/mbgl/gl/state.hpp
+    src/mbgl/gl/value.cpp
+    src/mbgl/gl/value.hpp
 
     # layout
     src/mbgl/layout/clip_lines.cpp
diff --git a/include/mbgl/gl/gl_helper.hpp b/include/mbgl/gl/gl_helper.hpp
deleted file mode 100644
index 9d46d018c3..0000000000
--- a/include/mbgl/gl/gl_helper.hpp
+++ /dev/null
@@ -1,20 +0,0 @@
-#pragma once
-
-namespace mbgl {
-namespace gl {
-
-template <typename T>
-class Preserve {
-public:
-    Preserve() : data(T::Get()) {
-    }
-    ~Preserve() {
-        T::Set(data);
-    }
-
-private:
-    const typename T::Type data;
-};
-
-} // namespace gl
-} // namespace mbgl
diff --git a/platform/default/glfw_view.cpp b/platform/default/glfw_view.cpp
index 6995c6e9bd..17b233a414 100644
--- a/platform/default/glfw_view.cpp
+++ b/platform/default/glfw_view.cpp
@@ -3,14 +3,15 @@
 #include <mbgl/sprite/sprite_image.hpp>
 #include <mbgl/style/transition_options.hpp>
 #include <mbgl/gl/gl.hpp>
-#include <mbgl/gl/gl_values.hpp>
-#include <mbgl/gl/gl_helper.hpp>
 #include <mbgl/platform/log.hpp>
 #include <mbgl/platform/platform.hpp>
 #include <mbgl/util/string.hpp>
 #include <mbgl/util/chrono.hpp>
 #include <mbgl/map/camera.hpp>
 
+#include <mbgl/gl/state.hpp>
+#include <mbgl/gl/value.hpp>
+
 #include <cassert>
 #include <cstdlib>
 
@@ -533,8 +534,8 @@ void showDebugImage(std::string name, const char *data, size_t width, size_t hei
     float scale = static_cast<float>(fbWidth) / static_cast<float>(width);
 
     {
-        gl::Preserve<gl::PixelZoom> pixelZoom;
-        gl::Preserve<gl::RasterPos> rasterPos;
+        gl::PreserveState<gl::value::PixelZoom> pixelZoom;
+        gl::PreserveState<gl::value::RasterPos> rasterPos;
 
         MBGL_CHECK_ERROR(glPixelZoom(scale, -scale));
         MBGL_CHECK_ERROR(glRasterPos2f(-1.0f, 1.0f));
@@ -570,11 +571,11 @@ void showColorDebugImage(std::string name, const char *data, size_t logicalWidth
     float yScale = static_cast<float>(fbHeight) / static_cast<float>(height);
 
     {
-        gl::Preserve<gl::ClearColor> clearColor;
-        gl::Preserve<gl::Blend> blend;
-        gl::Preserve<gl::BlendFunc> blendFunc;
-        gl::Preserve<gl::PixelZoom> pixelZoom;
-        gl::Preserve<gl::RasterPos> rasterPos;
+        gl::PreserveState<gl::value::ClearColor> clearColor;
+        gl::PreserveState<gl::value::Blend> blend;
+        gl::PreserveState<gl::value::BlendFunc> blendFunc;
+        gl::PreserveState<gl::value::PixelZoom> pixelZoom;
+        gl::PreserveState<gl::value::RasterPos> rasterPos;
 
         MBGL_CHECK_ERROR(glClearColor(0.8, 0.8, 0.8, 1));
         MBGL_CHECK_ERROR(glClear(GL_COLOR_BUFFER_BIT));
diff --git a/src/mbgl/gl/gl_config.hpp b/src/mbgl/gl/gl_config.hpp
index 225f4f8de6..fed79594e4 100644
--- a/src/mbgl/gl/gl_config.hpp
+++ b/src/mbgl/gl/gl_config.hpp
@@ -1,72 +1,14 @@
 #pragma once
 
-#include <cstdint>
-#include <tuple>
-#include <array>
-
-#include <mbgl/gl/gl_values.hpp>
+#include <mbgl/gl/state.hpp>
+#include <mbgl/gl/value.hpp>
 
 namespace mbgl {
 namespace gl {
 
-
-template <typename T, typename = void>
-struct DefaultValue {
-    static typename T::Type Get() {
-        return T::Get();
-    }
-};
-template <typename T>
-struct DefaultValue<T, decltype((void)T::Default, void())> {
-    static typename T::Type Get() {
-        return T::Default;
-    }
-};
-
-template <typename T>
-class Value {
-public:
-    void operator=(const typename T::Type& value) {
-        if (*this != value) {
-            dirty = false;
-            current = value;
-            T::Set(current);
-        }
-    }
-
-    bool operator!=(const typename T::Type& value) const {
-        return dirty || current != value;
-    }
-
-    void reset() {
-        *this = defaultValue;
-    }
-
-    void setDirty() {
-        dirty = true;
-    }
-
-    typename T::Type getCurrent() const {
-        return current;
-    }
-
-    bool getDirty() const {
-        return dirty;
-    }
-
-    void setDefaultValue(const typename T::Type& value) {
-        defaultValue = value;
-    }
-
-private:
-    typename T::Type defaultValue = DefaultValue<T>::Get();
-    typename T::Type current = defaultValue;
-    bool dirty = false;
-};
-
 class Config {
 public:
-    void reset() {
+    void resetState() {
         stencilFunc.reset();
         stencilMask.reset();
         stencilTest.reset();
@@ -91,12 +33,15 @@ public:
         pixelZoom.reset();
         rasterPos.reset();
 #endif // GL_ES_VERSION_2_0
+        for (auto& tex : texture) {
+            tex.reset();
+        }
         vertexBuffer.reset();
         elementBuffer.reset();
         vertexArrayObject.reset();
     }
 
-    void setDirty() {
+    void setDirtyState() {
         stencilFunc.setDirty();
         stencilMask.setDirty();
         stencilTest.setDirty();
@@ -121,39 +66,42 @@ public:
         pixelZoom.setDirty();
         rasterPos.setDirty();
 #endif // GL_ES_VERSION_2_0
+        for (auto& tex : texture) {
+            tex.setDirty();
+        }
         vertexBuffer.setDirty();
         elementBuffer.setDirty();
         vertexArrayObject.setDirty();
     }
 
-    Value<StencilFunc> stencilFunc;
-    Value<StencilMask> stencilMask;
-    Value<StencilTest> stencilTest;
-    Value<StencilOp> stencilOp;
-    Value<DepthRange> depthRange;
-    Value<DepthMask> depthMask;
-    Value<DepthTest> depthTest;
-    Value<DepthFunc> depthFunc;
-    Value<Blend> blend;
-    Value<BlendFunc> blendFunc;
-    Value<BlendColor> blendColor;
-    Value<ColorMask> colorMask;
-    Value<ClearDepth> clearDepth;
-    Value<ClearColor> clearColor;
-    Value<ClearStencil> clearStencil;
-    Value<Program> program;
-    Value<LineWidth> lineWidth;
-    Value<ActiveTexture> activeTexture;
-    Value<BindFramebuffer> bindFramebuffer;
-    Value<Viewport> viewport;
+    State<value::StencilFunc> stencilFunc;
+    State<value::StencilMask> stencilMask;
+    State<value::StencilTest> stencilTest;
+    State<value::StencilOp> stencilOp;
+    State<value::DepthRange> depthRange;
+    State<value::DepthMask> depthMask;
+    State<value::DepthTest> depthTest;
+    State<value::DepthFunc> depthFunc;
+    State<value::Blend> blend;
+    State<value::BlendFunc> blendFunc;
+    State<value::BlendColor> blendColor;
+    State<value::ColorMask> colorMask;
+    State<value::ClearDepth> clearDepth;
+    State<value::ClearColor> clearColor;
+    State<value::ClearStencil> clearStencil;
+    State<value::Program> program;
+    State<value::LineWidth> lineWidth;
+    State<value::ActiveTexture> activeTexture;
+    State<value::BindFramebuffer> bindFramebuffer;
+    State<value::Viewport> viewport;
 #ifndef GL_ES_VERSION_2_0
-    Value<PixelZoom> pixelZoom;
-    Value<RasterPos> rasterPos;
+    State<value::PixelZoom> pixelZoom;
+    State<value::RasterPos> rasterPos;
 #endif // GL_ES_VERSION_2_0
-    std::array<Value<BindTexture>, 2> texture;
-    Value<BindBuffer<GL_ARRAY_BUFFER>> vertexBuffer;
-    Value<BindBuffer<GL_ELEMENT_ARRAY_BUFFER>> elementBuffer;
-    Value<BindVAO> vertexArrayObject;
+    std::array<State<value::BindTexture>, 2> texture;
+    State<value::BindBuffer<GL_ARRAY_BUFFER>> vertexBuffer;
+    State<value::BindBuffer<GL_ELEMENT_ARRAY_BUFFER>> elementBuffer;
+    State<value::BindVAO> vertexArrayObject;
 };
 
 } // namespace gl
diff --git a/src/mbgl/gl/state.hpp b/src/mbgl/gl/state.hpp
new file mode 100644
index 0000000000..274ae5e2fe
--- /dev/null
+++ b/src/mbgl/gl/state.hpp
@@ -0,0 +1,90 @@
+#pragma once
+
+namespace mbgl {
+namespace gl {
+
+// Helper struct that allows obtaining the default value of a Value class
+template <typename T, typename = void>
+struct DefaultValue {
+    static typename T::Type Get() {
+        return T::Get();
+    }
+};
+template <typename T>
+struct DefaultValue<T, decltype((void)T::Default, void())> {
+    static typename T::Type Get() {
+        return T::Default;
+    }
+};
+
+// Wraps a piece of OpenGL state and remember its value to avoid redundant state calls.
+// Wrapped types need to implement to the Value class interface:
+//
+// class Value {
+//     using Type = ...;
+//     static const constexpr Type Default = ...;
+//     static void Set(const Type& value);
+//     static Type Get();
+// };
+//
+// The Get() function is optional, but if it is omitted, you must provide a Default.
+// Default is also optional, but if it is omitted, you must provide a Get() function.
+// If both are present, DefaultValue<T>::Get() will use the Default member.
+template <typename T>
+class State {
+public:
+    void operator=(const typename T::Type& value) {
+        if (*this != value) {
+            dirty = false;
+            currentValue = value;
+            T::Set(currentValue);
+        }
+    }
+
+    bool operator!=(const typename T::Type& value) const {
+        return dirty || currentValue != value;
+    }
+
+    void reset() {
+        *this = defaultValue;
+    }
+
+    void setDirty() {
+        dirty = true;
+    }
+
+    typename T::Type getCurrentValue() const {
+        return currentValue;
+    }
+
+    bool isDirty() const {
+        return dirty;
+    }
+
+    void setDefaultValue(const typename T::Type& value) {
+        defaultValue = value;
+    }
+
+private:
+    typename T::Type defaultValue = DefaultValue<T>::Get();
+    typename T::Type currentValue = defaultValue;
+    bool dirty = false;
+};
+
+// Helper struct that stores the current state and restores it upon destruction. You should not use
+// this code normally, except for debugging purposes.
+template <typename T>
+class PreserveState {
+public:
+    PreserveState() : value(T::Get()) {
+    }
+    ~PreserveState() {
+        T::Set(value);
+    }
+
+private:
+    const typename T::Type value;
+};
+
+} // namespace gl
+} // namespace mbgl
diff --git a/src/mbgl/gl/gl_values.cpp b/src/mbgl/gl/value.cpp
index c987c65aa0..7df3f4e3bd 100644
--- a/src/mbgl/gl/gl_values.cpp
+++ b/src/mbgl/gl/value.cpp
@@ -1,7 +1,8 @@
-#include <mbgl/gl/gl_values.hpp>
+#include <mbgl/gl/value.hpp>
 
 namespace mbgl {
 namespace gl {
+namespace value {
 
 const constexpr StencilFunc::Type StencilFunc::Default;
 const constexpr StencilMask::Type StencilMask::Default;
@@ -29,5 +30,6 @@ const constexpr PixelZoom::Type PixelZoom::Default;
 const constexpr RasterPos::Type RasterPos::Default;
 #endif // GL_ES_VERSION_2_0
 
+} // namespace value
 } // namespace gl
 } // namespace mbgl
diff --git a/include/mbgl/gl/gl_values.hpp b/src/mbgl/gl/value.hpp
index fa3b87c435..6436228ed1 100644
--- a/include/mbgl/gl/gl_values.hpp
+++ b/src/mbgl/gl/value.hpp
@@ -10,6 +10,7 @@
 
 namespace mbgl {
 namespace gl {
+namespace value {
 
 struct ClearDepth {
     using Type = GLfloat;
@@ -376,5 +377,6 @@ struct BindVAO {
 };
 
 
+} // namespace value
 } // namespace gl
 } // namespace mbgl
diff --git a/src/mbgl/renderer/painter.cpp b/src/mbgl/renderer/painter.cpp
index 438e4b5e47..2b3189cc55 100644
--- a/src/mbgl/renderer/painter.cpp
+++ b/src/mbgl/renderer/painter.cpp
@@ -48,8 +48,8 @@ Painter::Painter(const TransformState& state_,
 #endif
 
     // Reset GL values
-    config.setDirty();
-    config.reset();
+    config.setDirtyState();
+    config.resetState();
 }
 
 Painter::~Painter() = default;
@@ -225,7 +225,7 @@ void Painter::render(const Style& style, const FrameData& frame_, SpriteAtlas& a
     }
 
     if (frame.contextMode == GLContextMode::Shared) {
-        config.setDirty();
+        config.setDirtyState();
     }
 }
 
@@ -276,7 +276,7 @@ void Painter::renderPass(PaintParameters& parameters,
             config.stencilTest = GL_FALSE;
             setDepthSublayer(0);
             layer.as<CustomLayer>()->impl->render(state);
-            config.setDirty();
+            config.setDirtyState();
             config.bindFramebuffer.reset();
             config.viewport.reset();
         } else {
diff --git a/src/mbgl/renderer/painter_debug.cpp b/src/mbgl/renderer/painter_debug.cpp
index 8c15070cd5..adecc20518 100644
--- a/src/mbgl/renderer/painter_debug.cpp
+++ b/src/mbgl/renderer/painter_debug.cpp
@@ -6,8 +6,6 @@
 #include <mbgl/util/string.hpp>
 #include <mbgl/gl/debugging.hpp>
 #include <mbgl/gl/gl.hpp>
-#include <mbgl/gl/gl_values.hpp>
-#include <mbgl/gl/gl_helper.hpp>
 #include <mbgl/util/color.hpp>
 
 namespace mbgl {
diff --git a/test/gl/object.cpp b/test/gl/object.cpp
index 5733a15e4f..68cd2ddead 100644
--- a/test/gl/object.cpp
+++ b/test/gl/object.cpp
@@ -4,7 +4,6 @@
 #include <mbgl/platform/default/headless_view.hpp>
 
 #include <mbgl/gl/gl.hpp>
-#include <mbgl/gl/gl_helper.hpp>
 #include <mbgl/gl/gl_config.hpp>
 #include <mbgl/gl/object_store.hpp>
 
@@ -26,11 +25,11 @@ struct MockGLObject {
 
 const bool MockGLObject::Default = false;
 
-TEST(GLObject, Preserve) {
+TEST(GLObject, PreserveState) {
     getFlag = false;
     setFlag = false;
 
-    auto object = std::make_unique<mbgl::gl::Preserve<MockGLObject>>();
+    auto object = std::make_unique<mbgl::gl::PreserveState<MockGLObject>>();
     EXPECT_TRUE(getFlag);
     EXPECT_FALSE(setFlag);
 
@@ -43,28 +42,28 @@ TEST(GLObject, Preserve) {
 TEST(GLObject, Value) {
     setFlag = false;
 
-    auto object = std::make_unique<mbgl::gl::Value<MockGLObject>>();
-    EXPECT_EQ(object->getCurrent(), false);
-    EXPECT_FALSE(object->getDirty());
+    auto object = std::make_unique<mbgl::gl::State<MockGLObject>>();
+    EXPECT_EQ(object->getCurrentValue(), false);
+    EXPECT_FALSE(object->isDirty());
     EXPECT_FALSE(setFlag);
 
     object->setDirty();
-    EXPECT_TRUE(object->getDirty());
+    EXPECT_TRUE(object->isDirty());
 
     *object = false;
-    EXPECT_EQ(object->getCurrent(), false);
-    EXPECT_FALSE(object->getDirty());
+    EXPECT_EQ(object->getCurrentValue(), false);
+    EXPECT_FALSE(object->isDirty());
     EXPECT_TRUE(setFlag);
 
     setFlag = false;
     *object = true;
-    EXPECT_EQ(object->getCurrent(), true);
-    EXPECT_FALSE(object->getDirty());
+    EXPECT_EQ(object->getCurrentValue(), true);
+    EXPECT_FALSE(object->isDirty());
     EXPECT_TRUE(setFlag);
 
     object->reset();
-    EXPECT_EQ(object->getCurrent(), false);
-    EXPECT_FALSE(object->getDirty());
+    EXPECT_EQ(object->getCurrentValue(), false);
+    EXPECT_FALSE(object->isDirty());
     EXPECT_TRUE(setFlag);
 }
 
diff --git a/test/util/offscreen_texture.cpp b/test/util/offscreen_texture.cpp
index 74a616134e..b4d98078b3 100644
--- a/test/util/offscreen_texture.cpp
+++ b/test/util/offscreen_texture.cpp
@@ -130,7 +130,7 @@ void main() {
         test::checkImage("test/fixtures/offscreen_texture/render-to-texture", image, 0, 0);
 
         // Now reset the FBO back to normal and retrieve the original (restored) framebuffer.
-        config.reset();
+        config.resetState();
 
         image = view.readStillImage();
         test::checkImage("test/fixtures/offscreen_texture/render-to-fbo", image, 0, 0);