diff --git a/include/mbgl/map/map_observer.hpp b/include/mbgl/map/map_observer.hpp index 7de6a3fa84e..258699a2a6a 100644 --- a/include/mbgl/map/map_observer.hpp +++ b/include/mbgl/map/map_observer.hpp @@ -34,6 +34,12 @@ class MapObserver { Full }; + struct RenderFrameStatus { + RenderMode mode; + bool needsRepaint; // In continous mode, shows that there are ongoig transitions. + bool placementChanged; + }; + virtual void onCameraWillChange(CameraChangeMode) {} virtual void onCameraIsChanging() {} virtual void onCameraDidChange(CameraChangeMode) {} @@ -41,7 +47,7 @@ class MapObserver { virtual void onDidFinishLoadingMap() {} virtual void onDidFailLoadingMap(MapLoadError, const std::string&) {} virtual void onWillStartRenderingFrame() {} - virtual void onDidFinishRenderingFrame(RenderMode, bool /*placementChanged*/) {} + virtual void onDidFinishRenderingFrame(RenderFrameStatus) {} virtual void onWillStartRenderingMap() {} virtual void onDidFinishRenderingMap(RenderMode) {} virtual void onDidFinishLoadingStyle() {} diff --git a/platform/android/CHANGELOG.md b/platform/android/CHANGELOG.md index 9c262bcb779..6937b7e6d1b 100644 --- a/platform/android/CHANGELOG.md +++ b/platform/android/CHANGELOG.md @@ -4,6 +4,9 @@ Mapbox welcomes participation and contributions from everyone. If you'd like to ## master +### Bug fixes + - Fixed constant repainting for the sources with invisible layers, caused by `RenderSource::hasFadingTiles()` returning `true` all the time. [#15600](https://github.com/mapbox/mapbox-gl-native/pull/15600) + ## 8.4.0-alpha.2 - September 11, 2019 [Changes](https://github.com/mapbox/mapbox-gl-native/compare/android-v8.4.0-alpha.1...android-v8.4.0-alpha.2) since [Mapbox Maps SDK for Android v8.4.0](https://github.com/mapbox/mapbox-gl-native/releases/tag/android-v8.4.0-alpha.1): diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp index 8ef757de15d..7b87693cf52 100755 --- a/platform/android/src/native_map_view.cpp +++ b/platform/android/src/native_map_view.cpp @@ -185,7 +185,7 @@ void NativeMapView::onWillStartRenderingFrame() { } } -void NativeMapView::onDidFinishRenderingFrame(MapObserver::RenderMode mode, bool) { +void NativeMapView::onDidFinishRenderingFrame(MapObserver::RenderFrameStatus status) { assert(vm != nullptr); android::UniqueEnv _env = android::AttachEnv(); @@ -193,7 +193,7 @@ void NativeMapView::onDidFinishRenderingFrame(MapObserver::RenderMode mode, bool static auto onDidFinishRenderingFrame = javaClass.GetMethod(*_env, "onDidFinishRenderingFrame"); auto weakReference = javaPeer.get(*_env); if (weakReference) { - weakReference.Call(*_env, onDidFinishRenderingFrame, (jboolean) (mode != MapObserver::RenderMode::Partial)); + weakReference.Call(*_env, onDidFinishRenderingFrame, (jboolean) (status.mode != MapObserver::RenderMode::Partial)); } } diff --git a/platform/android/src/native_map_view.hpp b/platform/android/src/native_map_view.hpp index 26567a003c3..ba2178022e6 100755 --- a/platform/android/src/native_map_view.hpp +++ b/platform/android/src/native_map_view.hpp @@ -62,7 +62,7 @@ class NativeMapView : public MapObserver { void onDidFinishLoadingMap() override; void onDidFailLoadingMap(MapLoadError, const std::string&) override; void onWillStartRenderingFrame() override; - void onDidFinishRenderingFrame(MapObserver::RenderMode, bool) override; + void onDidFinishRenderingFrame(MapObserver::RenderFrameStatus) override; void onWillStartRenderingMap() override; void onDidFinishRenderingMap(MapObserver::RenderMode) override; void onDidBecomeIdle() override; diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index b15d939e338..15f5904b5d5 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -28,6 +28,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT * Fixed a potential integer overflow at high zoom levels. ([#15560](https://github.com/mapbox/mapbox-gl-native/pull/15560)) * Fixed a bug with annotation view positions after camera transitions. ([#15122](https://github.com/mapbox/mapbox-gl-native/pull/15122/)) * Fixed a bug where the completion block passed to `-[MGLMapView flyToCamera:completionHandler:]` (and related methods) wouldn't be called. ([#15473](https://github.com/mapbox/mapbox-gl-native/pull/15473)) +* Fixed constant repainting for the sources with invisible layers, caused by `RenderSource::hasFadingTiles()` returning `true` all the time. ([#15600](https://github.com/mapbox/mapbox-gl-native/pull/15600)) ## 5.3.0 - August 28, 2019 diff --git a/platform/ios/src/MGLMapView+Impl.h b/platform/ios/src/MGLMapView+Impl.h index 66dc4082747..232215bd1b9 100644 --- a/platform/ios/src/MGLMapView+Impl.h +++ b/platform/ios/src/MGLMapView+Impl.h @@ -62,7 +62,7 @@ class MGLMapViewImpl : public mbgl::MapObserver { void onDidFinishLoadingMap() override; void onDidFailLoadingMap(mbgl::MapLoadError mapError, const std::string& what) override; void onWillStartRenderingFrame() override; - void onDidFinishRenderingFrame(mbgl::MapObserver::RenderMode, bool) override; + void onDidFinishRenderingFrame(mbgl::MapObserver::RenderFrameStatus) override; void onWillStartRenderingMap() override; void onDidFinishRenderingMap(mbgl::MapObserver::RenderMode) override; void onDidFinishLoadingStyle() override; diff --git a/platform/ios/src/MGLMapView+Impl.mm b/platform/ios/src/MGLMapView+Impl.mm index 76c9c0f9ba5..9cdab4dc639 100644 --- a/platform/ios/src/MGLMapView+Impl.mm +++ b/platform/ios/src/MGLMapView+Impl.mm @@ -68,8 +68,8 @@ [mapView mapViewWillStartRenderingFrame]; } -void MGLMapViewImpl::onDidFinishRenderingFrame(mbgl::MapObserver::RenderMode mode, bool) { - bool fullyRendered = mode == mbgl::MapObserver::RenderMode::Full; +void MGLMapViewImpl::onDidFinishRenderingFrame(mbgl::MapObserver::RenderFrameStatus status) { + bool fullyRendered = status.mode == mbgl::MapObserver::RenderMode::Full; [mapView mapViewDidFinishRenderingFrameFullyRendered:fullyRendered]; } diff --git a/platform/macos/src/MGLMapView+Impl.h b/platform/macos/src/MGLMapView+Impl.h index de235fa1477..d33a19dbab9 100644 --- a/platform/macos/src/MGLMapView+Impl.h +++ b/platform/macos/src/MGLMapView+Impl.h @@ -30,7 +30,7 @@ class MGLMapViewImpl : public mbgl::MapObserver { void onDidFinishLoadingMap() override; void onDidFailLoadingMap(mbgl::MapLoadError mapError, const std::string& what) override; void onWillStartRenderingFrame() override; - void onDidFinishRenderingFrame(mbgl::MapObserver::RenderMode, bool /*placementChanged*/) override; + void onDidFinishRenderingFrame(mbgl::MapObserver::RenderFrameStatus) override; void onWillStartRenderingMap() override; void onDidFinishRenderingMap(mbgl::MapObserver::RenderMode) override; void onDidFinishLoadingStyle() override; diff --git a/platform/macos/src/MGLMapView+Impl.mm b/platform/macos/src/MGLMapView+Impl.mm index 1ed5e2ceef7..c00f8581537 100644 --- a/platform/macos/src/MGLMapView+Impl.mm +++ b/platform/macos/src/MGLMapView+Impl.mm @@ -67,8 +67,8 @@ [mapView mapViewWillStartRenderingFrame]; } -void MGLMapViewImpl::onDidFinishRenderingFrame(mbgl::MapObserver::RenderMode mode, bool) { - bool fullyRendered = mode == mbgl::MapObserver::RenderMode::Full; +void MGLMapViewImpl::onDidFinishRenderingFrame(mbgl::MapObserver::RenderFrameStatus status) { + bool fullyRendered = status.mode == mbgl::MapObserver::RenderMode::Full; [mapView mapViewDidFinishRenderingFrameFullyRendered:fullyRendered]; } diff --git a/platform/qt/src/qmapboxgl_map_observer.cpp b/platform/qt/src/qmapboxgl_map_observer.cpp index 7d2af08cc4d..00af666b0f5 100644 --- a/platform/qt/src/qmapboxgl_map_observer.cpp +++ b/platform/qt/src/qmapboxgl_map_observer.cpp @@ -77,9 +77,9 @@ void QMapboxGLMapObserver::onWillStartRenderingFrame() emit mapChanged(QMapboxGL::MapChangeWillStartRenderingFrame); } -void QMapboxGLMapObserver::onDidFinishRenderingFrame(mbgl::MapObserver::RenderMode mode, bool) +void QMapboxGLMapObserver::onDidFinishRenderingFrame(mbgl::MapObserver::RenderFrameStatus status) { - if (mode == mbgl::MapObserver::RenderMode::Partial) { + if (status.mode == mbgl::MapObserver::RenderMode::Partial) { emit mapChanged(QMapboxGL::MapChangeDidFinishRenderingFrame); } else { emit mapChanged(QMapboxGL::MapChangeDidFinishRenderingFrameFullyRendered); diff --git a/platform/qt/src/qmapboxgl_map_observer.hpp b/platform/qt/src/qmapboxgl_map_observer.hpp index 1620c1ef87f..d9e51db28fb 100644 --- a/platform/qt/src/qmapboxgl_map_observer.hpp +++ b/platform/qt/src/qmapboxgl_map_observer.hpp @@ -28,7 +28,7 @@ class QMapboxGLMapObserver : public QObject, public mbgl::MapObserver void onDidFinishLoadingMap() final; void onDidFailLoadingMap(mbgl::MapLoadError, const std::string&) final; void onWillStartRenderingFrame() final; - void onDidFinishRenderingFrame(mbgl::MapObserver::RenderMode, bool /*placementChanged*/) final; + void onDidFinishRenderingFrame(mbgl::MapObserver::RenderFrameStatus) final; void onWillStartRenderingMap() final; void onDidFinishRenderingMap(mbgl::MapObserver::RenderMode) final; void onDidFinishLoadingStyle() final; diff --git a/src/mbgl/map/map_impl.cpp b/src/mbgl/map/map_impl.cpp index ea55dfd1a81..69c3de97834 100644 --- a/src/mbgl/map/map_impl.cpp +++ b/src/mbgl/map/map_impl.cpp @@ -134,7 +134,7 @@ void Map::Impl::onDidFinishRenderingFrame(RenderMode renderMode, bool needsRepai rendererFullyLoaded = renderMode == RenderMode::Full; if (mode == MapMode::Continuous) { - observer.onDidFinishRenderingFrame(MapObserver::RenderMode(renderMode), placemenChanged); + observer.onDidFinishRenderingFrame({MapObserver::RenderMode(renderMode), needsRepaint, placemenChanged}); if (needsRepaint || transform.inTransition()) { onUpdate(); diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp index 54e0b1eb26a..7f0fad1500a 100644 --- a/src/mbgl/renderer/tile_pyramid.cpp +++ b/src/mbgl/renderer/tile_pyramid.cpp @@ -221,8 +221,6 @@ void TilePyramid::update(const std::vector>& l pair.second->setShowCollisionBoxes(parameters.debugOptions & MapDebugOptions::Collision); } - fadingTiles = false; - // Initialize renderable tiles and update the contained layer render data. for (auto& entry : renderedTiles) { Tile& tile = entry.second; @@ -230,7 +228,6 @@ void TilePyramid::update(const std::vector>& l tile.usedByRenderedLayers = false; const bool holdForFade = tile.holdForFade(); - fadingTiles = (fadingTiles || holdForFade); for (const auto& layerProperties : layers) { const auto* typeInfo = layerProperties->baseImpl->getTypeInfo(); if (holdForFade && typeInfo->fadingTiles == LayerTypeInfo::FadingTiles::NotRequired) { @@ -374,6 +371,7 @@ void TilePyramid::dumpDebugLogs() const { } void TilePyramid::clearAll() { + fadingTiles = false; tiles.clear(); renderedTiles.clear(); cache.clear(); @@ -385,9 +383,11 @@ void TilePyramid::addRenderTile(const UnwrappedTileID& tileID, Tile& tile) { } void TilePyramid::updateFadingTiles() { + fadingTiles = false; for (auto& entry : renderedTiles) { Tile& tile = entry.second; if (tile.holdForFade()) { + fadingTiles = true; tile.performedFadePlacement(); } } diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp index 8cb781c6df8..09e1b923363 100644 --- a/test/map/map.test.cpp +++ b/test/map/map.test.cpp @@ -762,7 +762,7 @@ TEST(Map, TEST_DISABLED_ON_CI(ContinuousRendering)) { HeadlessFrontend frontend(1); StubMapObserver observer; - observer.didFinishRenderingFrameCallback = [&] (MapObserver::RenderMode) { + observer.didFinishRenderingFrameCallback = [&] (MapObserver::RenderFrameStatus) { // Start a timer that ends the test one second from now. If we are continuing to render // indefinitely, the timer will be constantly restarted and never trigger. Instead, the // emergency shutoff above will trigger, failing the test. @@ -877,4 +877,53 @@ TEST(Map, Issue15216) { test.map.getStyle().addLayer(std::make_unique("RasterLayer", "ImageSource")); // Passes, if there is no assertion hit. test.runLoop.runOnce(); +} + +// https://github.com/mapbox/mapbox-gl-native/issues/15342 +// Tests the fix for constant repaint caused by `RenderSource::hasFadingTiles()` returning `true` all the time. +TEST(Map, Issue15342) { + MapTest<> test { 1, MapMode::Continuous }; + + test.fileSource->tileResponse = [&](const Resource&) { + Response result; + result.data = std::make_shared(util::read_file("test/fixtures/map/issue12432/0-0-0.mvt")); + return result; + }; + test.map.jumpTo(CameraOptions().withZoom(3.0)); + test.map.getStyle().loadJSON(R"STYLE({ + "version": 8, + "sources": { + "mapbox": { + "type": "vector", + "tiles": ["http://example.com/{z}-{x}-{y}.vector.pbf"] + } + }, + "layers": [{ + "id": "water", + "type": "fill", + "source": "mapbox", + "source-layer": "water" + }] + })STYLE"); + + test.observer.didFinishLoadingMapCallback = [&]() { + test.map.getStyle().loadJSON(R"STYLE({ + "version": 8, + "sources": { + "mapbox": { + "type": "vector", + "tiles": ["http://example.com/{z}-{x}-{y}.vector.pbf"] + } + }, + "layers": [] + })STYLE"); + test.map.jumpTo(CameraOptions().withZoom(20.0)); + test.observer.didFinishRenderingFrameCallback = [&] (MapObserver::RenderFrameStatus status) { + if (!status.needsRepaint) { + test.runLoop.stop(); + } + }; + }; + + test.runLoop.run(); } \ No newline at end of file diff --git a/test/src/mbgl/test/stub_map_observer.hpp b/test/src/mbgl/test/stub_map_observer.hpp index da150ea83cd..89ee4e79532 100644 --- a/test/src/mbgl/test/stub_map_observer.hpp +++ b/test/src/mbgl/test/stub_map_observer.hpp @@ -32,9 +32,9 @@ class StubMapObserver : public MapObserver { } } - void onDidFinishRenderingFrame(RenderMode mode, bool) final { + void onDidFinishRenderingFrame(RenderFrameStatus status) final { if (didFinishRenderingFrameCallback) { - didFinishRenderingFrameCallback(mode); + didFinishRenderingFrameCallback(status); } } @@ -42,7 +42,7 @@ class StubMapObserver : public MapObserver { std::function didFinishLoadingMapCallback; std::function didFailLoadingMapCallback; std::function didFinishLoadingStyleCallback; - std::function didFinishRenderingFrameCallback; + std::function didFinishRenderingFrameCallback; }; diff --git a/test/storage/offline_download.test.cpp b/test/storage/offline_download.test.cpp index 4e967c297cc..ebe3f82ee95 100644 --- a/test/storage/offline_download.test.cpp +++ b/test/storage/offline_download.test.cpp @@ -930,8 +930,8 @@ TEST(OfflineDownload, ResourceOfflineUsageUnset) { }; StubMapObserver mapObserver; - mapObserver.didFinishRenderingFrameCallback = [&] (MapObserver::RenderMode mode) { - if (mode == MapObserver::RenderMode::Full) { + mapObserver.didFinishRenderingFrameCallback = [&] (MapObserver::RenderFrameStatus status) { + if (status.mode == MapObserver::RenderMode::Full) { test.loop.stop(); } };