From 2ae96ad6879a00eba0b9115590f58e3eb3cbd019 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 19 Mar 2020 17:21:21 +0200 Subject: [core] Introduce Renderer::clearData() instead of keepRenderData map options `Renderer::clearData()` is a better API than the removed `MapOptions::keepRenderData()`: - gives more flexibility to the client - similar to the existing `Renderer::reduceMemoryUse()` - the `MapOptions::keepRenderData()` API implementation could not handle the raise condition, which happened if the new still image request had come before all tiles from the previous requests were loaded. Co-authored-by: Dane Springmeyer --- include/mbgl/map/map_options.hpp | 22 ---------------- include/mbgl/renderer/renderer.hpp | 1 + render-test/runner.cpp | 4 +-- src/mbgl/map/map_impl.cpp | 6 ++--- src/mbgl/map/map_impl.hpp | 1 - src/mbgl/map/map_options.cpp | 10 ------- src/mbgl/renderer/render_orchestrator.cpp | 4 --- src/mbgl/renderer/render_orchestrator.hpp | 3 +-- src/mbgl/renderer/renderer.cpp | 4 +++ src/mbgl/renderer/update_parameters.hpp | 5 +--- test/map/map.test.cpp | 44 ++++++++++++------------------- 11 files changed, 28 insertions(+), 76 deletions(-) diff --git a/include/mbgl/map/map_options.hpp b/include/mbgl/map/map_options.hpp index 7fd17c2dff..fcb8c8f32f 100644 --- a/include/mbgl/map/map_options.hpp +++ b/include/mbgl/map/map_options.hpp @@ -88,28 +88,6 @@ public: */ bool crossSourceCollisions() const; - /** - * @brief Specify whether render data for layers, sources and images should be kept between renderStill() calls. - * - * This flag is ignored in Continuous mode. In Static mode and Tile mode, if this flag is set to false, all the data - * are created from scratch for every renderStill() call, which guaranties that no extra memory is used, however it - * might cause higher CPU load and network traffic. - * - * By default, it is set to true. - * - * @param keepRenderData true to enable, false to disable - * @return MapOptions for chaining options together. - */ - MapOptions& withKeepRenderData(bool keepRenderData); - - /** - * @brief Gets the previously set (or default) keepRenderData value. - * - * @return true if render data is kept between renderStill() calls, - * false otherwise. - */ - bool keepRenderData() const; - /** * @brief Sets the orientation of the Map. By default, it is set to * Upwards. diff --git a/include/mbgl/renderer/renderer.hpp b/include/mbgl/renderer/renderer.hpp index 91f2b6146c..1d71a016b6 100644 --- a/include/mbgl/renderer/renderer.hpp +++ b/include/mbgl/renderer/renderer.hpp @@ -63,6 +63,7 @@ public: // Memory void reduceMemoryUse(); + void clearData(); private: class Impl; diff --git a/render-test/runner.cpp b/render-test/runner.cpp index 4fc68d0a1a..de43e5ddd8 100644 --- a/render-test/runner.cpp +++ b/render-test/runner.cpp @@ -641,6 +641,7 @@ TestOperations getAfterOperations(const Manifest& manifest) { } void resetContext(const TestMetadata& metadata, TestContext& ctx) { + ctx.getFrontend().getRenderer()->clearData(); ctx.getFrontend().setSize(metadata.size); auto& map = ctx.getMap(); map.setSize(metadata.size); @@ -683,8 +684,7 @@ TestRunner::Impl::Impl(const TestMetadata& metadata, const mbgl::ResourceOptions .withMapMode(metadata.mapMode) .withSize(metadata.size) .withPixelRatio(metadata.pixelRatio) - .withCrossSourceCollisions(metadata.crossSourceCollisions) - .withKeepRenderData(metadata.mapMode != MapMode::Tile), + .withCrossSourceCollisions(metadata.crossSourceCollisions), resourceOptions) {} TestRunner::Impl::~Impl() {} diff --git a/src/mbgl/map/map_impl.cpp b/src/mbgl/map/map_impl.cpp index 60d4d6021c..0edc715134 100644 --- a/src/mbgl/map/map_impl.cpp +++ b/src/mbgl/map/map_impl.cpp @@ -17,7 +17,6 @@ Map::Impl::Impl(RendererFrontend& frontend_, mode(mapOptions.mapMode()), pixelRatio(mapOptions.pixelRatio()), crossSourceCollisions(mapOptions.crossSourceCollisions()), - keepRenderData(mapOptions.keepRenderData()), fileSource(std::move(fileSource_)), style(std::make_unique(fileSource, pixelRatio)), annotationManager(*style) { @@ -65,9 +64,8 @@ void Map::Impl::onUpdate() { annotationManager.makeWeakPtr(), fileSource, prefetchZoomDelta, - stillImageRequest.get(), - crossSourceCollisions, - keepRenderData}; + bool(stillImageRequest), + crossSourceCollisions}; rendererFrontend.update(std::make_shared(std::move(params))); } diff --git a/src/mbgl/map/map_impl.hpp b/src/mbgl/map/map_impl.hpp index 56b18e4b2d..416662f9e5 100644 --- a/src/mbgl/map/map_impl.hpp +++ b/src/mbgl/map/map_impl.hpp @@ -59,7 +59,6 @@ public: const MapMode mode; const float pixelRatio; const bool crossSourceCollisions; - const bool keepRenderData; MapDebugOptions debugOptions { MapDebugOptions::NoDebug }; diff --git a/src/mbgl/map/map_options.cpp b/src/mbgl/map/map_options.cpp index 1e93125fd7..4cebb6adab 100644 --- a/src/mbgl/map/map_options.cpp +++ b/src/mbgl/map/map_options.cpp @@ -9,7 +9,6 @@ public: ViewportMode viewportMode = ViewportMode::Default; NorthOrientation orientation = NorthOrientation::Upwards; bool crossSourceCollisions = true; - bool keepRenderData = true; Size size = { 64, 64 }; float pixelRatio = 1.0; }; @@ -55,15 +54,6 @@ bool MapOptions::crossSourceCollisions() const { return impl_->crossSourceCollisions; } -MapOptions& MapOptions::withKeepRenderData(bool keepRenderData_) { - impl_->keepRenderData = keepRenderData_; - return *this; -} - -bool MapOptions::keepRenderData() const { - return impl_->keepRenderData; -} - MapOptions& MapOptions::withNorthOrientation(NorthOrientation orientation) { impl_->orientation = orientation; return *this; diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index 985aac98b4..08338ff616 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -141,10 +141,6 @@ std::unique_ptr RenderOrchestrator::createRenderTree( if (!isMapModeContinuous) { // Reset zoom history state. zoomHistory.first = true; - if (!updateParameters->keepRenderData && stillImageRequest != updateParameters->stillImageRequest) { - clearData(); - stillImageRequest = updateParameters->stillImageRequest; - } } if (LayerManager::annotationsEnabled) { diff --git a/src/mbgl/renderer/render_orchestrator.hpp b/src/mbgl/renderer/render_orchestrator.hpp index 312b28fd56..7c1abfe1f4 100644 --- a/src/mbgl/renderer/render_orchestrator.hpp +++ b/src/mbgl/renderer/render_orchestrator.hpp @@ -75,11 +75,11 @@ public: void reduceMemoryUse(); void dumpDebugLogs(); + void clearData(); private: bool isLoaded() const; bool hasTransitions(TimePoint) const; - void clearData(); RenderSource* getRenderSource(const std::string& id) const; @@ -127,7 +127,6 @@ private: const bool backgroundLayerAsColor; bool contextLost = false; - const void* stillImageRequest = nullptr; // Vectors with reserved capacity of layerImpls->size() to avoid reallocation // on each frame. diff --git a/src/mbgl/renderer/renderer.cpp b/src/mbgl/renderer/renderer.cpp index 5afbbbd47e..ac2f327a4e 100644 --- a/src/mbgl/renderer/renderer.cpp +++ b/src/mbgl/renderer/renderer.cpp @@ -131,4 +131,8 @@ void Renderer::reduceMemoryUse() { impl->orchestrator.reduceMemoryUse(); } +void Renderer::clearData() { + impl->orchestrator.clearData(); +} + } // namespace mbgl diff --git a/src/mbgl/renderer/update_parameters.hpp b/src/mbgl/renderer/update_parameters.hpp index fbdebc4197..4336fa0972 100644 --- a/src/mbgl/renderer/update_parameters.hpp +++ b/src/mbgl/renderer/update_parameters.hpp @@ -41,12 +41,9 @@ public: const uint8_t prefetchZoomDelta; // For still image requests, render requested - const void* stillImageRequest; + const bool stillImageRequest; const bool crossSourceCollisions; - - // If set, the render data from the previous render calls is kept. - const bool keepRenderData; }; } // namespace mbgl diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp index 0e619d3d1f..baf1cd03a6 100644 --- a/test/map/map.test.cpp +++ b/test/map/map.test.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -1362,26 +1363,7 @@ constexpr auto styleJSON = R"STYLE({ } TEST(Map, KeepRenderData) { - MapTest<> test{std::move(MapOptions().withMapMode(MapMode::Static).withKeepRenderData(true))}; - int requestsCount = 0; - test.fileSource->tileResponse = [&](const Resource&) { - ++requestsCount; - Response res; - res.noContent = true; - return res; - }; - test.map.jumpTo(CameraOptions().withZoom(10)); - test.map.getStyle().loadJSON(styleJSON); - test.frontend.render(test.map); - EXPECT_EQ(4, requestsCount); - - test.map.getStyle().loadJSON(styleJSON); - test.frontend.render(test.map); - EXPECT_EQ(4, requestsCount); -} - -TEST(Map, DontKeepRenderData) { - MapTest<> test{std::move(MapOptions().withMapMode(MapMode::Static).withKeepRenderData(false))}; + MapTest<> test; int requestsCount = 0; test.fileSource->tileResponse = [&](const Resource&) { ++requestsCount; @@ -1390,11 +1372,19 @@ TEST(Map, DontKeepRenderData) { return res; }; test.map.jumpTo(CameraOptions().withZoom(10)); - test.map.getStyle().loadJSON(styleJSON); - test.frontend.render(test.map); - EXPECT_EQ(4, requestsCount); - - test.map.getStyle().loadJSON(styleJSON); - test.frontend.render(test.map); - EXPECT_EQ(8, requestsCount); + const int iterations = 3; + // Keep render data. + for (int i = 1; i <= iterations; ++i) { + test.map.getStyle().loadJSON(styleJSON); + test.frontend.render(test.map); + EXPECT_EQ(4, requestsCount); + } + requestsCount = 0; + // Clear render data. + for (int i = 1; i <= iterations; ++i) { + test.frontend.getRenderer()->clearData(); + test.map.getStyle().loadJSON(styleJSON); + test.frontend.render(test.map); + EXPECT_EQ(4 * i, requestsCount); + } } \ No newline at end of file -- cgit v1.2.1