[core] Replace MapChange enum with MapObserver

11 files changed, 134 insertions, 78 deletions

diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake
index afc45a24cc..a22523331e 100644
--- a/cmake/core-files.cmake
+++ b/cmake/core-files.cmake
@@ -107,6 +107,7 @@ set(MBGL_CORE_FILES
     include/mbgl/map/backend_scope.hpp
     include/mbgl/map/camera.hpp
     include/mbgl/map/map.hpp
+    include/mbgl/map/map_observer.hpp
     include/mbgl/map/mode.hpp
     include/mbgl/map/query.hpp
     include/mbgl/map/view.hpp
diff --git a/include/mbgl/map/backend.hpp b/include/mbgl/map/backend.hpp
index 4a43921c0b..cd756abe43 100644
--- a/include/mbgl/map/backend.hpp
+++ b/include/mbgl/map/backend.hpp
@@ -1,6 +1,6 @@
 #pragma once
 
-#include <mbgl/map/change.hpp>
+#include <mbgl/map/map_observer.hpp>
 
 #include <memory>
 
@@ -12,7 +12,7 @@ class Context;
 
 class BackendScope;
 
-class Backend {
+class Backend : public MapObserver {
 public:
     Backend();
     virtual ~Backend();
@@ -24,9 +24,6 @@ public:
     // in the near future. (Not called for Map::renderStill() mode.)
     virtual void invalidate() = 0;
 
-    // Notifies a watcher of map x/y/scale/rotation changes.
-    virtual void notifyMapChange(MapChange change);
-
 protected:
     // Called when the backend's GL context needs to be made active or inactive. These are called,
     // as a matched pair, in four situations:
diff --git a/include/mbgl/map/map.hpp b/include/mbgl/map/map.hpp
index 02e14bea4e..79fcd34d9b 100644
--- a/include/mbgl/map/map.hpp
+++ b/include/mbgl/map/map.hpp
@@ -2,6 +2,7 @@
 
 #include <mbgl/util/optional.hpp>
 #include <mbgl/util/chrono.hpp>
+#include <mbgl/map/map_observer.hpp>
 #include <mbgl/map/mode.hpp>
 #include <mbgl/util/geo.hpp>
 #include <mbgl/util/feature.hpp>
diff --git a/include/mbgl/map/map_observer.hpp b/include/mbgl/map/map_observer.hpp
new file mode 100644
index 0000000000..e0a734cf5b
--- /dev/null
+++ b/include/mbgl/map/map_observer.hpp
@@ -0,0 +1,38 @@
+#pragma once
+
+#include <cstdint>
+
+namespace mbgl {
+
+class MapObserver {
+public:
+    static MapObserver& nullObserver() {
+        static MapObserver mapObserver;
+        return mapObserver;
+    }
+
+    enum class CameraChangeMode : uint32_t {
+        Immediate,
+        Animated
+    };
+
+    enum class RenderMode : uint32_t {
+        Partial,
+        Full
+    };
+
+    virtual void onCameraWillChange(CameraChangeMode) {}
+    virtual void onCameraIsChanging() {}
+    virtual void onCameraDidChange(CameraChangeMode) {}
+    virtual void onWillStartLoadingMap() {}
+    virtual void onDidFinishLoadingMap() {}
+    virtual void onDidFailLoadingMap() {}
+    virtual void onWillStartRenderingFrame() {}
+    virtual void onDidFinishRenderingFrame(RenderMode) {}
+    virtual void onWillStartRenderingMap() {}
+    virtual void onDidFinishRenderingMap(RenderMode) {}
+    virtual void onDidFinishLoadingStyle() {}
+    virtual void onSourceDidChange() {}
+};
+
+} // namespace mbgl
diff --git a/src/mbgl/map/backend.cpp b/src/mbgl/map/backend.cpp
index 8a06fe2d91..2a171cdb76 100644
--- a/src/mbgl/map/backend.cpp
+++ b/src/mbgl/map/backend.cpp
@@ -14,9 +14,4 @@ gl::Context& Backend::getContext() {
 
 Backend::~Backend() = default;
 
-void Backend::notifyMapChange(MapChange) {
-    // no-op
-}
-
-
 } // namespace mbgl
diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index f4e994c931..a006571892 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -70,6 +70,7 @@ public:
     void loadStyleJSON(const std::string&);
 
     Map& map;
+    MapObserver& observer;
     Backend& backend;
     FileSource& fileSource;
     Scheduler& scheduler;
@@ -134,10 +135,11 @@ Map::Impl::Impl(Map& map_,
                 ConstrainMode constrainMode_,
                 ViewportMode viewportMode_)
     : map(map_),
+      observer(backend_),
       backend(backend_),
       fileSource(fileSource_),
       scheduler(scheduler_),
-      transform([this](MapChange change) { backend.notifyMapChange(change); },
+      transform(observer,
                 constrainMode_,
                 viewportMode_),
       mode(mode_),
@@ -264,10 +266,10 @@ void Map::Impl::render(View& view) {
 
     if (mode == MapMode::Continuous) {
         if (renderState == RenderState::Never) {
-            backend.notifyMapChange(MapChangeWillStartRenderingMap);
+            observer.onWillStartRenderingMap();
         }
 
-        backend.notifyMapChange(MapChangeWillStartRenderingFrame);
+        observer.onWillStartRenderingFrame();
 
         FrameData frameData { timePoint,
                               pixelRatio,
@@ -282,18 +284,16 @@ void Map::Impl::render(View& view) {
 
         painter->cleanup();
 
-        backend.notifyMapChange(style->isLoaded() ?
-            MapChangeDidFinishRenderingFrameFullyRendered :
-            MapChangeDidFinishRenderingFrame);
+        observer.onDidFinishRenderingFrame(style->isLoaded() ? MapObserver::RenderMode::Full : MapObserver::RenderMode::Partial);
 
         if (!style->isLoaded()) {
             renderState = RenderState::Partial;
         } else if (renderState != RenderState::Fully) {
             renderState = RenderState::Fully;
-            backend.notifyMapChange(MapChangeDidFinishRenderingMapFullyRendered);
+            observer.onDidFinishRenderingMap(MapObserver::RenderMode::Full);
             if (loading) {
                 loading = false;
-                backend.notifyMapChange(MapChangeDidFinishLoadingMap);
+                observer.onDidFinishLoadingMap();
             }
         }
 
@@ -341,7 +341,7 @@ void Map::setStyleURL(const std::string& url) {
 
     impl->loading = true;
 
-    impl->backend.notifyMapChange(MapChangeWillStartLoadingMap);
+    impl->observer.onWillStartLoadingMap();
 
     impl->styleRequest = nullptr;
     impl->styleURL = url;
@@ -385,7 +385,7 @@ void Map::setStyleJSON(const std::string& json) {
 
     impl->loading = true;
 
-    impl->backend.notifyMapChange(MapChangeWillStartLoadingMap);
+    impl->observer.onWillStartLoadingMap();
 
     impl->styleURL.clear();
     impl->styleJSON.clear();
@@ -1086,7 +1086,7 @@ void Map::onLowMemory() {
 }
 
 void Map::Impl::onSourceAttributionChanged(style::Source&, const std::string&) {
-    backend.notifyMapChange(MapChangeSourceDidChange);
+    observer.onSourceDidChange();
 }
 
 void Map::Impl::onUpdate(Update flags) {
@@ -1095,11 +1095,11 @@ void Map::Impl::onUpdate(Update flags) {
 }
 
 void Map::Impl::onStyleLoaded() {
-    backend.notifyMapChange(MapChangeDidFinishLoadingStyle);
+    observer.onDidFinishLoadingStyle();
 }
 
 void Map::Impl::onStyleError() {
-    backend.notifyMapChange(MapChangeDidFailLoadingMap);
+    observer.onDidFailLoadingMap();
 }
 
 void Map::Impl::onResourceError(std::exception_ptr error) {
diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp
index eab9e97bc0..d325271388 100644
--- a/src/mbgl/map/transform.cpp
+++ b/src/mbgl/map/transform.cpp
@@ -37,10 +37,10 @@ static double _normalizeAngle(double angle, double anchorAngle)
     return angle;
 }
 
-Transform::Transform(std::function<void(MapChange)> callback_,
+Transform::Transform(MapObserver& observer_,
                      ConstrainMode constrainMode,
                      ViewportMode viewportMode)
-    : callback(std::move(callback_)), state(constrainMode, viewportMode) {
+    : observer(observer_), state(constrainMode, viewportMode) {
 }
 
 #pragma mark - Map View
@@ -50,16 +50,12 @@ bool Transform::resize(const Size size) {
         return false;
     }
 
-    if (callback) {
-        callback(MapChangeRegionWillChange);
-    }
+    observer.onCameraWillChange(MapObserver::CameraChangeMode::Immediate);
 
     state.size = size;
     state.constrain(state.scale, state.x, state.y);
 
-    if (callback) {
-        callback(MapChangeRegionDidChange);
-    }
+    observer.onCameraDidChange(MapObserver::CameraChangeMode::Immediate);
 
     return true;
 }
@@ -556,9 +552,7 @@ void Transform::startTransition(const CameraOptions& camera,
     }
 
     bool isAnimated = duration != Duration::zero();
-    if (callback) {
-        callback(isAnimated ? MapChangeRegionWillChangeAnimated : MapChangeRegionWillChange);
-    }
+    observer.onCameraWillChange(isAnimated ? MapObserver::CameraChangeMode::Animated : MapObserver::CameraChangeMode::Immediate);
 
     // Associate the anchor, if given, with a coordinate.
     optional<ScreenCoordinate> anchor = camera.anchor;
@@ -588,9 +582,7 @@ void Transform::startTransition(const CameraOptions& camera,
             if (animation.transitionFrameFn) {
                 animation.transitionFrameFn(t);
             }
-            if (callback) {
-                callback(MapChangeRegionIsChanging);
-            }
+            observer.onCameraIsChanging();
         } else {
             transitionFinishFn();
             transitionFinishFn = nullptr;
@@ -609,9 +601,7 @@ void Transform::startTransition(const CameraOptions& camera,
         if (animation.transitionFinishFn) {
             animation.transitionFinishFn();
         }
-        if (callback) {
-            callback(isAnimated ? MapChangeRegionDidChangeAnimated : MapChangeRegionDidChange);
-        }
+        observer.onCameraDidChange(isAnimated ? MapObserver::CameraChangeMode::Animated : MapObserver::CameraChangeMode::Immediate);
     };
 
     if (!isAnimated) {
diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp
index 66c9915715..087ae8e690 100644
--- a/src/mbgl/map/transform.hpp
+++ b/src/mbgl/map/transform.hpp
@@ -1,8 +1,8 @@
 #pragma once
 
 #include <mbgl/map/camera.hpp>
+#include <mbgl/map/map_observer.hpp>
 #include <mbgl/map/mode.hpp>
-#include <mbgl/map/change.hpp>
 #include <mbgl/map/transform_state.hpp>
 #include <mbgl/map/update.hpp>
 #include <mbgl/util/chrono.hpp>
@@ -18,7 +18,7 @@ namespace mbgl {
 
 class Transform : private util::noncopyable {
 public:
-    Transform(std::function<void(MapChange)> = nullptr,
+    Transform(MapObserver& = MapObserver::nullObserver(),
               ConstrainMode = ConstrainMode::HeightOnly,
               ViewportMode = ViewportMode::Default);
 
@@ -163,8 +163,7 @@ public:
     LatLng screenCoordinateToLatLng(const ScreenCoordinate&) const;
 
 private:
-    std::function<void(MapChange)> callback;
-
+    MapObserver& observer;
     TransformState state;
 
     void startTransition(const CameraOptions&,
diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp
index 618c2e6a74..c5d579be3a 100644
--- a/test/map/map.test.cpp
+++ b/test/map/map.test.cpp
@@ -24,9 +24,37 @@ using namespace mbgl;
 using namespace mbgl::style;
 using namespace std::literals::string_literals;
 
+class BackendTest : public HeadlessBackend {
+public:
+    BackendTest() : HeadlessBackend(test::sharedDisplay()) {}
+
+    void setDidFailLoadingMapCallback(std::function<void()>&& callback) {
+        didFailLoadingMapCallback = std::move(callback);
+    }
+
+    void setDidFinishLoadingStyleCallback(std::function<void()>&& callback) {
+        didFinishLoadingStyleCallback = std::move(callback);
+    }
+
+    void onDidFailLoadingMap() final {
+        if (didFailLoadingMapCallback) {
+            didFailLoadingMapCallback();
+        }
+    }
+
+    void onDidFinishLoadingStyle() final {
+        if (didFinishLoadingStyleCallback) {
+            didFinishLoadingStyleCallback();
+        }
+    }
+
+    std::function<void()> didFailLoadingMapCallback;
+    std::function<void()> didFinishLoadingStyleCallback;
+};
+
 struct MapTest {
     util::RunLoop runLoop;
-    HeadlessBackend backend { test::sharedDisplay() };
+    BackendTest backend;
     OffscreenView view { backend.getContext() };
     StubFileSource fileSource;
     ThreadPool threadPool { 4 };
@@ -85,10 +113,8 @@ TEST(Map, SetStyleInvalidJSON) {
     Log::setObserver(std::make_unique<FixtureLogObserver>());
 
     bool fail = false;
-    test.backend.setMapChangeCallback([&](MapChange change) {
-        if (change == mbgl::MapChangeDidFailLoadingMap) {
-            fail = true;
-        }
+    test.backend.setDidFailLoadingMapCallback([&]() {
+        fail = true;
     });
 
     {
@@ -118,10 +144,8 @@ TEST(Map, SetStyleInvalidURL) {
         return response;
     };
 
-    test.backend.setMapChangeCallback([&](MapChange change) {
-        if (change == mbgl::MapChangeDidFailLoadingMap) {
-            test.runLoop.stop();
-        }
+    test.backend.setDidFailLoadingMapCallback([&]() {
+        test.runLoop.stop();
     });
 
     Map map(test.backend, test.view.getSize(), 1, test.fileSource, test.threadPool, MapMode::Still);
@@ -233,10 +257,8 @@ TEST(Map, StyleLoadedSignal) {
 
     // The map should emit a signal on style loaded
     bool emitted = false;
-    test.backend.setMapChangeCallback([&](MapChange change) {
-        if (change == mbgl::MapChangeDidFinishLoadingStyle) {
-            emitted = true;
-        }
+    test.backend.setDidFinishLoadingStyleCallback([&]() {
+        emitted = true;
     });
     map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
     EXPECT_TRUE(emitted);
@@ -255,10 +277,8 @@ TEST(Map, TEST_REQUIRES_SERVER(StyleNetworkErrorRetry)) {
     Map map(test.backend, test.view.getSize(), 1, fileSource, test.threadPool, MapMode::Still);
     map.setStyleURL("http://127.0.0.1:3000/style-fail-once-500");
 
-    test.backend.setMapChangeCallback([&](MapChange change) {
-        if (change == mbgl::MapChangeDidFinishLoadingStyle) {
-            test.runLoop.stop();
-        }
+    test.backend.setDidFinishLoadingStyleCallback([&]() {
+        test.runLoop.stop();
     });
 
     test.runLoop.run();
@@ -275,16 +295,14 @@ TEST(Map, TEST_REQUIRES_SERVER(StyleNotFound)) {
     util::Timer timer;
 
     // Not found errors should not trigger a retry like other errors.
-    test.backend.setMapChangeCallback([&](MapChange change) {
-        if (change == mbgl::MapChangeDidFinishLoadingStyle) {
-            FAIL() << "Should not retry on not found!";
-        }
+    test.backend.setDidFinishLoadingStyleCallback([&]() {
+        FAIL() << "Should not retry on not found!";
+    });
 
-        if (change == mbgl::MapChangeDidFailLoadingMap) {
-            timer.start(Milliseconds(1100), 0s, [&] {
-                test.runLoop.stop();
-            });
-        }
+    test.backend.setDidFailLoadingMapCallback([&]() {
+        timer.start(Milliseconds(1100), 0s, [&] {
+            test.runLoop.stop();
+        });
     });
 
     test.runLoop.run();
diff --git a/test/map/transform.test.cpp b/test/map/transform.test.cpp
index 9125b6ef1d..5bff4ee5ef 100644
--- a/test/map/transform.test.cpp
+++ b/test/map/transform.test.cpp
@@ -215,7 +215,7 @@ TEST(Transform, ConstrainHeightOnly) {
 TEST(Transform, ConstrainWidthAndHeight) {
     LatLng loc;
 
-    Transform transform(nullptr, ConstrainMode::WidthAndHeight);
+    Transform transform(MapObserver::nullObserver(), ConstrainMode::WidthAndHeight);
     transform.resize({ 1000, 1000 });
     transform.setScale(std::pow(2, util::MAX_ZOOM));
 
diff --git a/test/tile/tile_coordinate.test.cpp b/test/tile/tile_coordinate.test.cpp
index 4d58bf8c51..d1bef85cb5 100644
--- a/test/tile/tile_coordinate.test.cpp
+++ b/test/tile/tile_coordinate.test.cpp
@@ -1,6 +1,5 @@
 #include <mbgl/test/util.hpp>
 
-#include <mbgl/map/change.hpp>
 #include <mbgl/map/transform.hpp>
 #include <mbgl/map/transform_state.hpp>
 #include <mbgl/tile/tile.hpp>
@@ -13,17 +12,35 @@
 using namespace mbgl;
 
 TEST(TileCoordinate, FromLatLng) {
+
     size_t changeCount = 0;
-    std::vector<MapChange> changes = {
-        MapChangeRegionWillChange,
-        MapChangeRegionDidChange,
+    struct TransformObserver : public mbgl::MapObserver {
+        void onCameraWillChange(MapObserver::CameraChangeMode mode) final {
+            if (mode == MapObserver::CameraChangeMode::Immediate && cameraWillChangeImmediateCallback) {
+                cameraWillChangeImmediateCallback();
+            }
+        }
+
+        void onCameraDidChange(MapObserver::CameraChangeMode mode) final {
+            if (mode == MapObserver::CameraChangeMode::Immediate && cameraDidChangeImmediateCallback) {
+                cameraDidChangeImmediateCallback();
+            }
+        }
+
+        std::function<void()> cameraWillChangeImmediateCallback;
+        std::function<void()> cameraDidChangeImmediateCallback;
     };
-    auto onMapChange = [&](MapChange change) {
-        ASSERT_EQ(change, changes[changeCount]);
+
+    TransformObserver observer;
+    observer.cameraWillChangeImmediateCallback = [&]() {
+        ASSERT_EQ(changeCount, 0u);
         ++changeCount;
     };
+    observer.cameraDidChangeImmediateCallback = [&]() {
+        ASSERT_EQ(changeCount, 1u);
+    };
 
-    Transform transform(onMapChange);
+    Transform transform(observer);
 
     const double max = util::tileSize;
     transform.resize({ static_cast<uint32_t>(max), static_cast<uint32_t>(max) });