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

[core] Add Renderer::clearData() instead of keepRenderData map options #16323

Merged
merged 4 commits into from
Mar 20, 2020
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
16 changes: 2 additions & 14 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,9 @@

### ✨ New features

- [tile mode][static mode] Clear render data for the new still image request ([#16318](https://github.com/mapbox/mapbox-gl-native/pull/16318))
- [core] Add Renderer::clearData() ([#16323](https://github.com/mapbox/mapbox-gl-native/pull/16323))

The new `keepRenderData` map options flag is added to control whether render data shall be kept between `renderStill()` calls.

### 🐞 Bug fixes

- [tile mode][static mode] Clear render data for the new still image request ([#16318](https://github.com/mapbox/mapbox-gl-native/pull/16318))

If the `keepRenderData` map options flag is unset all render data is cleared between `renderStill()` calls, thus stale tiles from the previous `renderStill()` call are never shown.

### 🏁 Performance improvements

- [tile mode][static mode] Clear render data for the new still image request ([#16318](https://github.com/mapbox/mapbox-gl-native/pull/16318))

If the `keepRenderData` map options flag is unset all render data is cleared between `renderStill()` calls, thus saving memory being used.
The newly added `Renderer::clearData()` method allows to clear render data and thus save memory and make sure outdated tiles are not shown. It clears data more agressively than `Renderer::reduceMemoryUse()` does, as it clears not only the cache but all orchestration data, including the data used by the currently rendered frame.

## maps-v1.4.1

Expand Down
22 changes: 0 additions & 22 deletions include/mbgl/map/map_options.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,28 +88,6 @@ class MapOptions final {
*/
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.
Expand Down
1 change: 1 addition & 0 deletions include/mbgl/renderer/renderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class Renderer {

// Memory
void reduceMemoryUse();
void clearData();

private:
class Impl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
12,
340753
8,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious: why did these change with this PR? I would have though that this PR would not change any results and it would only fix a rare race (that I presume was not previously impacting the baselines).

246523
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
5,
183111
6,
268053
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
5,
1500180
8,
1755006
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
13,
2413450
16,
2668276
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
15,
2585276
22,
3183754
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
12,
340753
8,
246523
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
5,
183111
6,
268053
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
5,
1500180
8,
1755006
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
13,
2413450
16,
2668276
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
15,
2585276
22,
3183754
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
12,
340753
8,
246523
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
5,
183111
6,
268053
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
5,
1500180
8,
1755006
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
13,
2413450
16,
2668276
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
15,
2585276
22,
3183754
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
12,
340753
8,
246523
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
5,
183111
6,
268053
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
5,
1500180
8,
1755006
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
13,
2413450
16,
2668276
],
[
"probeNetwork - default - start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"network": [
[
"probeNetwork - default - end",
15,
2585276
22,
3183754
],
[
"probeNetwork - default - start",
Expand Down
4 changes: 2 additions & 2 deletions render-test/runner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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() {}
Expand Down
6 changes: 2 additions & 4 deletions src/mbgl/map/map_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<style::Style>(fileSource, pixelRatio)),
annotationManager(*style) {
Expand Down Expand Up @@ -65,9 +64,8 @@ void Map::Impl::onUpdate() {
annotationManager.makeWeakPtr(),
fileSource,
prefetchZoomDelta,
stillImageRequest.get(),
crossSourceCollisions,
keepRenderData};
bool(stillImageRequest),
crossSourceCollisions};

rendererFrontend.update(std::make_shared<UpdateParameters>(std::move(params)));
}
Expand Down
1 change: 0 additions & 1 deletion src/mbgl/map/map_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class Map::Impl : public style::Observer, public RendererObserver {
const MapMode mode;
const float pixelRatio;
const bool crossSourceCollisions;
const bool keepRenderData;

MapDebugOptions debugOptions { MapDebugOptions::NoDebug };

Expand Down
10 changes: 0 additions & 10 deletions src/mbgl/map/map_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ class MapOptions::Impl {
ViewportMode viewportMode = ViewportMode::Default;
NorthOrientation orientation = NorthOrientation::Upwards;
bool crossSourceCollisions = true;
bool keepRenderData = true;
Size size = { 64, 64 };
float pixelRatio = 1.0;
};
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 1 addition & 4 deletions src/mbgl/renderer/render_orchestrator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,6 @@ std::unique_ptr<RenderTree> RenderOrchestrator::createRenderTree(
if (!isMapModeContinuous) {
// Reset zoom history state.
zoomHistory.first = true;
if (!updateParameters->keepRenderData && stillImageRequest != updateParameters->stillImageRequest) {
clearData();
stillImageRequest = updateParameters->stillImageRequest;
}
}

if (LayerManager::annotationsEnabled) {
Expand Down Expand Up @@ -713,6 +709,7 @@ void RenderOrchestrator::clearData() {
if (!patternAtlas->isEmpty()) patternAtlas = std::make_unique<PatternAtlas>();

imageManager->clear();
glyphManager->evict(fontStacks(*layerImpls));
}

void RenderOrchestrator::onGlyphsError(const FontStack& fontStack, const GlyphRange& glyphRange, std::exception_ptr error) {
Expand Down
3 changes: 1 addition & 2 deletions src/mbgl/renderer/render_orchestrator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ class RenderOrchestrator final : public GlyphManagerObserver,

void reduceMemoryUse();
void dumpDebugLogs();
void clearData();

private:
bool isLoaded() const;
bool hasTransitions(TimePoint) const;
void clearData();

RenderSource* getRenderSource(const std::string& id) const;

Expand Down Expand Up @@ -127,7 +127,6 @@ class RenderOrchestrator final : public GlyphManagerObserver,

const bool backgroundLayerAsColor;
bool contextLost = false;
const void* stillImageRequest = nullptr;

// Vectors with reserved capacity of layerImpls->size() to avoid reallocation
// on each frame.
Expand Down
4 changes: 4 additions & 0 deletions src/mbgl/renderer/renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,8 @@ void Renderer::reduceMemoryUse() {
impl->orchestrator.reduceMemoryUse();
}

void Renderer::clearData() {
impl->orchestrator.clearData();
}

} // namespace mbgl
Loading