[core] Generalize Map::{add,remove}CustomLayer

include/mbgl/layer/custom_layer.hpp

@@ -4,6 +4,45 @@
 
 namespace mbgl {
 
+/**
+ * Initialize any GL state needed by the custom layer. This method is called once, from the
+ * rendering 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 {
+    double width;
+    double height;
+    double latitude;
+    double longitude;
+    double zoom;
+    double bearing;
+    double pitch;
+    double altitude;
+};
+
+/**
+ * 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
+ * make changes to the state, and is not required to reset values such as the depth mask, stencil
+ * mask, and corresponding test flags to their original values.
+ */
+using CustomLayerRenderFunction = void (*)(void* context, const CustomLayerRenderParameters&);
+
+/**
+ * Destroy any GL state needed by the custom layer, and deallocate context, if necessary. This
+ * method is called once, from the rendering thread, at a point when the GL context is active.
+ *
+ * Note that it may be called even when the InitializeFunction has not been called.
+ */
+using CustomLayerDeinitializeFunction = void (*)(void* context);
+
 class CustomLayer : public Layer {
 public:
     CustomLayer(const std::string& id,

include/mbgl/map/map.hpp

@@ -9,7 +9,6 @@
 #include <mbgl/util/feature.hpp>
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/annotation/annotation.hpp>
-#include <mbgl/style/types.hpp>
 #include <mbgl/style/property_transition.hpp>
 
 #include <cstdint>
@@ -23,6 +22,7 @@ namespace mbgl {
 class FileSource;
 class View;
 class SpriteImage;
+class Layer;
 struct CameraOptions;
 struct AnimationOptions;
 
@@ -146,13 +146,8 @@ class Map : private util::noncopyable {
 
     AnnotationIDs getPointAnnotationsInBounds(const LatLngBounds&);
 
-    void addCustomLayer(const std::string& id,
-                        CustomLayerInitializeFunction,
-                        CustomLayerRenderFunction,
-                        CustomLayerDeinitializeFunction,
-                        void* context,
-                        const char* before = nullptr);
-    void removeCustomLayer(const std::string& id);
+    void addLayer(std::unique_ptr<Layer>, const optional<std::string>& beforeLayerID = {});
+    void removeLayer(const std::string& layerID);
 
     // Feature queries
     std::vector<Feature> queryRenderedFeatures(const ScreenCoordinate&, const optional<std::vector<std::string>>& layerIDs = {});

include/mbgl/map/view.hpp

@@ -36,8 +36,8 @@ class View {
     // as a matched pair, in four situations:
     //
     //   1. When releasing GL resources during Map destruction
-    //   2. When calling a CustomLayerInitializeFunction, during Map::addCustomLayer
-    //   3. When calling a CustomLayerDeinitializeFunction, during Map::removeCustomLayer
+    //   2. When calling a CustomLayerInitializeFunction, during Map::addLayer
+    //   3. When calling a CustomLayerDeinitializeFunction, during Map::removeLayer
     //   4. When rendering for Map::renderStill
     //
     // They are *not* called for Map::render; it is assumed that the correct context is already

include/mbgl/style/types.hpp

@@ -125,44 +125,5 @@ enum class TextTransformType : uint8_t {
     Lowercase,
 };
 
-/**
- * Initialize any GL state needed by the custom layer. This method is called once, from the
- * rendering 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 {
-    double width;
-    double height;
-    double latitude;
-    double longitude;
-    double zoom;
-    double bearing;
-    double pitch;
-    double altitude;
-};
-
-/**
- * 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
- * make changes to the state, and is not required to reset values such as the depth mask, stencil
- * mask, and corresponding test flags to their original values.
- */
-using CustomLayerRenderFunction = void (*)(void* context, const CustomLayerRenderParameters&);
-
-/**
- * Destroy any GL state needed by the custom layer, and deallocate context, if necessary. This
- * method is called once, from the rendering thread, at a point when the GL context is active.
- *
- * Note that it may be called even when the InitializeFunction has not been called.
- */
-using CustomLayerDeinitializeFunction = void (*)(void* context);
-
 } // namespace mbgl
 

platform/android/src/example_custom_layer.cpp

@@ -1,7 +1,7 @@
 #include <jni.h>
 #include <GLES2/gl2.h>
 
-#include <mbgl/style/types.hpp>
+#include <mbgl/layer/custom_layer.hpp>
 
 static const GLchar * vertexShaderSource = "attribute vec2 a_pos; void main() { gl_Position = vec4(a_pos, 0, 1); }";
 static const GLchar * fragmentShaderSource = "void main() { gl_FragColor = vec4(0, 1, 0, 1); }";

platform/android/src/jni.cpp

@@ -14,6 +14,7 @@
 #include <mbgl/map/map.hpp>
 #include <mbgl/map/camera.hpp>
 #include <mbgl/annotation/annotation.hpp>
+#include <mbgl/layer/custom_layer.hpp>
 #include <mbgl/sprite/sprite_image.hpp>
 #include <mbgl/platform/event.hpp>
 #include <mbgl/platform/log.hpp>
@@ -1111,20 +1112,20 @@ void nativeAddCustomLayer(JNIEnv *env, jni::jobject* obj, jlong nativeMapViewPtr
     mbgl::Log::Debug(mbgl::Event::JNI, "nativeAddCustomLayer");
     assert(nativeMapViewPtr != 0);
     NativeMapView *nativeMapView = reinterpret_cast<NativeMapView *>(nativeMapViewPtr);
-    nativeMapView->getMap().addCustomLayer(
+    nativeMapView->getMap().addLayer(std::make_unique<mbgl::CustomLayer>(
         std_string_from_jstring(env, reinterpret_cast<jni::jstring*>(jni::GetField<jni::jobject*>(*env, customLayer, *customLayerIdId))),
         reinterpret_cast<mbgl::CustomLayerInitializeFunction>(jni::GetField<jlong>(*env, customLayer, *customLayerInitializeFunctionId)),
         reinterpret_cast<mbgl::CustomLayerRenderFunction>(jni::GetField<jlong>(*env, customLayer, *customLayerRenderFunctionId)),
         reinterpret_cast<mbgl::CustomLayerDeinitializeFunction>(jni::GetField<jlong>(*env, customLayer, *customLayerDeinitializeFunctionId)),
-        reinterpret_cast<void*>(jni::GetField<jlong>(*env, customLayer, *customLayerContextId)),
-        before ? std_string_from_jstring(env, before).c_str() : nullptr);
+        reinterpret_cast<void*>(jni::GetField<jlong>(*env, customLayer, *customLayerContextId))),
+        before ? mbgl::optional<std::string>(std_string_from_jstring(env, before)) : mbgl::optional<std::string>());
 }
 
 void nativeRemoveCustomLayer(JNIEnv *env, jni::jobject* obj, jlong nativeMapViewPtr, jni::jstring* id) {
     mbgl::Log::Debug(mbgl::Event::JNI, "nativeRemoveCustomLayer");
     assert(nativeMapViewPtr != 0);
     NativeMapView *nativeMapView = reinterpret_cast<NativeMapView *>(nativeMapViewPtr);
-    nativeMapView->getMap().removeCustomLayer(std_string_from_jstring(env, id));
+    nativeMapView->getMap().removeLayer(std_string_from_jstring(env, id));
 }
 
 // Offline calls begin

platform/ios/src/MGLMapView.mm

@@ -17,6 +17,7 @@
 #include <mbgl/storage/network_status.hpp>
 #include <mbgl/style/property_transition.hpp>
 #include <mbgl/math/wrap.hpp>
+#include <mbgl/layer/custom_layer.hpp>
 #include <mbgl/util/geo.hpp>
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/image.hpp>
@@ -5004,14 +5005,14 @@ - (void)insertCustomStyleLayerWithIdentifier:(NSString *)identifier preparationH
 {
     NSAssert(identifier, @"Style layer needs an identifier");
     MGLCustomStyleLayerHandlers *context = new MGLCustomStyleLayerHandlers(preparation, drawing, completion);
-    _mbglMap->addCustomLayer(identifier.UTF8String, MGLPrepareCustomStyleLayer,
-                             MGLDrawCustomStyleLayer, MGLFinishCustomStyleLayer,
-                             context, otherIdentifier.UTF8String);
+    _mbglMap->addLayer(std::make_unique<mbgl::CustomLayer>(identifier.UTF8String, MGLPrepareCustomStyleLayer,
+                                                           MGLDrawCustomStyleLayer, MGLFinishCustomStyleLayer, context),
+                       otherIdentifier ? mbgl::optional<std::string>(otherIdentifier.UTF8String) : mbgl::optional<std::string>());
 }
 
 - (void)removeCustomStyleLayerWithIdentifier:(NSString *)identifier
 {
-    _mbglMap->removeCustomLayer(identifier.UTF8String);
+    _mbglMap->removeLayer(identifier.UTF8String);
 }
 
 - (void)setCustomStyleLayersNeedDisplay

platform/qt/src/qmapboxgl.cpp

@@ -4,6 +4,7 @@
 #include <mbgl/gl/gl.hpp>
 #include <mbgl/map/camera.hpp>
 #include <mbgl/map/map.hpp>
+#include <mbgl/layer/custom_layer.hpp>
 #include <mbgl/sprite/sprite_image.hpp>
 #include <mbgl/storage/network_status.hpp>
 #include <mbgl/util/constants.hpp>
@@ -556,20 +557,20 @@ void QMapboxGL::addCustomLayer(const QString &id,
         void *context_,
         char *before)
 {
-    d_ptr->mapObj->addCustomLayer(
+    d_ptr->mapObj->addLayer(std::make_unique<mbgl::CustomLayer>(
             id.toStdString(),
             reinterpret_cast<mbgl::CustomLayerInitializeFunction>(initFn),
             // This cast is safe as long as both mbgl:: and QMapbox::
             // CustomLayerRenderParameters members remains the same.
             (mbgl::CustomLayerRenderFunction)renderFn,
             reinterpret_cast<mbgl::CustomLayerDeinitializeFunction>(deinitFn),
-            context_,
-            before == NULL ? nullptr : before);
+            context_),
+            before ? mbgl::optional<std::string>(before) : mbgl::optional<std::string>());
 }
 
 void QMapboxGL::removeCustomLayer(const QString& id)
 {
-    d_ptr->mapObj->removeCustomLayer(id.toStdString());
+    d_ptr->mapObj->removeLayer(id.toStdString());
 }
 
 void QMapboxGL::render()

src/mbgl/map/map.cpp

@@ -740,24 +740,17 @@ std::vector<Feature> Map::queryRenderedFeatures(const ScreenBox& box, const opti
 
 #pragma mark - Style API
 
-void Map::addCustomLayer(const std::string& id,
-                         CustomLayerInitializeFunction initialize,
-                         CustomLayerRenderFunction render_,
-                         CustomLayerDeinitializeFunction deinitialize,
-                         void* context_,
-                         const char* before) {
+void Map::addLayer(std::unique_ptr<Layer> layer, const optional<std::string>& before) {
     impl->view.activate();
 
-    impl->style->addLayer(
-        std::make_unique<CustomLayer>(id, initialize, render_, deinitialize, context_),
-        before ? std::string(before) : optional<std::string>());
+    impl->style->addLayer(std::move(layer), before);
     impl->updateFlags |= Update::Classes;
     impl->asyncUpdate.send();
 
     impl->view.deactivate();
 }
 
-void Map::removeCustomLayer(const std::string& id) {
+void Map::removeLayer(const std::string& id) {
     impl->view.activate();
 
     impl->style->removeLayer(id);

test/api/custom_layer.cpp

@@ -77,7 +77,7 @@ TEST(CustomLayer, Basic) {
 
     Map map(view, fileSource, MapMode::Still);
     map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"), "");
-    map.addCustomLayer(
+    map.addLayer(std::make_unique<CustomLayer>(
         "custom",
         [] (void* context) {
             reinterpret_cast<TestLayer*>(context)->initialize();
@@ -87,7 +87,7 @@ TEST(CustomLayer, Basic) {
         },
         [] (void* context) {
             delete reinterpret_cast<TestLayer*>(context);
-        }, new TestLayer());
+        }, new TestLayer()));
 
     test::checkImage("test/fixtures/custom_layer/basic", test::render(map));
 }