[core] Change CustomLayer to use a host interface instead of function pointers

Co-authored-by: Julian Rex <julian.rex@mapbox.com>

7 files changed, 45 insertions, 86 deletions

diff --git a/include/mbgl/style/layers/custom_layer.hpp b/include/mbgl/style/layers/custom_layer.hpp
index bf3387f95b..be656b0e14 100644
--- a/include/mbgl/style/layers/custom_layer.hpp
+++ b/include/mbgl/style/layers/custom_layer.hpp
@@ -6,15 +6,6 @@ namespace mbgl {
 namespace style {
 
 /**
- * Initialize any GL state needed by the custom layer. This method is called once, from the
- * main thread, at a point when the GL context is active but before rendering for the first
- * time.
- *
- * Resources that are acquired in this method must be released in the UninitializeFunction.
- */
-using CustomLayerInitializeFunction = void (*)(void* context);
-
-/**
  * Parameters that define the current camera position for a CustomLayerRenderFunction.
  */
 struct CustomLayerRenderParameters {
@@ -28,6 +19,18 @@ struct CustomLayerRenderParameters {
     double fieldOfView;
 };
 
+class CustomLayerHost {
+public:
+virtual ~CustomLayerHost() = default;
+/**
+ * Initialize any GL state needed by the custom layer. This method is called once, from the
+ * main thread, at a point when the GL context is active but before rendering for the first
+ * time.
+ *
+ * Resources that are acquired in this method must be released in the UninitializeFunction.
+ */
+virtual void initialize() = 0;
+
 /**
  * Render the layer. This method is called once per frame. The implementation should not make
  * any assumptions about the GL state (other than that the correct context is active). It may
@@ -36,7 +39,7 @@ struct CustomLayerRenderParameters {
  * Make sure that you are drawing your fragments with a z value of 1 to take advantage of the
  * opaque fragment culling in case there are opaque layers above your custom layer.
  */
-using CustomLayerRenderFunction = void (*)(void* context, const CustomLayerRenderParameters&);
+virtual void render(const CustomLayerRenderParameters&) = 0;
 
 /**
  * Called when the system has destroyed the underlying GL context. The
@@ -44,7 +47,7 @@ using CustomLayerRenderFunction = void (*)(void* context, const CustomLayerRende
  * `CustomLayerInitializeFunction` will be called instead to prepare for a new render.
  *
  */
-using CustomLayerContextLostFunction = void (*)(void* context);
+virtual void contextLost() = 0;
 
 /**
  * Destroy any GL state needed by the custom layer, and deallocate context, if necessary. This
@@ -52,22 +55,13 @@ using CustomLayerContextLostFunction = void (*)(void* context);
  *
  * Note that it may be called even when the InitializeFunction has not been called.
  */
-using CustomLayerDeinitializeFunction = void (*)(void* context);
+virtual void deinitialize() = 0;
+};
 
 class CustomLayer : public Layer {
 public:
     CustomLayer(const std::string& id,
-                CustomLayerInitializeFunction,
-                CustomLayerRenderFunction,
-                CustomLayerContextLostFunction,
-                CustomLayerDeinitializeFunction,
-                void* context);
-
-    CustomLayer(const std::string& id,
-                CustomLayerInitializeFunction,
-                CustomLayerRenderFunction,
-                CustomLayerDeinitializeFunction,
-                void* context);
+                std::unique_ptr<CustomLayerHost> host);
 
     ~CustomLayer() final;
 
diff --git a/src/mbgl/renderer/layers/render_custom_layer.cpp b/src/mbgl/renderer/layers/render_custom_layer.cpp
index a429b8d82e..cc9eede463 100644
--- a/src/mbgl/renderer/layers/render_custom_layer.cpp
+++ b/src/mbgl/renderer/layers/render_custom_layer.cpp
@@ -12,17 +12,16 @@ namespace mbgl {
 using namespace style;
 
 RenderCustomLayer::RenderCustomLayer(Immutable<style::CustomLayer::Impl> _impl)
-    : RenderLayer(LayerType::Custom, _impl) {
+    : RenderLayer(LayerType::Custom, _impl), host(_impl->host) {
+    host->initialize();
 }
 
 RenderCustomLayer::~RenderCustomLayer() {
     assert(BackendScope::exists());
-    if (initialized) {
-        if (contextDestroyed && impl().contextLostFn ) {
-            impl().contextLostFn(impl().context);
-        } else if (!contextDestroyed && impl().deinitializeFn) {
-            impl().deinitializeFn(impl().context);
-        }
+    if (contextDestroyed) {
+        host->contextLost();
+    } else if (!contextDestroyed) {
+        host->deinitialize();
     }
 }
 
@@ -44,15 +43,13 @@ std::unique_ptr<Bucket> RenderCustomLayer::createBucket(const BucketParameters&,
 }
 
 void RenderCustomLayer::render(PaintParameters& paintParameters, RenderSource*) {
-    if (context != impl().context || !initialized) {
+    if (host != impl().host) {
         //If the context changed, deinitialize the previous one before initializing the new one.
-        if (context && !contextDestroyed && impl().deinitializeFn) {
-            MBGL_CHECK_ERROR(impl().deinitializeFn(context));
+        if (host && !contextDestroyed) {
+            MBGL_CHECK_ERROR(host->deinitialize());
         }
-        context = impl().context;
-        assert(impl().initializeFn);
-        MBGL_CHECK_ERROR(impl().initializeFn(impl().context));
-        initialized = true;
+        host = impl().host;
+        MBGL_CHECK_ERROR(host->initialize());
     }
 
     gl::Context& glContext = paintParameters.context;
@@ -75,8 +72,7 @@ void RenderCustomLayer::render(PaintParameters& paintParameters, RenderSource*)
     parameters.pitch = state.getPitch();
     parameters.fieldOfView = state.getFieldOfView();
 
-    assert(impl().renderFn);
-    MBGL_CHECK_ERROR(impl().renderFn(context, parameters));
+    MBGL_CHECK_ERROR(host->render(parameters));
 
     // Reset the view back to our original one, just in case the CustomLayer changed
     // the viewport or Framebuffer.
diff --git a/src/mbgl/renderer/layers/render_custom_layer.hpp b/src/mbgl/renderer/layers/render_custom_layer.hpp
index 6d1fea99d3..971d8d8f42 100644
--- a/src/mbgl/renderer/layers/render_custom_layer.hpp
+++ b/src/mbgl/renderer/layers/render_custom_layer.hpp
@@ -24,9 +24,8 @@ public:
     };
 
 private:
-    bool initialized = false;
     bool contextDestroyed = false;
-    void * context = nullptr;
+    std::shared_ptr<style::CustomLayerHost> host;
 };
 
 template <>
diff --git a/src/mbgl/style/layers/custom_layer.cpp b/src/mbgl/style/layers/custom_layer.cpp
index 854c771847..0e51a70e50 100644
--- a/src/mbgl/style/layers/custom_layer.cpp
+++ b/src/mbgl/style/layers/custom_layer.cpp
@@ -6,20 +6,8 @@ namespace mbgl {
 namespace style {
 
 CustomLayer::CustomLayer(const std::string& layerID,
-                         CustomLayerInitializeFunction init,
-                         CustomLayerRenderFunction render,
-                         CustomLayerContextLostFunction contextLost,
-                         CustomLayerDeinitializeFunction deinit,
-                         void* context)
-        : Layer(makeMutable<Impl>(layerID, init, render, contextLost, deinit, context)) {
-}
-
-CustomLayer::CustomLayer(const std::string& layerID,
-                         CustomLayerInitializeFunction init,
-                         CustomLayerRenderFunction render,
-                         CustomLayerDeinitializeFunction deinit,
-                         void* context)
-    : Layer(makeMutable<Impl>(layerID, init, render, nullptr, deinit, context)) {
+                         std::unique_ptr<CustomLayerHost> host)
+    : Layer(makeMutable<Impl>(layerID, std::move(host))) {
 }
 
 CustomLayer::~CustomLayer() = default;
diff --git a/src/mbgl/style/layers/custom_layer_impl.cpp b/src/mbgl/style/layers/custom_layer_impl.cpp
index 1de268d2e2..05c41623c4 100644
--- a/src/mbgl/style/layers/custom_layer_impl.cpp
+++ b/src/mbgl/style/layers/custom_layer_impl.cpp
@@ -4,17 +4,9 @@ namespace mbgl {
 namespace style {
 
 CustomLayer::Impl::Impl(const std::string& id_,
-                         CustomLayerInitializeFunction initializeFn_,
-                         CustomLayerRenderFunction renderFn_,
-                         CustomLayerContextLostFunction contextLostFn_,
-                         CustomLayerDeinitializeFunction deinitializeFn_,
-                         void* context_)
+                        std::unique_ptr<CustomLayerHost> host_)
     : Layer::Impl(LayerType::Custom, id_, std::string()) {
-    initializeFn = initializeFn_;
-    renderFn = renderFn_;
-    deinitializeFn = deinitializeFn_;
-    contextLostFn = contextLostFn_;
-    context = context_;
+    host = std::move(host_);
 }
 
 bool CustomLayer::Impl::hasLayoutDifference(const Layer::Impl&) const {
diff --git a/src/mbgl/style/layers/custom_layer_impl.hpp b/src/mbgl/style/layers/custom_layer_impl.hpp
index 62efbbe15b..a41962c276 100644
--- a/src/mbgl/style/layers/custom_layer_impl.hpp
+++ b/src/mbgl/style/layers/custom_layer_impl.hpp
@@ -3,6 +3,8 @@
 #include <mbgl/style/layer_impl.hpp>
 #include <mbgl/style/layers/custom_layer.hpp>
 
+#include <memory>
+
 namespace mbgl {
 
 class TransformState;
@@ -12,20 +14,12 @@ namespace style {
 class CustomLayer::Impl : public Layer::Impl {
 public:
     Impl(const std::string& id,
-         CustomLayerInitializeFunction,
-         CustomLayerRenderFunction,
-         CustomLayerContextLostFunction,
-         CustomLayerDeinitializeFunction,
-         void* context);
+         std::unique_ptr<CustomLayerHost> host);
 
     bool hasLayoutDifference(const Layer::Impl&) const override;
     void stringifyLayout(rapidjson::Writer<rapidjson::StringBuffer>&) const override;
 
-    CustomLayerInitializeFunction initializeFn = nullptr;
-    CustomLayerRenderFunction renderFn = nullptr;
-    CustomLayerContextLostFunction contextLostFn = nullptr;
-    CustomLayerDeinitializeFunction deinitializeFn = nullptr;
-    void* context = nullptr;
+    std::shared_ptr<CustomLayerHost> host;
 };
 
 } // namespace style
diff --git a/test/api/custom_layer.test.cpp b/test/api/custom_layer.test.cpp
index eb1d7e0d3a..b95e4117ae 100644
--- a/test/api/custom_layer.test.cpp
+++ b/test/api/custom_layer.test.cpp
@@ -34,7 +34,7 @@ void main() {
 // layer implementation because it is intended to reflect how someone using custom layers
 // might actually write their own implementation.
 
-class TestLayer {
+class TestLayer : public mbgl::style::CustomLayerHost {
 public:
     ~TestLayer() {
         if (program) {
@@ -67,7 +67,7 @@ public:
         MBGL_CHECK_ERROR(glBufferData(GL_ARRAY_BUFFER, 6 * sizeof(GLfloat), triangle, GL_STATIC_DRAW));
     }
 
-    void render() {
+    void render(const mbgl::style::CustomLayerRenderParameters&) {
         MBGL_CHECK_ERROR(glUseProgram(program));
         MBGL_CHECK_ERROR(glBindBuffer(GL_ARRAY_BUFFER, buffer));
         MBGL_CHECK_ERROR(glEnableVertexAttribArray(a_pos));
@@ -75,6 +75,10 @@ public:
         MBGL_CHECK_ERROR(glDrawArrays(GL_TRIANGLE_STRIP, 0, 3));
     }
 
+    void contextLost() {}
+
+    void deinitialize() {}
+
     GLuint program = 0;
     GLuint vertexShader = 0;
     GLuint fragmentShader = 0;
@@ -95,15 +99,7 @@ TEST(CustomLayer, Basic) {
     map.setLatLngZoom({ 37.8, -122.5 }, 10);
     map.getStyle().addLayer(std::make_unique<CustomLayer>(
         "custom",
-        [] (void* context) {
-            reinterpret_cast<TestLayer*>(context)->initialize();
-        },
-        [] (void* context, const CustomLayerRenderParameters&) {
-            reinterpret_cast<TestLayer*>(context)->render();
-        },
-        [] (void* context) {
-            delete reinterpret_cast<TestLayer*>(context);
-        }, new TestLayer()));
+        std::make_unique<TestLayer>()));
 
     auto layer = std::make_unique<FillLayer>("landcover", "mapbox");
     layer->setSourceLayer("landcover");