From fc70cbf1e12e27737c7a256682524cd617d18f71 Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Mon, 13 Mar 2017 16:16:38 +0200 Subject: [core] Replace MapChange enum with MapObserver --- cmake/core-files.cmake | 1 + include/mbgl/map/backend.hpp | 7 ++-- include/mbgl/map/map.hpp | 1 + include/mbgl/map/map_observer.hpp | 38 +++++++++++++++++++++ src/mbgl/map/backend.cpp | 5 --- src/mbgl/map/map.cpp | 26 +++++++------- src/mbgl/map/transform.cpp | 24 ++++--------- src/mbgl/map/transform.hpp | 7 ++-- test/map/map.test.cpp | 70 ++++++++++++++++++++++++-------------- test/map/transform.test.cpp | 2 +- test/tile/tile_coordinate.test.cpp | 31 +++++++++++++---- 11 files changed, 134 insertions(+), 78 deletions(-) create mode 100644 include/mbgl/map/map_observer.hpp 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 +#include #include @@ -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 #include +#include #include #include #include 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 + +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 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 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 +#include #include -#include #include #include #include @@ -18,7 +18,7 @@ namespace mbgl { class Transform : private util::noncopyable { public: - Transform(std::function = nullptr, + Transform(MapObserver& = MapObserver::nullObserver(), ConstrainMode = ConstrainMode::HeightOnly, ViewportMode = ViewportMode::Default); @@ -163,8 +163,7 @@ public: LatLng screenCoordinateToLatLng(const ScreenCoordinate&) const; private: - std::function 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&& callback) { + didFailLoadingMapCallback = std::move(callback); + } + + void setDidFinishLoadingStyleCallback(std::function&& callback) { + didFinishLoadingStyleCallback = std::move(callback); + } + + void onDidFailLoadingMap() final { + if (didFailLoadingMapCallback) { + didFailLoadingMapCallback(); + } + } + + void onDidFinishLoadingStyle() final { + if (didFinishLoadingStyleCallback) { + didFinishLoadingStyleCallback(); + } + } + + std::function didFailLoadingMapCallback; + std::function 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()); 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 -#include #include #include #include @@ -13,17 +12,35 @@ using namespace mbgl; TEST(TileCoordinate, FromLatLng) { + size_t changeCount = 0; - std::vector 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 cameraWillChangeImmediateCallback; + std::function 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(max), static_cast(max) }); -- cgit v1.2.1