diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bc957c6616..ce1ff8a4d12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ New feature provides means of modifying style of a MapSnapshotter. The new API enables several use-cases, such as: adding route overlays, removing extra information (layers) from a base style, adding custom images that are missing from a style. +- [core] Improve stability of symbol placement when the map is tilted ([#16287](https://github.com/mapbox/mapbox-gl-native/pull/16287)) + + These changes improve performance and bring more stability to the symbol placement for the tilted view, which is mainly used for navigation scenarios. + ### 🐞 Bug fixes - [core] Fix iterators in addRegularDash() ([#16249](https://github.com/mapbox/mapbox-gl-native/pull/16249)) diff --git a/metrics/binary-size/android-arm64-v8a/metrics.json b/metrics/binary-size/android-arm64-v8a/metrics.json index c523910a2eb..262799af888 100644 --- a/metrics/binary-size/android-arm64-v8a/metrics.json +++ b/metrics/binary-size/android-arm64-v8a/metrics.json @@ -3,7 +3,7 @@ [ "android-arm64-v8a", "/tmp/attach/install/android-arm64-v8a-release/lib/libmapbox-gl.so", - 1952143 + 1972354 ] ] } \ No newline at end of file diff --git a/metrics/binary-size/android-armeabi-v7a/metrics.json b/metrics/binary-size/android-armeabi-v7a/metrics.json index b5691f1e7dd..5ef1944ca39 100644 --- a/metrics/binary-size/android-armeabi-v7a/metrics.json +++ b/metrics/binary-size/android-armeabi-v7a/metrics.json @@ -3,7 +3,7 @@ [ "android-armeabi-v7a", "/tmp/attach/install/android-armeabi-v7a-release/lib/libmapbox-gl.so", - 1647895 + 1664652 ] ] } \ No newline at end of file diff --git a/metrics/expectations/platform-all/render-tests/text-pitch-alignment/viewport-overzoomed/expected.png b/metrics/expectations/platform-all/render-tests/text-pitch-alignment/viewport-overzoomed/expected.png new file mode 100644 index 00000000000..b996d264c4b Binary files /dev/null and b/metrics/expectations/platform-all/render-tests/text-pitch-alignment/viewport-overzoomed/expected.png differ diff --git a/metrics/expectations/platform-all/render-tests/text-pitch-scaling/line-half/expected.png b/metrics/expectations/platform-all/render-tests/text-pitch-scaling/line-half/expected.png new file mode 100644 index 00000000000..dbb104ee0fd Binary files /dev/null and b/metrics/expectations/platform-all/render-tests/text-pitch-scaling/line-half/expected.png differ diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index f00f9ee9be4..7fb6eff29cf 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -281,6 +281,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree( renderSources.emplace(entry.first, std::move(renderSource)); } transformState = updateParameters->transformState; + const bool tiltedView = transformState.getPitch() != 0.0f; // Create parameters for the render tree. auto renderTreeParameters = std::make_unique(updateParameters->transformState, @@ -386,14 +387,20 @@ std::unique_ptr RenderOrchestrator::createRenderTree( symbolBucketsAdded = symbolBucketsAdded || (result & CrossTileSymbolIndex::AddLayerResult::BucketsAdded); symbolBucketsChanged = symbolBucketsChanged || (result != CrossTileSymbolIndex::AddLayerResult::NoChanges); } - // We want new symbols to show up faster, however simple setting `placementChanged` to `true` would - // initiate placement too often as new buckets ususally come from several rendered tiles in a row within - // a short period of time. Instead, we squeeze placement update period to coalesce buckets updates from several - // tiles. - optional maximumPlacementUpdatePeriod; - if (symbolBucketsAdded) maximumPlacementUpdatePeriod = optional(Milliseconds(30)); + + optional placementUpdatePeriodOverride; + if (symbolBucketsAdded && !tiltedView) { + // If the view is not tilted, we want *the new* symbols to show up faster, however simple setting + // `placementChanged` to `true` would initiate placement too often as new buckets usually come from several + // rendered tiles in a row within a short period of time. Instead, we squeeze placement update period to + // coalesce buckets updates from several tiles. On contrary, with the tilted view it's more important to + // make placement rarely for performance reasons and as the new symbols are normally "far away" and the user + // is not that interested to see them ASAP. + placementUpdatePeriodOverride = optional(Milliseconds(30)); + } + renderTreeParameters->placementChanged = !placementController.placementIsRecent( - updateParameters->timePoint, updateParameters->transformState.getZoom(), maximumPlacementUpdatePeriod); + updateParameters->timePoint, updateParameters->transformState.getZoom(), placementUpdatePeriodOverride); symbolBucketsChanged |= renderTreeParameters->placementChanged; std::set usedSymbolLayers; diff --git a/src/mbgl/text/collision_index.cpp b/src/mbgl/text/collision_index.cpp index e749db3ffa9..04478861665 100644 --- a/src/mbgl/text/collision_index.cpp +++ b/src/mbgl/text/collision_index.cpp @@ -16,19 +16,27 @@ namespace mbgl { +namespace { // When a symbol crosses the edge that causes it to be included in // collision detection, it will cause changes in the symbols around // it. This constant specifies how many pixels to pad the edge of // the viewport for collision detection so that the bulk of the changes // occur offscreen. Making this constant greater increases label // stability, but it's expensive. -static const float viewportPaddingDefault = 100; +const float viewportPaddingDefault = 100; // Viewport padding must be much larger for static tiles to avoid clipped labels. -static const float viewportPaddingForStaticTiles = 1024; +const float viewportPaddingForStaticTiles = 1024; + +inline float getViewportPadding(const TransformState& transformState, MapMode mapMode) { + if (mapMode == MapMode::Tile) return viewportPaddingForStaticTiles; + return (transformState.getPitch() != 0.0f) ? viewportPaddingDefault * 2 : viewportPaddingDefault; +} + +} // namespace CollisionIndex::CollisionIndex(const TransformState& transformState_, MapMode mapMode) : transformState(transformState_), - viewportPadding(mapMode == MapMode::Tile ? viewportPaddingForStaticTiles : viewportPaddingDefault), + viewportPadding(getViewportPadding(transformState_, mapMode)), collisionGrid(transformState.getSize().width + 2 * viewportPadding, transformState.getSize().height + 2 * viewportPadding, 25), diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index d6d6e269bc6..fff11ab185e 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -68,14 +68,10 @@ void PlacementController::setPlacement(Immutable placement_) { stale = false; } -bool PlacementController::placementIsRecent(TimePoint now, const float zoom, optional maximumDuration) const { +bool PlacementController::placementIsRecent(TimePoint now, const float zoom, optional periodOverride) const { if (!placement->transitionsEnabled()) return false; - auto updatePeriod = placement->getUpdatePeriod(zoom); - - if (maximumDuration) { - updatePeriod = std::min(*maximumDuration, updatePeriod); - } + auto updatePeriod = periodOverride ? *periodOverride : placement->getUpdatePeriod(zoom); return placement->getCommitTime() + updatePeriod > now; } @@ -315,12 +311,12 @@ void Placement::placeSymbolBucket(const BucketPlacementData& params, std::setsecond.anchor; auto found = std::find(variableTextAnchors.begin(), variableTextAnchors.end(), prevAnchor); if (found != variableTextAnchors.begin() && found != variableTextAnchors.end()) { - std::vector filtered; - filtered.reserve(variableTextAnchors.size()); - filtered.push_back(prevAnchor); - for (auto anchor : variableTextAnchors) { - if (anchor != prevAnchor) { - filtered.push_back(anchor); + std::vector filtered{prevAnchor}; + if (!isTiltedView()) { + for (auto anchor : variableTextAnchors) { + if (anchor != prevAnchor) { + filtered.push_back(anchor); + } } } variableTextAnchors = std::move(filtered); @@ -596,7 +592,8 @@ void Placement::placeSymbolBucket(const BucketPlacementData& params, std::set shift) -> bool { + auto collisionBoxIntersectsTileEdges = [&](const CollisionBox& collisionBox, + Point shift) noexcept->bool { bool intersects = collisionIndex.intersectsTileEdges(collisionBox, shift, renderTile.matrix, pixelRatio, *tileBorders); // Check if this symbol intersects the neighbor tile borders. If so, it also shall be placed with priority. @@ -608,7 +605,14 @@ void Placement::placeSymbolBucket(const BucketPlacementData& params, std::set bool { + auto symbolIntersectsTileEdges = [ + &locationCache, + &collisionBoxIntersectsTileEdges, + variableAnchor, + pitchTextWithMap, + rotateTextWithMap, + bearing = state.getBearing() + ](const SymbolInstance& symbol) noexcept->bool { auto it = locationCache.find(symbol.crossTileID); if (it != locationCache.end()) return it->second; @@ -627,7 +631,7 @@ void Placement::placeSymbolBucket(const BucketPlacementData& params, std::setgetSymbolPlacement(a); + auto* bPlacement = previousPlacement->getSymbolPlacement(b); + if (!aPlacement) { + // a < b, if 'a' is new and if 'b' was previously hidden. + return bPlacement && !bPlacement->placed(); + } + if (!bPlacement) { + // a < b, if 'b' is new and 'a' was previously shown. + return aPlacement && aPlacement->placed(); + } + // a < b, if 'a' was shown and 'b' was hidden. + return aPlacement->placed() && !bPlacement->placed(); + }); + } + for (const SymbolInstance& symbol : sortedSymbols) { placeSymbol(symbol); } } @@ -1187,6 +1212,10 @@ void Placement::markUsedOrientation(SymbolBucket& bucket, } } +bool Placement::isTiltedView() const { + return updateParameters->transformState.getPitch() != 0.0f; +} + float Placement::symbolFadeChange(TimePoint now) const { if (transitionsEnabled() && transitionOptions.duration.value_or(util::DEFAULT_TRANSITION_DURATION) > Milliseconds(0)) { @@ -1203,6 +1232,12 @@ float Placement::zoomAdjustment(const float zoom) const { return std::max(0.0, (placementZoom - zoom) / 1.5); } +const JointPlacement* Placement::getSymbolPlacement(const SymbolInstance& symbol) const { + assert(symbol.crossTileID != 0); + auto found = placements.find(symbol.crossTileID); + return (found == placements.end()) ? &found->second : nullptr; +} + Duration Placement::getUpdatePeriod(const float zoom) const { // Even if transitionOptions.duration is set to a value < 300ms, we still wait for this default transition duration // before attempting another placement operation. diff --git a/src/mbgl/text/placement.hpp b/src/mbgl/text/placement.hpp index df7345d4cca..d3f92eb0682 100644 --- a/src/mbgl/text/placement.hpp +++ b/src/mbgl/text/placement.hpp @@ -49,6 +49,8 @@ class JointPlacement { : text(text_), icon(icon_), skipFade(skipFade_) {} + bool placed() const { return text || icon; } + const bool text; const bool icon; // skipFade = outside viewport, but within CollisionIndex::viewportPadding px of the edge @@ -94,7 +96,7 @@ class PlacementController { void setPlacement(Immutable); const Immutable& getPlacement() const { return placement; } void setPlacementStale() { stale = true; } - bool placementIsRecent(TimePoint now, const float zoom, optional maximumDuration = nullopt) const; + bool placementIsRecent(TimePoint now, const float zoom, optional periodOverride = nullopt) const; bool hasTransitions(TimePoint now) const; private: @@ -119,6 +121,7 @@ class Placement { Duration getUpdatePeriod(const float zoom) const; float zoomAdjustment(const float zoom) const; + const JointPlacement* getSymbolPlacement(const SymbolInstance&) const; const RetainedQueryData& getQueryData(uint32_t bucketInstanceId) const; @@ -134,6 +137,7 @@ class Placement { style::TextWritingModeType orientation) const; void markUsedOrientation(SymbolBucket&, style::TextWritingModeType, const SymbolInstance&) const; const Placement* getPrevPlacement() const { return prevPlacement ? prevPlacement->get() : nullptr; } + bool isTiltedView() const; std::shared_ptr updateParameters; CollisionIndex collisionIndex;