From 9142feba5c151ba23a8cd17ee22868a4c73b2b20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Fri, 16 Sep 2016 18:12:23 +0200 Subject: [PATCH] [core] do not render layers that are outside their zoom range So far, we didn't properly disable layers that are outside the zoom range. This means that we rendered layers that should not have been rendered, albeit we didn't make any attempt to load tiles for those layers. However, when zooming in/out, existing tiles might already have been loaded in the source which continued to be rendered. In most cases they weren't actually visible because either the matrices weren't updated, or the clip IDs weren't set so that they would be "rendered" off-screen and clipped completely. In any case, we did way too much work. --- src/mbgl/style/layer_impl.hpp | 1 + src/mbgl/style/style.cpp | 25 +++++-- .../map/disabled_layers/first/expected.png | Bin 0 -> 103 bytes .../map/disabled_layers/second/expected.png | Bin 0 -> 103 bytes test/fixtures/map/disabled_layers/tile.png | Bin 0 -> 103 bytes test/map/map.cpp | 61 ++++++++++++++++++ 6 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/map/disabled_layers/first/expected.png create mode 100644 test/fixtures/map/disabled_layers/second/expected.png create mode 100644 test/fixtures/map/disabled_layers/tile.png diff --git a/src/mbgl/style/layer_impl.hpp b/src/mbgl/style/layer_impl.hpp index 55b1ff058c4..2efea506deb 100644 --- a/src/mbgl/style/layer_impl.hpp +++ b/src/mbgl/style/layer_impl.hpp @@ -85,6 +85,7 @@ class Layer::Impl { float minZoom = -std::numeric_limits::infinity(); float maxZoom = std::numeric_limits::infinity(); VisibilityType visibility = VisibilityType::Visible; + bool enabled = false; LayerObserver nullObserver; LayerObserver* observer = &nullObserver; diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index a00c5e4efcc..19baf702095 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -267,10 +267,20 @@ void Style::recalculate(float z, const TimePoint& timePoint, MapMode mode) { hasPendingTransitions = false; for (const auto& layer : layers) { - hasPendingTransitions |= layer->baseImpl->recalculate(parameters); + const bool hasTransitions = layer->baseImpl->recalculate(parameters); - Source* source = getSource(layer->baseImpl->source); - if (source && layer->baseImpl->needsRendering(z)) { + // Disable this layer if it doesn't need to be rendered. + const bool needsRendering = layer->baseImpl->needsRendering(z); + if (!needsRendering) { + layer->baseImpl->enabled = false; + continue; + } + + hasPendingTransitions |= hasTransitions; + layer->baseImpl->enabled = true; + + // If this layer has a source, make sure that it gets loaded. + if (Source* source = getSource(layer->baseImpl->source)) { source->baseImpl->enabled = true; if (!source->baseImpl->loaded) { source->baseImpl->loadDescription(fileSource); @@ -319,8 +329,9 @@ RenderData Style::getRenderData(MapDebugOptions debugOptions) const { } for (const auto& layer : layers) { - if (layer->baseImpl->visibility == VisibilityType::None) + if (!layer->baseImpl->enabled) { continue; + } if (const BackgroundLayer* background = layer->as()) { if (debugOptions & MapDebugOptions::Overdraw) { @@ -400,6 +411,9 @@ std::vector Style::queryRenderedFeatures(const QueryParameters& paramet // Combine all results based on the style layer order. for (const auto& layer : layers) { + if (!layer->baseImpl->enabled) { + continue; + } auto it = resultsByLayer.find(layer->baseImpl->id); if (it != resultsByLayer.end()) { std::move(it->second.begin(), it->second.end(), std::back_inserter(result)); @@ -412,6 +426,9 @@ std::vector Style::queryRenderedFeatures(const QueryParameters& paramet float Style::getQueryRadius() const { float additionalRadius = 0; for (auto& layer : layers) { + if (!layer->baseImpl->enabled) { + continue; + } additionalRadius = util::max(additionalRadius, layer->baseImpl->getQueryRadius()); } return additionalRadius; diff --git a/test/fixtures/map/disabled_layers/first/expected.png b/test/fixtures/map/disabled_layers/first/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..388180c6d1585f762930b3903f368ed09f7bbaf6 GIT binary patch literal 103 zcmeAS@N?(olHy`uVBq!ia0y~yU<5K585o&?RN5XZRUpM2;1l9H=YLUM%tRd^SKiab uF{C2y?EywkAn%aD@8xF46QgB-Y!EoWs0h^GoXY optional { + if (res.url == "asset://tile.png") { + Response response; + response.data = std::make_shared( + util::read_file("test/fixtures/map/disabled_layers/tile.png")); + return {std::move(response)}; + } + return {}; + }; + + Map map(test.view, test.fileSource, MapMode::Still); + map.setZoom(1); + + // This stylesheet has two raster layers, one that starts at zoom 1, the other at zoom 0. + // We first render a map at zoom level 1, which should show both layers (both are "visible" due + // to an opacity of 0.5). Then, we are zooming back out to a zoom level of 0.5 and rerender. + // The "raster1" layer should not be visible anymore since it has minzoom 1, while "raster2" + // should still be there. Both layers have a distinct color through "raster-hue-rotate". + map.setStyleJSON(R"STYLE( +{ + "version": 8, + "name": "Test", + "sources": { + "raster": { + "type": "raster", + "tiles": [ "asset://tile.png" ], + "tileSize": 256 + } + }, + "layers": [{ + "id": "background", + "type": "background", + "paint": { + "background-color": "white" + } + }, { + "id": "raster1", + "type": "raster", + "source": "raster", + "minzoom": 0 + }, { + "id": "raster2", + "type": "raster", + "source": "raster", + "minzoom": 1, + "paint": { + "raster-hue-rotate": 180 + } + }] +} +)STYLE"); + + test::checkImage("test/fixtures/map/disabled_layers/first", test::render(map)); + map.setZoom(0.5); + test::checkImage("test/fixtures/map/disabled_layers/second", test::render(map)); +} + TEST(Map, Classes) { MapTest test;