Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Set fading tiles flag at TilePyramid::updateFadingTiles() #15600

Merged
merged 4 commits into from
Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion include/mbgl/map/map_observer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,20 @@ 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) {}
virtual void onWillStartLoadingMap() {}
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() {}
Expand Down
3 changes: 3 additions & 0 deletions platform/android/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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):

Expand Down
4 changes: 2 additions & 2 deletions platform/android/src/native_map_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,15 @@ void NativeMapView::onWillStartRenderingFrame() {
}
}

void NativeMapView::onDidFinishRenderingFrame(MapObserver::RenderMode mode, bool) {
void NativeMapView::onDidFinishRenderingFrame(MapObserver::RenderFrameStatus status) {
assert(vm != nullptr);

android::UniqueEnv _env = android::AttachEnv();
static auto& javaClass = jni::Class<NativeMapView>::Singleton(*_env);
static auto onDidFinishRenderingFrame = javaClass.GetMethod<void (jboolean)>(*_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));
}
}

Expand Down
2 changes: 1 addition & 1 deletion platform/android/src/native_map_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion platform/ios/src/MGLMapView+Impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions platform/ios/src/MGLMapView+Impl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down
2 changes: 1 addition & 1 deletion platform/macos/src/MGLMapView+Impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions platform/macos/src/MGLMapView+Impl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down
4 changes: 2 additions & 2 deletions platform/qt/src/qmapboxgl_map_observer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion platform/qt/src/qmapboxgl_map_observer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/mbgl/map/map_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions src/mbgl/renderer/tile_pyramid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,16 +221,13 @@ void TilePyramid::update(const std::vector<Immutable<style::LayerProperties>>& 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;
assert(tile.isRenderable());
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) {
Expand Down Expand Up @@ -374,6 +371,7 @@ void TilePyramid::dumpDebugLogs() const {
}

void TilePyramid::clearAll() {
fadingTiles = false;
tiles.clear();
renderedTiles.clear();
cache.clear();
Expand All @@ -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();
}
}
Expand Down
51 changes: 50 additions & 1 deletion test/map/map.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -877,4 +877,53 @@ TEST(Map, Issue15216) {
test.map.getStyle().addLayer(std::make_unique<RasterLayer>("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<std::string>(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();
}
6 changes: 3 additions & 3 deletions test/src/mbgl/test/stub_map_observer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,17 @@ class StubMapObserver : public MapObserver {
}
}

void onDidFinishRenderingFrame(RenderMode mode, bool) final {
void onDidFinishRenderingFrame(RenderFrameStatus status) final {
if (didFinishRenderingFrameCallback) {
didFinishRenderingFrameCallback(mode);
didFinishRenderingFrameCallback(status);
}
}

std::function<void()> willStartLoadingMapCallback;
std::function<void()> didFinishLoadingMapCallback;
std::function<void()> didFailLoadingMapCallback;
std::function<void()> didFinishLoadingStyleCallback;
std::function<void(RenderMode)> didFinishRenderingFrameCallback;
std::function<void(RenderFrameStatus)> didFinishRenderingFrameCallback;
};


Expand Down
4 changes: 2 additions & 2 deletions test/storage/offline_download.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
};
Expand Down