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

[core] Avoid GeoJSON source flickering on style transitions #15907

Merged
merged 3 commits into from
Nov 12, 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
1 change: 1 addition & 0 deletions platform/android/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Mapbox welcomes participation and contributions from everyone. If you'd like to

### Bug fixes
- Fixed the rendering bug caused by redundant pending requests for already requested images [#15864](https://github.com/mapbox/mapbox-gl-native/pull/15864)
- Fixed Geo JSON source flickering on style transition [#15907](https://github.com/mapbox/mapbox-gl-native/pull/15907)

### Performance improvements
- Enable incremental vacuum for the offline database in order to make data removal requests faster and to avoid the excessive disk space usage (creating a backup file on VACUUM call) [#15837](https://github.com/mapbox/mapbox-gl-native/pull/15837)
Expand Down
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
### Bug fixes
* Fixed the rendering bug caused by redundant pending requests for already requested images [#15864](https://github.com/mapbox/mapbox-gl-native/pull/15864)
* Enable incremental vacuum for the offline database in order to make data removal requests faster and to avoid the excessive disk space usage (creating a backup file on VACUUM call). ([#15837](https://github.com/mapbox/mapbox-gl-native/pull/15837))
* Fixed Geo JSON source flickering on style transition. ([#15907](https://github.com/mapbox/mapbox-gl-native/pull/15907))

## 5.5.0

Expand Down
25 changes: 13 additions & 12 deletions src/mbgl/renderer/sources/render_geojson_source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,26 +85,27 @@ void RenderGeoJSONSource::update(Immutable<style::Source::Impl> baseImpl_,
enabled = needsRendering;

auto data_ = impl().getData().lock();
if (!data_) {
// In Continuous mode, keep the existing tiles if the new data_ is not
// yet available, thus providing smart style transitions without flickering.
// In other modes, allow clearing the tile pyramid first, before the early
// return in order to avoid render tests being flaky.
if (parameters.mode != MapMode::Continuous) tilePyramid.clearAll();
return;
}

if (data.lock() != data_) {
data = data_;
tilePyramid.reduceMemoryUse();

if (data_) {
const uint8_t maxZ = impl().getZoomRange().max;
for (const auto& pair : tilePyramid.getTiles()) {
if (pair.first.canonical.z <= maxZ) {
static_cast<GeoJSONTile*>(pair.second.get())->updateData(data_->getTile(pair.first.canonical), needsRelayout);
}
const uint8_t maxZ = impl().getZoomRange().max;
for (const auto& pair : tilePyramid.getTiles()) {
if (pair.first.canonical.z <= maxZ) {
static_cast<GeoJSONTile*>(pair.second.get())
->updateData(data_->getTile(pair.first.canonical), needsRelayout);
}
}
}

if (!data_) {
tilePyramid.clearAll();
return;
}

tilePyramid.update(layers,
needsRendering,
needsRelayout,
Expand Down
44 changes: 34 additions & 10 deletions test/style/source.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@
#include <mbgl/test/stub_style_observer.hpp>
#include <mbgl/test/stub_render_source_observer.hpp>

#include <mbgl/style/style.hpp>
#include <mbgl/style/source_impl.hpp>
#include <mbgl/style/sources/raster_source.hpp>
#include <mbgl/style/sources/raster_dem_source.hpp>
#include <mbgl/style/sources/vector_source.hpp>
#include <mbgl/style/sources/geojson_source.hpp>
#include <mbgl/style/sources/image_source.hpp>
#include <mbgl/style/sources/custom_geometry_source.hpp>
#include <mbgl/style/layers/circle_layer.hpp>
#include <mbgl/style/layers/circle_layer_impl.hpp>
#include <mbgl/style/layers/hillshade_layer.hpp>
#include <mbgl/style/layers/hillshade_layer_impl.hpp>
#include <mbgl/style/layers/raster_layer.hpp>
#include <mbgl/style/layers/raster_layer_impl.hpp>
#include <mbgl/style/layers/line_layer.hpp>
#include <mbgl/style/layers/line_layer_impl.hpp>
#include <mbgl/style/layers/raster_layer.hpp>
#include <mbgl/style/layers/raster_layer_impl.hpp>
#include <mbgl/style/source_impl.hpp>
#include <mbgl/style/sources/custom_geometry_source.hpp>
#include <mbgl/style/sources/geojson_source.hpp>
#include <mbgl/style/sources/image_source.hpp>
#include <mbgl/style/sources/raster_dem_source.hpp>
#include <mbgl/style/sources/raster_source.hpp>
#include <mbgl/style/sources/vector_source.hpp>
#include <mbgl/style/style.hpp>

#include <mbgl/renderer/sources/render_raster_source.hpp>
#include <mbgl/renderer/sources/render_raster_dem_source.hpp>
Expand Down Expand Up @@ -906,3 +908,25 @@ TEST(Source, RenderTileSetSourceUpdate) {
VectorSource uninitialized("source", "http://url");
renderSource->update(uninitialized.baseImpl, layers, true, true, test.tileParameters);
}

TEST(Source, GeoJSONSourceTilesRemainAfterDataReset) {
SourceTest test;
GeoJSONSource source("source");
source.setGeoJSONData(GeoJSONData::create(
mapbox::geojson::parse(
R"({"geometry": {"type": "Point", "coordinates": [1.1, 1.1]}, "type": "Feature", "properties": {}})"),
{}));
RenderGeoJSONSource renderSource{staticImmutableCast<GeoJSONSource::Impl>(source.baseImpl)};

CircleLayer layer("id", "source");
Immutable<LayerProperties> layerProperties =
makeMutable<CircleLayerProperties>(staticImmutableCast<CircleLayer::Impl>(layer.baseImpl));
std::vector<Immutable<LayerProperties>> layers{layerProperties};

static_cast<RenderSource&>(renderSource).update(source.baseImpl, layers, true, true, test.tileParameters);
EXPECT_FALSE(renderSource.isLoaded()); // loaded == false, means that the source contains pending tiles.

source.setGeoJSONData(nullptr);
static_cast<RenderSource&>(renderSource).update(source.baseImpl, layers, true, true, test.tileParameters);
EXPECT_FALSE(renderSource.isLoaded()); // Tiles remain.
}