From 302b13bf7c3ecb59e92288ec80dd0ba35f395970 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Tue, 14 Jan 2020 20:59:22 +0200 Subject: [PATCH 01/11] [core] Consider symbol bucket leader id in cross-tile symbol indexing Only buckets with the same leader id participate in `TileLayerIndex::findMatches()` in order to improve its performace. `TileLayerIndex` constness is fixed. --- src/mbgl/text/cross_tile_symbol_index.cpp | 47 +++++++++++++++-------- src/mbgl/text/cross_tile_symbol_index.hpp | 12 ++++-- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp index 55ab2cc3c53..d8bd0292ec5 100644 --- a/src/mbgl/text/cross_tile_symbol_index.cpp +++ b/src/mbgl/text/cross_tile_symbol_index.cpp @@ -6,15 +6,19 @@ namespace mbgl { - -TileLayerIndex::TileLayerIndex(OverscaledTileID coord_, std::vector& symbolInstances, uint32_t bucketInstanceId_) - : coord(coord_), bucketInstanceId(bucketInstanceId_) { - for (SymbolInstance& symbolInstance : symbolInstances) { - indexedSymbolInstances[symbolInstance.key].emplace_back(symbolInstance.crossTileID, getScaledCoordinates(symbolInstance, coord)); - } +TileLayerIndex::TileLayerIndex(OverscaledTileID coord_, + std::vector& symbolInstances, + uint32_t bucketInstanceId_, + std::string bucketLeaderId_) + : coord(coord_), bucketInstanceId(bucketInstanceId_), bucketLeaderId(std::move(bucketLeaderId_)) { + for (SymbolInstance& symbolInstance : symbolInstances) { + indexedSymbolInstances[symbolInstance.key].emplace_back(symbolInstance.crossTileID, + getScaledCoordinates(symbolInstance, coord)); } +} -Point TileLayerIndex::getScaledCoordinates(SymbolInstance& symbolInstance, const OverscaledTileID& childTileCoord) { +Point TileLayerIndex::getScaledCoordinates(SymbolInstance& symbolInstance, + const OverscaledTileID& childTileCoord) const { // Round anchor positions to roughly 4 pixel grid const double roundingFactor = 512.0 / util::EXTENT / 2.0; const double scale = roundingFactor / std::pow(2, childTileCoord.canonical.z - coord.canonical.z); @@ -24,9 +28,14 @@ Point TileLayerIndex::getScaledCoordinates(SymbolInstance& symbolInstan }; } -void TileLayerIndex::findMatches(std::vector& symbolInstances, const OverscaledTileID& newCoord, std::set& zoomCrossTileIDs) { +void TileLayerIndex::findMatches(SymbolBucket& bucket, + const OverscaledTileID& newCoord, + std::set& zoomCrossTileIDs) const { + auto& symbolInstances = bucket.symbolInstances; float tolerance = coord.canonical.z < newCoord.canonical.z ? 1 : std::pow(2, coord.canonical.z - newCoord.canonical.z); + if (bucket.bucketLeaderID != bucketLeaderId) return; + for (auto& symbolInstance : symbolInstances) { if (symbolInstance.crossTileID) { // already has a match, skip @@ -41,7 +50,7 @@ void TileLayerIndex::findMatches(std::vector& symbolInstances, c auto scaledSymbolCoord = getScaledCoordinates(symbolInstance, newCoord); - for (IndexedSymbolInstance& thisTileSymbol: it->second) { + for (const IndexedSymbolInstance& thisTileSymbol : it->second) { // Return any symbol with the same keys whose coordinates are within 1 // grid unit. (with a 4px grid, this covers a 12px by 12px area) if (std::abs(thisTileSymbol.coord.x - scaledSymbolCoord.x) <= tolerance && @@ -87,7 +96,7 @@ void CrossTileSymbolLayerIndex::handleWrapJump(float newLng) { } bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, SymbolBucket& bucket, uint32_t& maxCrossTileID) { - const auto& thisZoomIndexes = indexes[tileID.overscaledZ]; + auto& thisZoomIndexes = indexes[tileID.overscaledZ]; auto previousIndex = thisZoomIndexes.find(tileID); if (previousIndex != thisZoomIndexes.end()) { if (previousIndex->second.bucketInstanceId == bucket.bucketInstanceId) { @@ -105,20 +114,22 @@ bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, Symbol symbolInstance.crossTileID = 0; } + auto& thisZoomUsedCrossTileIDs = usedCrossTileIDs[tileID.overscaledZ]; + for (auto& it : indexes) { auto zoom = it.first; - auto zoomIndexes = it.second; + const auto& zoomIndexes = it.second; if (zoom > tileID.overscaledZ) { for (auto& childIndex : zoomIndexes) { if (childIndex.second.coord.isChildOf(tileID)) { - childIndex.second.findMatches(bucket.symbolInstances, tileID, usedCrossTileIDs[tileID.overscaledZ]); + childIndex.second.findMatches(bucket, tileID, thisZoomUsedCrossTileIDs); } } } else { auto parentTileID = tileID.scaledTo(zoom); auto parentIndex = zoomIndexes.find(parentTileID); if (parentIndex != zoomIndexes.end()) { - parentIndex->second.findMatches(bucket.symbolInstances, tileID, usedCrossTileIDs[tileID.overscaledZ]); + parentIndex->second.findMatches(bucket, tileID, thisZoomUsedCrossTileIDs); } } } @@ -127,13 +138,15 @@ bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, Symbol if (!symbolInstance.crossTileID) { // symbol did not match any known symbol, assign a new id symbolInstance.crossTileID = ++maxCrossTileID; - usedCrossTileIDs[tileID.overscaledZ].insert(symbolInstance.crossTileID); + thisZoomUsedCrossTileIDs.insert(symbolInstance.crossTileID); } } - - indexes[tileID.overscaledZ].erase(tileID); - indexes[tileID.overscaledZ].emplace(tileID, TileLayerIndex(tileID, bucket.symbolInstances, bucket.bucketInstanceId)); + thisZoomIndexes.erase(tileID); + thisZoomIndexes.emplace( + std::piecewise_construct, + std::forward_as_tuple(tileID), + std::forward_as_tuple(tileID, bucket.symbolInstances, bucket.bucketInstanceId, bucket.bucketLeaderID)); return true; } diff --git a/src/mbgl/text/cross_tile_symbol_index.hpp b/src/mbgl/text/cross_tile_symbol_index.hpp index 4e32698b3ea..ddcdb814c61 100644 --- a/src/mbgl/text/cross_tile_symbol_index.hpp +++ b/src/mbgl/text/cross_tile_symbol_index.hpp @@ -31,13 +31,17 @@ class IndexedSymbolInstance { class TileLayerIndex { public: - TileLayerIndex(OverscaledTileID coord, std::vector&, uint32_t bucketInstanceId); + TileLayerIndex(OverscaledTileID coord, + std::vector&, + uint32_t bucketInstanceId, + std::string bucketLeaderId); + + Point getScaledCoordinates(SymbolInstance&, const OverscaledTileID&) const; + void findMatches(SymbolBucket&, const OverscaledTileID&, std::set&) const; - Point getScaledCoordinates(SymbolInstance&, const OverscaledTileID&); - void findMatches(std::vector&, const OverscaledTileID&, std::set&); - OverscaledTileID coord; uint32_t bucketInstanceId; + std::string bucketLeaderId; std::map> indexedSymbolInstances; }; From c53ffcefdf2fe18dd9d847817e74a65896e97ce7 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Wed, 15 Jan 2020 15:27:09 +0200 Subject: [PATCH 02/11] [core] Pass placement commit parameters in constructor --- src/mbgl/renderer/render_orchestrator.cpp | 8 +++++--- src/mbgl/text/placement.cpp | 10 ++++++---- src/mbgl/text/placement.hpp | 5 +++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index 2462ee69866..aa32acc04a5 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -400,6 +400,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar updateParameters.mode, updateParameters.transitionOptions, updateParameters.crossSourceCollisions, + updateParameters.timePoint, placementController.getPlacement()); for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { @@ -408,7 +409,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar placement->placeLayer(layer, renderTreeParameters->transformParams.projMatrix, updateParameters.debugOptions & MapDebugOptions::Collision); } - placement->commit(updateParameters.timePoint, updateParameters.transformState.getZoom()); + placement->commit(); crossTileSymbolIndex.pruneUnusedLayers(usedSymbolLayers); for (const auto& entry : renderSources) { entry.second->updateFadingTiles(); @@ -427,7 +428,8 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar Mutable placement = makeMutable(updateParameters.transformState, updateParameters.mode, updateParameters.transitionOptions, - updateParameters.crossSourceCollisions); + updateParameters.crossSourceCollisions, + updateParameters.timePoint); for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; crossTileSymbolIndex.addLayer(layer, updateParameters.transformState.getLatLng().longitude()); @@ -435,7 +437,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar renderTreeParameters->transformParams.projMatrix, updateParameters.debugOptions & MapDebugOptions::Collision); } - placement->commit(updateParameters.timePoint, updateParameters.transformState.getZoom()); + placement->commit(); placementController.setPlacement(std::move(placement)); } renderTreeParameters->symbolFadeChange = 1.0f; diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index d6a78082fa4..f3917a8fdbd 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -61,7 +61,8 @@ const CollisionGroups::CollisionGroup& CollisionGroups::get(const std::string& s // PlacementController implemenation PlacementController::PlacementController() - : placement(makeMutable(TransformState{}, MapMode::Static, style::TransitionOptions{}, true, nullopt)) {} + : placement(makeMutable( + TransformState{}, MapMode::Static, style::TransitionOptions{}, true, TimePoint(), nullopt)) {} void PlacementController::setPlacement(Immutable placement_) { placement = std::move(placement_); @@ -94,10 +95,12 @@ Placement::Placement(const TransformState& state_, MapMode mapMode_, style::TransitionOptions transitionOptions_, const bool crossSourceCollisions, + TimePoint commitTime_, optional> prevPlacement_) : collisionIndex(state_, mapMode_), mapMode(mapMode_), transitionOptions(std::move(transitionOptions_)), + commitTime(commitTime_), placementZoom(state_.getZoom()), collisionGroups(crossSourceCollisions), prevPlacement(std::move(prevPlacement_)) { @@ -622,8 +625,7 @@ void Placement::placeBucket(const SymbolBucket& bucket, std::forward_as_tuple(bucket.bucketInstanceId, params.featureIndex, overscaledID)); } -void Placement::commit(TimePoint now, const double zoom) { - commitTime = now; +void Placement::commit() { if (!getPrevPlacement()) { assert(mapMode != MapMode::Continuous); fadeStartTime = commitTime; @@ -638,7 +640,7 @@ void Placement::commit(TimePoint now, const double zoom) { bool placementChanged = false; - prevZoomAdjustment = getPrevPlacement()->zoomAdjustment(zoom); + prevZoomAdjustment = getPrevPlacement()->zoomAdjustment(placementZoom); float increment = getPrevPlacement()->symbolFadeChange(commitTime); // add the opacities from the current placement, and copy their current values from the previous placement diff --git a/src/mbgl/text/placement.hpp b/src/mbgl/text/placement.hpp index 4dd9011da46..dd46970780d 100644 --- a/src/mbgl/text/placement.hpp +++ b/src/mbgl/text/placement.hpp @@ -116,9 +116,10 @@ class Placement { MapMode, style::TransitionOptions, const bool crossSourceCollisions, + TimePoint commitTime, optional> prevPlacement = nullopt); void placeLayer(const RenderLayer&, const mat4&, bool showCollisionBoxes); - void commit(TimePoint, const double zoom); + void commit(); void updateLayerBuckets(const RenderLayer&, const TransformState&, bool updateOpacities) const; float symbolFadeChange(TimePoint now) const; bool hasTransitions(TimePoint now) const; @@ -151,7 +152,7 @@ class Placement { TimePoint fadeStartTime; TimePoint commitTime; - float placementZoom; + const float placementZoom; float prevZoomAdjustment = 0; std::unordered_map placements; From 54d4da202ce6e43e40630db669cb6695d38859de Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Wed, 15 Jan 2020 16:30:34 +0200 Subject: [PATCH 03/11] [core] Pass std::shared_ptr to the render orchestrator So that it can retain ownership of the given parameters. --- include/mbgl/renderer/renderer.hpp | 2 +- platform/android/src/map_renderer.cpp | 2 +- platform/darwin/src/MGLRendererFrontend.h | 2 +- .../src/mbgl/gfx/headless_frontend.cpp | 2 +- platform/glfw/glfw_renderer_frontend.cpp | 2 +- platform/qt/src/qmapboxgl_map_renderer.cpp | 2 +- src/mbgl/renderer/render_orchestrator.cpp | 128 +++++++++--------- src/mbgl/renderer/render_orchestrator.hpp | 2 +- src/mbgl/renderer/renderer.cpp | 3 +- 9 files changed, 73 insertions(+), 72 deletions(-) diff --git a/include/mbgl/renderer/renderer.hpp b/include/mbgl/renderer/renderer.hpp index 7a1ddde1c13..91f2b6146ce 100644 --- a/include/mbgl/renderer/renderer.hpp +++ b/include/mbgl/renderer/renderer.hpp @@ -31,7 +31,7 @@ class Renderer { void setObserver(RendererObserver*); - void render(const UpdateParameters&); + void render(const std::shared_ptr&); // Feature queries std::vector queryRenderedFeatures(const ScreenLineString&, const RenderedQueryOptions& options = {}) const; diff --git a/platform/android/src/map_renderer.cpp b/platform/android/src/map_renderer.cpp index 0c0e907f149..f5336e3dd88 100644 --- a/platform/android/src/map_renderer.cpp +++ b/platform/android/src/map_renderer.cpp @@ -143,7 +143,7 @@ void MapRenderer::render(JNIEnv&) { framebufferSizeChanged = false; } - renderer->render(*params); + renderer->render(params); // Deliver the snapshot if requested if (snapshotCallback) { diff --git a/platform/darwin/src/MGLRendererFrontend.h b/platform/darwin/src/MGLRendererFrontend.h index 1358f5fafa7..e2b8260fd15 100644 --- a/platform/darwin/src/MGLRendererFrontend.h +++ b/platform/darwin/src/MGLRendererFrontend.h @@ -54,7 +54,7 @@ class MGLRenderFrontend : public mbgl::RendererFrontend // Copy the shared pointer here so that the parameters aren't destroyed while `render(...)` is // still using them. auto updateParameters_ = updateParameters; - renderer->render(*updateParameters_); + renderer->render(updateParameters_); } mbgl::Renderer* getRenderer() { diff --git a/platform/default/src/mbgl/gfx/headless_frontend.cpp b/platform/default/src/mbgl/gfx/headless_frontend.cpp index 5235b2f4084..996006bfe26 100644 --- a/platform/default/src/mbgl/gfx/headless_frontend.cpp +++ b/platform/default/src/mbgl/gfx/headless_frontend.cpp @@ -39,7 +39,7 @@ HeadlessFrontend::HeadlessFrontend(Size size_, // Copy the shared pointer here so that the parameters aren't destroyed while `render(...)` is // still using them. auto updateParameters_ = updateParameters; - renderer->render(*updateParameters_); + renderer->render(updateParameters_); auto endTime = mbgl::util::MonotonicTimer::now(); frameTime = (endTime - startTime).count(); diff --git a/platform/glfw/glfw_renderer_frontend.cpp b/platform/glfw/glfw_renderer_frontend.cpp index b8478a49f89..46f13099015 100644 --- a/platform/glfw/glfw_renderer_frontend.cpp +++ b/platform/glfw/glfw_renderer_frontend.cpp @@ -38,7 +38,7 @@ void GLFWRendererFrontend::render() { // Copy the shared pointer here so that the parameters aren't destroyed while `render(...)` is // still using them. auto updateParameters_ = updateParameters; - renderer->render(*updateParameters_); + renderer->render(updateParameters_); } mbgl::Renderer* GLFWRendererFrontend::getRenderer() { diff --git a/platform/qt/src/qmapboxgl_map_renderer.cpp b/platform/qt/src/qmapboxgl_map_renderer.cpp index 84234703237..7c7f5ee1511 100644 --- a/platform/qt/src/qmapboxgl_map_renderer.cpp +++ b/platform/qt/src/qmapboxgl_map_renderer.cpp @@ -84,7 +84,7 @@ void QMapboxGLMapRenderer::render() // The OpenGL implementation automatically enables the OpenGL context for us. mbgl::gfx::BackendScope scope(m_backend, mbgl::gfx::BackendScope::ScopeType::Implicit); - m_renderer->render(*params); + m_renderer->render(params); if (m_forceScheduler) { getScheduler()->processEvents(); diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index aa32acc04a5..239ad19876d 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -135,51 +135,49 @@ void RenderOrchestrator::setObserver(RendererObserver* observer_) { observer = observer_ ? observer_ : &nullObserver(); } -std::unique_ptr RenderOrchestrator::createRenderTree(const UpdateParameters& updateParameters) { - const bool isMapModeContinuous = updateParameters.mode == MapMode::Continuous; +std::unique_ptr RenderOrchestrator::createRenderTree( + const std::shared_ptr& updateParameters) { + const bool isMapModeContinuous = updateParameters->mode == MapMode::Continuous; if (!isMapModeContinuous) { // Reset zoom history state. zoomHistory.first = true; } if (LayerManager::annotationsEnabled) { - updateParameters.annotationManager.updateData(); + updateParameters->annotationManager.updateData(); } - const bool zoomChanged = zoomHistory.update(updateParameters.transformState.getZoom(), updateParameters.timePoint); + const bool zoomChanged = + zoomHistory.update(updateParameters->transformState.getZoom(), updateParameters->timePoint); - const TransitionOptions transitionOptions = isMapModeContinuous ? updateParameters.transitionOptions : TransitionOptions(); + const TransitionOptions transitionOptions = + isMapModeContinuous ? updateParameters->transitionOptions : TransitionOptions(); - const TransitionParameters transitionParameters { - updateParameters.timePoint, - transitionOptions - }; + const TransitionParameters transitionParameters{updateParameters->timePoint, transitionOptions}; - const PropertyEvaluationParameters evaluationParameters { + const PropertyEvaluationParameters evaluationParameters{ zoomHistory, - updateParameters.timePoint, - transitionOptions.duration.value_or(isMapModeContinuous ? util::DEFAULT_TRANSITION_DURATION : Duration::zero()) - }; - - const TileParameters tileParameters { - updateParameters.pixelRatio, - updateParameters.debugOptions, - updateParameters.transformState, - updateParameters.fileSource, - updateParameters.mode, - updateParameters.annotationManager, - *imageManager, - *glyphManager, - updateParameters.prefetchZoomDelta - }; - - glyphManager->setURL(updateParameters.glyphURL); + updateParameters->timePoint, + transitionOptions.duration.value_or(isMapModeContinuous ? util::DEFAULT_TRANSITION_DURATION + : Duration::zero())}; + + const TileParameters tileParameters{updateParameters->pixelRatio, + updateParameters->debugOptions, + updateParameters->transformState, + updateParameters->fileSource, + updateParameters->mode, + updateParameters->annotationManager, + *imageManager, + *glyphManager, + updateParameters->prefetchZoomDelta}; + + glyphManager->setURL(updateParameters->glyphURL); // Update light. - const bool lightChanged = renderLight.impl != updateParameters.light; + const bool lightChanged = renderLight.impl != updateParameters->light; if (lightChanged) { - renderLight.impl = updateParameters.light; + renderLight.impl = updateParameters->light; renderLight.transition(transitionParameters); } @@ -187,9 +185,8 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar renderLight.evaluate(evaluationParameters); } - - const ImageDifference imageDiff = diffImages(imageImpls, updateParameters.images); - imageImpls = updateParameters.images; + const ImageDifference imageDiff = diffImages(imageImpls, updateParameters->images); + imageImpls = updateParameters->images; // Only trigger tile reparse for changed images. Changed images only need a relayout when they have a different size. bool hasImageDiff = !imageDiff.removed.empty(); @@ -214,10 +211,10 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar } imageManager->notifyIfMissingImageAdded(); - imageManager->setLoaded(updateParameters.spriteLoaded); + imageManager->setLoaded(updateParameters->spriteLoaded); - const LayerDifference layerDiff = diffLayers(layerImpls, updateParameters.layers); - layerImpls = updateParameters.layers; + const LayerDifference layerDiff = diffLayers(layerImpls, updateParameters->layers); + layerImpls = updateParameters->layers; const bool layersAddedOrRemoved = !layerDiff.added.empty() || !layerDiff.removed.empty(); // Remove render layers for removed layers. @@ -266,8 +263,8 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar } } - const SourceDifference sourceDiff = diffSources(sourceImpls, updateParameters.sources); - sourceImpls = updateParameters.sources; + const SourceDifference sourceDiff = diffSources(sourceImpls, updateParameters->sources); + sourceImpls = updateParameters->sources; // Remove render layers for removed sources. for (const auto& entry : sourceDiff.removed) { @@ -280,15 +277,14 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar renderSource->setObserver(this); renderSources.emplace(entry.first, std::move(renderSource)); } - transformState = updateParameters.transformState; + transformState = updateParameters->transformState; // Create parameters for the render tree. - auto renderTreeParameters = std::make_unique( - updateParameters.transformState, - updateParameters.mode, - updateParameters.debugOptions, - updateParameters.timePoint, - renderLight.getEvaluated()); + auto renderTreeParameters = std::make_unique(updateParameters->transformState, + updateParameters->mode, + updateParameters->debugOptions, + updateParameters->timePoint, + renderLight.getEvaluated()); std::set layerRenderItems; layersNeedPlacement.clear(); @@ -349,7 +345,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar filteredLayersForSource.clear(); } - renderTreeParameters->loaded = updateParameters.styleLoaded && isLoaded(); + renderTreeParameters->loaded = updateParameters->styleLoaded && isLoaded(); if (!isMapModeContinuous && !renderTreeParameters->loaded) { return nullptr; } @@ -357,14 +353,16 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar // Prepare. Update all matrices and generate data that we should upload to the GPU. for (const auto& entry : renderSources) { if (entry.second->isEnabled()) { - entry.second->prepare({renderTreeParameters->transformParams, updateParameters.debugOptions, *imageManager}); + entry.second->prepare( + {renderTreeParameters->transformParams, updateParameters->debugOptions, *imageManager}); } } auto opaquePassCutOffEstimation = layerRenderItems.size(); for (auto& renderItem : layerRenderItems) { RenderLayer& renderLayer = renderItem.layer; - renderLayer.prepare({renderItem.source, *imageManager, *patternAtlas, *lineAtlas, updateParameters.transformState}); + renderLayer.prepare( + {renderItem.source, *imageManager, *patternAtlas, *lineAtlas, updateParameters->transformState}); if (renderLayer.needsPlacement()) { layersNeedPlacement.emplace_back(renderLayer); } @@ -380,7 +378,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar if (isMapModeContinuous) { bool symbolBucketsAdded = false; for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { - auto result = crossTileSymbolIndex.addLayer(*it, updateParameters.transformState.getLatLng().longitude()); + auto result = crossTileSymbolIndex.addLayer(*it, updateParameters->transformState.getLatLng().longitude()); symbolBucketsAdded = symbolBucketsAdded || (result & CrossTileSymbolIndex::AddLayerResult::BucketsAdded); symbolBucketsChanged = symbolBucketsChanged || (result != CrossTileSymbolIndex::AddLayerResult::NoChanges); } @@ -391,22 +389,24 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar optional maximumPlacementUpdatePeriod; if (symbolBucketsAdded) maximumPlacementUpdatePeriod = optional(Milliseconds(30)); renderTreeParameters->placementChanged = !placementController.placementIsRecent( - updateParameters.timePoint, updateParameters.transformState.getZoom(), maximumPlacementUpdatePeriod); + updateParameters->timePoint, updateParameters->transformState.getZoom(), maximumPlacementUpdatePeriod); symbolBucketsChanged |= renderTreeParameters->placementChanged; std::set usedSymbolLayers; if (renderTreeParameters->placementChanged) { - Mutable placement = makeMutable(updateParameters.transformState, - updateParameters.mode, - updateParameters.transitionOptions, - updateParameters.crossSourceCollisions, - updateParameters.timePoint, + Mutable placement = makeMutable(updateParameters->transformState, + updateParameters->mode, + updateParameters->transitionOptions, + updateParameters->crossSourceCollisions, + updateParameters->timePoint, placementController.getPlacement()); for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; usedSymbolLayers.insert(layer.getID()); - placement->placeLayer(layer, renderTreeParameters->transformParams.projMatrix, updateParameters.debugOptions & MapDebugOptions::Collision); + placement->placeLayer(layer, + renderTreeParameters->transformParams.projMatrix, + updateParameters->debugOptions & MapDebugOptions::Collision); } placement->commit(); @@ -419,23 +419,23 @@ std::unique_ptr RenderOrchestrator::createRenderTree(const UpdatePar placementController.setPlacementStale(); } renderTreeParameters->symbolFadeChange = - placementController.getPlacement()->symbolFadeChange(updateParameters.timePoint); - renderTreeParameters->needsRepaint = hasTransitions(updateParameters.timePoint); + placementController.getPlacement()->symbolFadeChange(updateParameters->timePoint); + renderTreeParameters->needsRepaint = hasTransitions(updateParameters->timePoint); } else { crossTileSymbolIndex.reset(); renderTreeParameters->placementChanged = symbolBucketsChanged = !layersNeedPlacement.empty(); if (renderTreeParameters->placementChanged) { - Mutable placement = makeMutable(updateParameters.transformState, - updateParameters.mode, - updateParameters.transitionOptions, - updateParameters.crossSourceCollisions, - updateParameters.timePoint); + Mutable placement = makeMutable(updateParameters->transformState, + updateParameters->mode, + updateParameters->transitionOptions, + updateParameters->crossSourceCollisions, + updateParameters->timePoint); for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; - crossTileSymbolIndex.addLayer(layer, updateParameters.transformState.getLatLng().longitude()); + crossTileSymbolIndex.addLayer(layer, updateParameters->transformState.getLatLng().longitude()); placement->placeLayer(layer, renderTreeParameters->transformParams.projMatrix, - updateParameters.debugOptions & MapDebugOptions::Collision); + updateParameters->debugOptions & MapDebugOptions::Collision); } placement->commit(); placementController.setPlacement(std::move(placement)); diff --git a/src/mbgl/renderer/render_orchestrator.hpp b/src/mbgl/renderer/render_orchestrator.hpp index c2b44c27928..49835387497 100644 --- a/src/mbgl/renderer/render_orchestrator.hpp +++ b/src/mbgl/renderer/render_orchestrator.hpp @@ -52,7 +52,7 @@ class RenderOrchestrator final : public GlyphManagerObserver, // TODO: Introduce RenderOrchestratorObserver. void setObserver(RendererObserver*); - std::unique_ptr createRenderTree(const UpdateParameters&); + std::unique_ptr createRenderTree(const std::shared_ptr&); std::vector queryRenderedFeatures(const ScreenLineString&, const RenderedQueryOptions&) const; std::vector querySourceFeatures(const std::string& sourceID, const SourceQueryOptions&) const; diff --git a/src/mbgl/renderer/renderer.cpp b/src/mbgl/renderer/renderer.cpp index a74a21030e6..5afbbbd47ee 100644 --- a/src/mbgl/renderer/renderer.cpp +++ b/src/mbgl/renderer/renderer.cpp @@ -25,7 +25,8 @@ void Renderer::setObserver(RendererObserver* observer) { impl->orchestrator.setObserver(observer); } -void Renderer::render(const UpdateParameters& updateParameters) { +void Renderer::render(const std::shared_ptr& updateParameters) { + assert(updateParameters); if (auto renderTree = impl->orchestrator.createRenderTree(updateParameters)) { renderTree->prepare(); impl->render(*renderTree); From 43011ad807be00719f0fd56c317a1fd992661070 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 16 Jan 2020 12:05:11 +0200 Subject: [PATCH 04/11] [core] Pass UpdateParameters to Placement constructor --- src/mbgl/renderer/render_orchestrator.cpp | 13 ++-------- src/mbgl/text/collision_index.cpp | 2 +- src/mbgl/text/collision_index.hpp | 2 +- src/mbgl/text/placement.cpp | 30 +++++++++++------------ src/mbgl/text/placement.hpp | 18 +++++++------- 5 files changed, 27 insertions(+), 38 deletions(-) diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index 239ad19876d..c77eaca37e1 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -394,12 +394,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree( std::set usedSymbolLayers; if (renderTreeParameters->placementChanged) { - Mutable placement = makeMutable(updateParameters->transformState, - updateParameters->mode, - updateParameters->transitionOptions, - updateParameters->crossSourceCollisions, - updateParameters->timePoint, - placementController.getPlacement()); + Mutable placement = makeMutable(updateParameters, placementController.getPlacement()); for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; @@ -425,11 +420,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree( crossTileSymbolIndex.reset(); renderTreeParameters->placementChanged = symbolBucketsChanged = !layersNeedPlacement.empty(); if (renderTreeParameters->placementChanged) { - Mutable placement = makeMutable(updateParameters->transformState, - updateParameters->mode, - updateParameters->transitionOptions, - updateParameters->crossSourceCollisions, - updateParameters->timePoint); + Mutable placement = makeMutable(updateParameters); for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; crossTileSymbolIndex.addLayer(layer, updateParameters->transformState.getLatLng().longitude()); diff --git a/src/mbgl/text/collision_index.cpp b/src/mbgl/text/collision_index.cpp index 50567ab8954..e75bdf8ba9e 100644 --- a/src/mbgl/text/collision_index.cpp +++ b/src/mbgl/text/collision_index.cpp @@ -26,7 +26,7 @@ static const float viewportPaddingDefault = 100; // Viewport padding must be much larger for static tiles to avoid clipped labels. static const float viewportPaddingForStaticTiles = 1024; -CollisionIndex::CollisionIndex(const TransformState& transformState_, MapMode& mapMode) +CollisionIndex::CollisionIndex(const TransformState& transformState_, MapMode mapMode) : transformState(transformState_), viewportPadding(mapMode == MapMode::Tile ? viewportPaddingForStaticTiles : viewportPaddingDefault), collisionGrid(transformState.getSize().width + 2 * viewportPadding, diff --git a/src/mbgl/text/collision_index.hpp b/src/mbgl/text/collision_index.hpp index d1a77b5492a..b9f3e9d88a0 100644 --- a/src/mbgl/text/collision_index.hpp +++ b/src/mbgl/text/collision_index.hpp @@ -20,7 +20,7 @@ class CollisionIndex { public: using CollisionGrid = GridIndex; - explicit CollisionIndex(const TransformState&, MapMode&); + explicit CollisionIndex(const TransformState&, MapMode); bool featureIntersectsTileBorders(const CollisionFeature& feature, Point shift, const mat4& posMatrix, diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index f3917a8fdbd..5673ff5041a 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -1,11 +1,12 @@ #include #include +#include +#include #include #include +#include #include -#include -#include #include #include @@ -60,9 +61,7 @@ const CollisionGroups::CollisionGroup& CollisionGroups::get(const std::string& s // PlacementController implemenation -PlacementController::PlacementController() - : placement(makeMutable( - TransformState{}, MapMode::Static, style::TransitionOptions{}, true, TimePoint(), nullopt)) {} +PlacementController::PlacementController() : placement(makeMutable()) {} void PlacementController::setPlacement(Immutable placement_) { placement = std::move(placement_); @@ -91,18 +90,15 @@ bool PlacementController::hasTransitions(TimePoint now) const { // Placement implementation -Placement::Placement(const TransformState& state_, - MapMode mapMode_, - style::TransitionOptions transitionOptions_, - const bool crossSourceCollisions, - TimePoint commitTime_, +Placement::Placement(std::shared_ptr updateParameters_, optional> prevPlacement_) - : collisionIndex(state_, mapMode_), - mapMode(mapMode_), - transitionOptions(std::move(transitionOptions_)), - commitTime(commitTime_), - placementZoom(state_.getZoom()), - collisionGroups(crossSourceCollisions), + : updateParameters(std::move(updateParameters_)), + collisionIndex(updateParameters->transformState, updateParameters->mode), + mapMode(updateParameters->mode), + transitionOptions(updateParameters->transitionOptions), + commitTime(updateParameters->timePoint), + placementZoom(updateParameters->transformState.getZoom()), + collisionGroups(updateParameters->crossSourceCollisions), prevPlacement(std::move(prevPlacement_)) { assert(prevPlacement || mapMode != MapMode::Continuous); if (prevPlacement) { @@ -110,6 +106,8 @@ Placement::Placement(const TransformState& state_, } } +Placement::Placement() : collisionIndex({}, MapMode::Static), collisionGroups(true) {} + void Placement::placeLayer(const RenderLayer& layer, const mat4& projMatrix, bool showCollisionBoxes) { std::set seenCrossTileIDs; for (const auto& item : layer.getPlacementData()) { diff --git a/src/mbgl/text/placement.hpp b/src/mbgl/text/placement.hpp index dd46970780d..04a0bc7f5d9 100644 --- a/src/mbgl/text/placement.hpp +++ b/src/mbgl/text/placement.hpp @@ -12,6 +12,7 @@ namespace mbgl { class SymbolBucket; class SymbolInstance; +class UpdateParameters; enum class PlacedSymbolOrientation : bool; class OpacityState { @@ -112,12 +113,9 @@ class PlacementController { class Placement { public: - Placement(const TransformState&, - MapMode, - style::TransitionOptions, - const bool crossSourceCollisions, - TimePoint commitTime, - optional> prevPlacement = nullopt); + Placement(std::shared_ptr, optional> prevPlacement = nullopt); + Placement(); + void placeLayer(const RenderLayer&, const mat4&, bool showCollisionBoxes); void commit(); void updateLayerBuckets(const RenderLayer&, const TransformState&, bool updateOpacities) const; @@ -132,6 +130,7 @@ class Placement { float zoomAdjustment(const float zoom) const; const RetainedQueryData& getQueryData(uint32_t bucketInstanceId) const; + private: friend SymbolBucket; void placeBucket(const SymbolBucket&, const BucketPlacementParameters&, std::set& seenCrossTileIDs); @@ -145,15 +144,16 @@ class Placement { void markUsedOrientation(SymbolBucket&, style::TextWritingModeType, const SymbolInstance&) const; const Placement* getPrevPlacement() const { return prevPlacement ? prevPlacement->get() : nullptr; } + std::shared_ptr updateParameters; CollisionIndex collisionIndex; - MapMode mapMode; + MapMode mapMode = MapMode::Static; style::TransitionOptions transitionOptions; TimePoint fadeStartTime; TimePoint commitTime; - const float placementZoom; - float prevZoomAdjustment = 0; + float placementZoom = 0.0f; + float prevZoomAdjustment = 0.0f; std::unordered_map placements; std::unordered_map opacities; From d0fa7824ff64a1589aa5eeba8de36ec1e47d29f4 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 16 Jan 2020 12:29:56 +0200 Subject: [PATCH 05/11] [core] Cache showCollisionBoxes in Placement --- src/mbgl/renderer/render_orchestrator.cpp | 8 ++------ src/mbgl/text/placement.cpp | 21 +++++++++------------ src/mbgl/text/placement.hpp | 4 ++-- 3 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index c77eaca37e1..5e5f2da3c33 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -399,9 +399,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree( for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; usedSymbolLayers.insert(layer.getID()); - placement->placeLayer(layer, - renderTreeParameters->transformParams.projMatrix, - updateParameters->debugOptions & MapDebugOptions::Collision); + placement->placeLayer(layer, renderTreeParameters->transformParams.projMatrix); } placement->commit(); @@ -424,9 +422,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree( for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; crossTileSymbolIndex.addLayer(layer, updateParameters->transformState.getLatLng().longitude()); - placement->placeLayer(layer, - renderTreeParameters->transformParams.projMatrix, - updateParameters->debugOptions & MapDebugOptions::Collision); + placement->placeLayer(layer, renderTreeParameters->transformParams.projMatrix); } placement->commit(); placementController.setPlacement(std::move(placement)); diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index 5673ff5041a..edd6506c2c7 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -99,7 +99,8 @@ Placement::Placement(std::shared_ptr updateParameters_, commitTime(updateParameters->timePoint), placementZoom(updateParameters->transformState.getZoom()), collisionGroups(updateParameters->crossSourceCollisions), - prevPlacement(std::move(prevPlacement_)) { + prevPlacement(std::move(prevPlacement_)), + showCollisionBoxes(updateParameters->debugOptions & MapDebugOptions::Collision) { assert(prevPlacement || mapMode != MapMode::Continuous); if (prevPlacement) { prevPlacement->get()->prevPlacement = nullopt; // Only hold on to one placement back @@ -108,16 +109,11 @@ Placement::Placement(std::shared_ptr updateParameters_, Placement::Placement() : collisionIndex({}, MapMode::Static), collisionGroups(true) {} -void Placement::placeLayer(const RenderLayer& layer, const mat4& projMatrix, bool showCollisionBoxes) { +void Placement::placeLayer(const RenderLayer& layer, const mat4& projMatrix) { std::set seenCrossTileIDs; for (const auto& item : layer.getPlacementData()) { Bucket& bucket = item.bucket; - BucketPlacementParameters params{ - item.tile, - projMatrix, - layer.baseImpl->source, - item.featureIndex, - showCollisionBoxes}; + BucketPlacementParameters params{item.tile, projMatrix, layer.baseImpl->source, item.featureIndex}; bucket.place(*this, params, seenCrossTileIDs); } } @@ -146,6 +142,7 @@ Point calculateVariableLayoutOffset(style::SymbolAnchorType anchor, void Placement::placeBucket(const SymbolBucket& bucket, const BucketPlacementParameters& params, std::set& seenCrossTileIDs) { + assert(updateParameters); const auto& layout = *bucket.layout; const auto& renderTile = params.tile; const auto& state = collisionIndex.getTransformState(); @@ -278,7 +275,7 @@ void Placement::placeBucket(const SymbolBucket& bucket, fontSize, layout.get(), pitchWithMap, - params.showCollisionBoxes, + showCollisionBoxes, avoidEdges, collisionGroup.second, textBoxes); @@ -376,7 +373,7 @@ void Placement::placeBucket(const SymbolBucket& bucket, fontSize, allowOverlap, pitchWithMap, - params.showCollisionBoxes, + showCollisionBoxes, avoidEdges, collisionGroup.second, textBoxes); @@ -392,7 +389,7 @@ void Placement::placeBucket(const SymbolBucket& bucket, fontSize, iconAllowOverlap, pitchWithMap, - params.showCollisionBoxes, + showCollisionBoxes, avoidEdges, collisionGroup.second, iconBoxes); @@ -490,7 +487,7 @@ void Placement::placeBucket(const SymbolBucket& bucket, fontSize, layout.get(), pitchWithMap, - params.showCollisionBoxes, + showCollisionBoxes, avoidEdges, collisionGroup.second, iconBoxes); diff --git a/src/mbgl/text/placement.hpp b/src/mbgl/text/placement.hpp index 04a0bc7f5d9..0d224f67de4 100644 --- a/src/mbgl/text/placement.hpp +++ b/src/mbgl/text/placement.hpp @@ -92,7 +92,6 @@ class BucketPlacementParameters { const mat4& projMatrix; std::string sourceId; std::shared_ptr featureIndex; - bool showCollisionBoxes; }; class Placement; @@ -116,7 +115,7 @@ class Placement { Placement(std::shared_ptr, optional> prevPlacement = nullopt); Placement(); - void placeLayer(const RenderLayer&, const mat4&, bool showCollisionBoxes); + void placeLayer(const RenderLayer&, const mat4&); void commit(); void updateLayerBuckets(const RenderLayer&, const TransformState&, bool updateOpacities) const; float symbolFadeChange(TimePoint now) const; @@ -163,6 +162,7 @@ class Placement { std::unordered_map retainedQueryData; CollisionGroups collisionGroups; mutable optional> prevPlacement; + bool showCollisionBoxes = false; // Used for debug purposes. std::unordered_map> collisionCircles; From 6fe66c08d2a73bc2def2814ae7def394eb0770ae Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 16 Jan 2020 12:57:27 +0200 Subject: [PATCH 06/11] [core] Avoid repeated calculations in symbol placement --- src/mbgl/renderer/render_orchestrator.cpp | 5 +- src/mbgl/text/placement.cpp | 84 ++++++++++++----------- 2 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index 5e5f2da3c33..48e7b098127 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -375,10 +375,11 @@ std::unique_ptr RenderOrchestrator::createRenderTree( } // Symbol placement. bool symbolBucketsChanged = false; + auto longitude = updateParameters->transformState.getLatLng().longitude(); if (isMapModeContinuous) { bool symbolBucketsAdded = false; for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { - auto result = crossTileSymbolIndex.addLayer(*it, updateParameters->transformState.getLatLng().longitude()); + auto result = crossTileSymbolIndex.addLayer(*it, longitude); symbolBucketsAdded = symbolBucketsAdded || (result & CrossTileSymbolIndex::AddLayerResult::BucketsAdded); symbolBucketsChanged = symbolBucketsChanged || (result != CrossTileSymbolIndex::AddLayerResult::NoChanges); } @@ -421,7 +422,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree( Mutable placement = makeMutable(updateParameters); for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; - crossTileSymbolIndex.addLayer(layer, updateParameters->transformState.getLatLng().longitude()); + crossTileSymbolIndex.addLayer(layer, longitude); placement->placeLayer(layer, renderTreeParameters->transformParams.projMatrix); } placement->commit(); diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index edd6506c2c7..dce1cb40dc6 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -146,31 +146,35 @@ void Placement::placeBucket(const SymbolBucket& bucket, const auto& layout = *bucket.layout; const auto& renderTile = params.tile; const auto& state = collisionIndex.getTransformState(); - const float pixelsToTileUnits = renderTile.id.pixelsToTileUnits(1, state.getZoom()); + const float pixelsToTileUnits = renderTile.id.pixelsToTileUnits(1, placementZoom); const OverscaledTileID& overscaledID = renderTile.getOverscaledTileID(); - const float scale = std::pow(2, state.getZoom() - overscaledID.overscaledZ); + const float scale = std::pow(2, placementZoom - overscaledID.overscaledZ); const float pixelRatio = (util::tileSize * overscaledID.overscaleFactor()) / util::EXTENT; + const bool rotateTextWithMap = layout.get() == style::AlignmentType::Map; + const bool pitchTextWithMap = layout.get() == style::AlignmentType::Map; + + const bool rotateIconWithMap = layout.get() == style::AlignmentType::Map; + const bool pitchIconWithMap = layout.get() == style::AlignmentType::Map; + mat4 posMatrix; state.matrixFor(posMatrix, renderTile.id); matrix::multiply(posMatrix, params.projMatrix, posMatrix); mat4 textLabelPlaneMatrix = - getLabelPlaneMatrix(posMatrix, - layout.get() == style::AlignmentType::Map, - layout.get() == style::AlignmentType::Map, - state, - pixelsToTileUnits); - - mat4 iconLabelPlaneMatrix = getLabelPlaneMatrix(posMatrix, - layout.get() == style::AlignmentType::Map, - layout.get() == style::AlignmentType::Map, - state, - pixelsToTileUnits); + getLabelPlaneMatrix(posMatrix, pitchTextWithMap, rotateTextWithMap, state, pixelsToTileUnits); + + mat4 iconLabelPlaneMatrix; + if (rotateTextWithMap == rotateIconWithMap && pitchTextWithMap == pitchIconWithMap) { + iconLabelPlaneMatrix = textLabelPlaneMatrix; + } else { + iconLabelPlaneMatrix = + getLabelPlaneMatrix(posMatrix, pitchIconWithMap, rotateIconWithMap, state, pixelsToTileUnits); + } const auto& collisionGroup = collisionGroups.get(params.sourceId); - auto partiallyEvaluatedTextSize = bucket.textSizeBinder->evaluateForZoom(state.getZoom()); - auto partiallyEvaluatedIconSize = bucket.iconSizeBinder->evaluateForZoom(state.getZoom()); + auto partiallyEvaluatedTextSize = bucket.textSizeBinder->evaluateForZoom(placementZoom); + auto partiallyEvaluatedIconSize = bucket.iconSizeBinder->evaluateForZoom(placementZoom); optional tileBorders; optional avoidEdges; @@ -200,8 +204,7 @@ void Placement::placeBucket(const SymbolBucket& bucket, const bool alwaysShowText = textAllowOverlap && (iconAllowOverlap || !(bucket.hasIconData() || bucket.hasSdfIconData()) || layout.get()); const bool alwaysShowIcon = iconAllowOverlap && (textAllowOverlap || !bucket.hasTextData() || layout.get()); std::vector variableTextAnchors = layout.get(); - const bool rotateWithMap = layout.get() == style::AlignmentType::Map; - const bool pitchWithMap = layout.get() == style::AlignmentType::Map; + const bool hasIconTextFit = layout.get() != style::IconTextFitType::None; const bool zOrderByViewportY = layout.get() == style::SymbolZOrderType::ViewportY; @@ -273,8 +276,8 @@ void Placement::placeBucket(const SymbolBucket& bucket, placedSymbol, scale, fontSize, - layout.get(), - pitchWithMap, + textAllowOverlap, + pitchTextWithMap, showCollisionBoxes, avoidEdges, collisionGroup.second, @@ -352,8 +355,8 @@ void Placement::placeBucket(const SymbolBucket& bucket, height, symbolInstance.variableTextOffset, textBoxScale, - rotateWithMap, - pitchWithMap, + rotateTextWithMap, + pitchTextWithMap, state.getBearing()); textBoxes.clear(); @@ -372,27 +375,28 @@ void Placement::placeBucket(const SymbolBucket& bucket, scale, fontSize, allowOverlap, - pitchWithMap, + pitchTextWithMap, showCollisionBoxes, avoidEdges, collisionGroup.second, textBoxes); if (doVariableIconPlacement) { - auto placedIconFeature = collisionIndex.placeFeature(iconCollisionFeature, - shift, - posMatrix, - iconLabelPlaneMatrix, - pixelRatio, - placedSymbol, - scale, - fontSize, - iconAllowOverlap, - pitchWithMap, - showCollisionBoxes, - avoidEdges, - collisionGroup.second, - iconBoxes); + auto placedIconFeature = + collisionIndex.placeFeature(iconCollisionFeature, + shift, + posMatrix, + iconLabelPlaneMatrix, + pixelRatio, + placedSymbol, + scale, + fontSize, + iconAllowOverlap, + pitchTextWithMap, // TODO: shall it be pitchIconWithMap? + showCollisionBoxes, + avoidEdges, + collisionGroup.second, + iconBoxes); iconBoxes.clear(); if (!placedIconFeature.first) continue; } @@ -485,8 +489,8 @@ void Placement::placeBucket(const SymbolBucket& bucket, placedSymbol, scale, fontSize, - layout.get(), - pitchWithMap, + iconAllowOverlap, + pitchTextWithMap, showCollisionBoxes, avoidEdges, collisionGroup.second, @@ -586,8 +590,8 @@ void Placement::placeBucket(const SymbolBucket& bucket, height, symbol.variableTextOffset, symbol.textBoxScale, - rotateWithMap, - pitchWithMap, + rotateTextWithMap, + pitchTextWithMap, state.getBearing()); } bool result = collisionIndex.featureIntersectsTileBorders( From 13c7770c049d69e07eb053a827a2460654a9e68a Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 16 Jan 2020 20:12:13 +0200 Subject: [PATCH 07/11] [core] Simplify CrossTileSymbolIndex::addLayer() --- src/mbgl/renderer/bucket.hpp | 2 +- src/mbgl/renderer/buckets/symbol_bucket.cpp | 5 ++-- src/mbgl/renderer/buckets/symbol_bucket.hpp | 2 +- src/mbgl/text/cross_tile_symbol_index.cpp | 16 ++++++++--- src/mbgl/text/cross_tile_symbol_index.hpp | 5 ++-- test/text/cross_tile_symbol_index.test.cpp | 30 ++++++++++----------- 6 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/mbgl/renderer/bucket.hpp b/src/mbgl/renderer/bucket.hpp index 6da0f280e65..3e756a06a46 100644 --- a/src/mbgl/renderer/bucket.hpp +++ b/src/mbgl/renderer/bucket.hpp @@ -56,7 +56,7 @@ class Bucket { // Returns a pair, the first element of which is a bucket cross-tile id // on success call; `0` otherwise. The second element is `true` if // the bucket was originally registered; `false` otherwise. - virtual std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&, uint32_t&) { + virtual std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&) { return std::make_pair(0u, false); } // Places this bucket to the given placement. diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp index 7efd81053ed..382aac6563a 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.cpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp @@ -295,8 +295,9 @@ bool SymbolBucket::hasFormatSectionOverrides() const { return *hasFormatSectionOverrides_; } -std::pair SymbolBucket::registerAtCrossTileIndex(CrossTileSymbolLayerIndex& index, const OverscaledTileID& tileID, uint32_t& maxCrossTileID) { - bool firstTimeAdded = index.addBucket(tileID, *this, maxCrossTileID); +std::pair SymbolBucket::registerAtCrossTileIndex(CrossTileSymbolLayerIndex& index, + const OverscaledTileID& tileID) { + bool firstTimeAdded = index.addBucket(tileID, *this); return std::make_pair(bucketInstanceId, firstTimeAdded); } diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp index 1d1aa13dd00..432589a52bc 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.hpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp @@ -81,7 +81,7 @@ class SymbolBucket final : public Bucket { void upload(gfx::UploadPass&) override; bool hasData() const override; - std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&, uint32_t& maxCrossTileID) override; + std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&) override; void place(Placement&, const BucketPlacementParameters&, std::set&) override; void updateVertices( const Placement&, bool updateOpacities, const TransformState&, const RenderTile&, std::set&) override; diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp index d8bd0292ec5..5b9d6ae69ce 100644 --- a/src/mbgl/text/cross_tile_symbol_index.cpp +++ b/src/mbgl/text/cross_tile_symbol_index.cpp @@ -67,7 +67,7 @@ void TileLayerIndex::findMatches(SymbolBucket& bucket, } } -CrossTileSymbolLayerIndex::CrossTileSymbolLayerIndex() = default; +CrossTileSymbolLayerIndex::CrossTileSymbolLayerIndex(uint32_t& maxCrossTileID_) : maxCrossTileID(maxCrossTileID_) {} /* * Sometimes when a user pans across the antimeridian the longitude value gets wrapped. @@ -95,7 +95,7 @@ void CrossTileSymbolLayerIndex::handleWrapJump(float newLng) { lng = newLng; } -bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, SymbolBucket& bucket, uint32_t& maxCrossTileID) { +bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, SymbolBucket& bucket) { auto& thisZoomIndexes = indexes[tileID.overscaledZ]; auto previousIndex = thisZoomIndexes.find(tileID); if (previousIndex != thisZoomIndexes.end()) { @@ -177,7 +177,15 @@ bool CrossTileSymbolLayerIndex::removeStaleBuckets(const std::unordered_set AddLayerResult { - auto& layerIndex = layerIndexes[layer.getID()]; + auto found = layerIndexes.find(layer.getID()); + if (found == layerIndexes.end()) { + found = layerIndexes + .emplace(std::piecewise_construct, + std::forward_as_tuple(layer.getID()), + std::forward_as_tuple(maxCrossTileID)) + .first; + } + auto& layerIndex = found->second; AddLayerResult result = AddLayerResult::NoChanges; std::unordered_set currentBucketIDs; @@ -187,7 +195,7 @@ auto CrossTileSymbolIndex::addLayer(const RenderLayer& layer, float lng) -> AddL for (const auto& item : layer.getPlacementData()) { const RenderTile& renderTile = item.tile; Bucket& bucket = item.bucket; - auto pair = bucket.registerAtCrossTileIndex(layerIndex, renderTile.getOverscaledTileID(), maxCrossTileID); + auto pair = bucket.registerAtCrossTileIndex(layerIndex, renderTile.getOverscaledTileID()); assert(pair.first != 0u); if (pair.second) result |= AddLayerResult::BucketsAdded; currentBucketIDs.insert(pair.first); diff --git a/src/mbgl/text/cross_tile_symbol_index.hpp b/src/mbgl/text/cross_tile_symbol_index.hpp index ddcdb814c61..3b7c3677269 100644 --- a/src/mbgl/text/cross_tile_symbol_index.hpp +++ b/src/mbgl/text/cross_tile_symbol_index.hpp @@ -47,8 +47,8 @@ class TileLayerIndex { class CrossTileSymbolLayerIndex { public: - CrossTileSymbolLayerIndex(); - bool addBucket(const OverscaledTileID&, SymbolBucket&, uint32_t& maxCrossTileID); + CrossTileSymbolLayerIndex(uint32_t& maxCrossTileID); + bool addBucket(const OverscaledTileID&, SymbolBucket&); bool removeStaleBuckets(const std::unordered_set& currentIDs); void handleWrapJump(float newLng); private: @@ -57,6 +57,7 @@ class CrossTileSymbolLayerIndex { std::map> indexes; std::map> usedCrossTileIDs; float lng = 0; + uint32_t& maxCrossTileID; }; class CrossTileSymbolIndex { diff --git a/test/text/cross_tile_symbol_index.test.cpp b/test/text/cross_tile_symbol_index.test.cpp index fe1375c66db..f0c78ce6329 100644 --- a/test/text/cross_tile_symbol_index.test.cpp +++ b/test/text/cross_tile_symbol_index.test.cpp @@ -36,7 +36,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { uint32_t maxCrossTileID = 0; uint32_t maxBucketInstanceId = 0; - CrossTileSymbolLayerIndex index; + CrossTileSymbolLayerIndex index(maxCrossTileID); Immutable layout = makeMutable(); @@ -62,7 +62,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; mainBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(mainID, mainBucket, maxCrossTileID); + index.addBucket(mainID, mainBucket); // Assigned new IDs ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); @@ -89,7 +89,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; childBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(childID, childBucket, maxCrossTileID); + index.addBucket(childID, childBucket); // matched parent tile ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 1u); @@ -117,7 +117,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; parentBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(parentID, parentBucket, maxCrossTileID); + index.addBucket(parentID, parentBucket); // matched child tile ASSERT_EQ(parentBucket.symbolInstances.at(0).crossTileID, 1u); @@ -145,7 +145,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; grandchildBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(grandchildID, grandchildBucket, maxCrossTileID); + index.addBucket(grandchildID, grandchildBucket); // Matches the symbol in `mainBucket` ASSERT_EQ(grandchildBucket.symbolInstances.at(0).crossTileID, 1u); @@ -158,7 +158,7 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { uint32_t maxCrossTileID = 0; uint32_t maxBucketInstanceId = 0; - CrossTileSymbolLayerIndex index; + CrossTileSymbolLayerIndex index(maxCrossTileID); Immutable layout = makeMutable(); @@ -203,7 +203,7 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { childBucket.bucketInstanceId = ++maxBucketInstanceId; // assigns a new id - index.addBucket(mainID, mainBucket, maxCrossTileID); + index.addBucket(mainID, mainBucket); ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); // removes the tile @@ -211,18 +211,18 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { index.removeStaleBuckets(currentIDs); // assigns a new id - index.addBucket(childID, childBucket, maxCrossTileID); + index.addBucket(childID, childBucket); ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 2u); // overwrites the old id to match the already-added tile - index.addBucket(mainID, mainBucket, maxCrossTileID); + index.addBucket(mainID, mainBucket); ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 2u); } TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) { uint32_t maxCrossTileID = 0; uint32_t maxBucketInstanceId = 0; - CrossTileSymbolLayerIndex index; + CrossTileSymbolLayerIndex index(maxCrossTileID); Immutable layout = makeMutable(); @@ -270,12 +270,12 @@ TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) { childBucket.bucketInstanceId = ++maxBucketInstanceId; // assigns new ids - index.addBucket(mainID, mainBucket, maxCrossTileID); + index.addBucket(mainID, mainBucket); ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); ASSERT_EQ(mainBucket.symbolInstances.at(1).crossTileID, 2u); // copies parent ids without duplicate ids in this tile - index.addBucket(childID, childBucket, maxCrossTileID); + index.addBucket(childID, childBucket); ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 1u); // A' copies from A ASSERT_EQ(childBucket.symbolInstances.at(1).crossTileID, 2u); // B' copies from B ASSERT_EQ(childBucket.symbolInstances.at(2).crossTileID, 3u); // C' gets new ID @@ -284,7 +284,7 @@ TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) { TEST(CrossTileSymbolLayerIndex, bucketReplacement) { uint32_t maxCrossTileID = 0; uint32_t maxBucketInstanceId = 0; - CrossTileSymbolLayerIndex index; + CrossTileSymbolLayerIndex index(maxCrossTileID); Immutable layout = makeMutable(); @@ -331,12 +331,12 @@ TEST(CrossTileSymbolLayerIndex, bucketReplacement) { secondBucket.bucketInstanceId = ++maxBucketInstanceId; // assigns new ids - index.addBucket(tileID, firstBucket, maxCrossTileID); + index.addBucket(tileID, firstBucket); ASSERT_EQ(firstBucket.symbolInstances.at(0).crossTileID, 1u); ASSERT_EQ(firstBucket.symbolInstances.at(1).crossTileID, 2u); // copies parent ids without duplicate ids in this tile - index.addBucket(tileID, secondBucket, maxCrossTileID); + index.addBucket(tileID, secondBucket); ASSERT_EQ(secondBucket.symbolInstances.at(0).crossTileID, 1u); // A' copies from A ASSERT_EQ(secondBucket.symbolInstances.at(1).crossTileID, 2u); // B' copies from B ASSERT_EQ(secondBucket.symbolInstances.at(2).crossTileID, 3u); // C' gets new ID From 8da64ebeb7ef2d9253856e7533eb8422ece74436 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Sun, 19 Jan 2020 21:28:59 +0200 Subject: [PATCH 08/11] [core] Simplify Placement::addLayer() --- src/mbgl/map/transform_state.hpp | 3 ++- src/mbgl/renderer/render_orchestrator.cpp | 6 +++--- src/mbgl/text/placement.cpp | 7 ++++--- src/mbgl/text/placement.hpp | 3 +-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/mbgl/map/transform_state.hpp b/src/mbgl/map/transform_state.hpp index edc7f3b9cf8..8597f3b5df8 100644 --- a/src/mbgl/map/transform_state.hpp +++ b/src/mbgl/map/transform_state.hpp @@ -210,6 +210,7 @@ class TransformState { void setLatLngZoom(const LatLng& latLng, double zoom); void constrain(double& scale, double& x, double& y) const; + const mat4& getProjectionMatrix() const; private: bool rotatedNorth() const; @@ -236,7 +237,7 @@ class TransformState { void updateMatricesIfNeeded() const; bool needsMatricesUpdate() const { return requestMatricesUpdate; } - const mat4& getProjectionMatrix() const; + const mat4& getCoordMatrix() const; const mat4& getInvertedMatrix() const; diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index 48e7b098127..79059164ae9 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -400,7 +400,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree( for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; usedSymbolLayers.insert(layer.getID()); - placement->placeLayer(layer, renderTreeParameters->transformParams.projMatrix); + placement->placeLayer(layer); } placement->commit(); @@ -422,8 +422,8 @@ std::unique_ptr RenderOrchestrator::createRenderTree( Mutable placement = makeMutable(updateParameters); for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; - crossTileSymbolIndex.addLayer(layer, longitude); - placement->placeLayer(layer, renderTreeParameters->transformParams.projMatrix); + crossTileSymbolIndex.addLayer(layer, updateParameters->transformState); + placement->placeLayer(layer); } placement->commit(); placementController.setPlacement(std::move(placement)); diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index dce1cb40dc6..43d97137173 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -109,11 +109,11 @@ Placement::Placement(std::shared_ptr updateParameters_, Placement::Placement() : collisionIndex({}, MapMode::Static), collisionGroups(true) {} -void Placement::placeLayer(const RenderLayer& layer, const mat4& projMatrix) { +void Placement::placeLayer(const RenderLayer& layer) { std::set seenCrossTileIDs; for (const auto& item : layer.getPlacementData()) { Bucket& bucket = item.bucket; - BucketPlacementParameters params{item.tile, projMatrix, layer.baseImpl->source, item.featureIndex}; + BucketPlacementParameters params{item.tile, layer.baseImpl->source, item.featureIndex}; bucket.place(*this, params, seenCrossTileIDs); } } @@ -158,8 +158,9 @@ void Placement::placeBucket(const SymbolBucket& bucket, const bool pitchIconWithMap = layout.get() == style::AlignmentType::Map; mat4 posMatrix; + const auto& projMatrix = state.getProjectionMatrix(); state.matrixFor(posMatrix, renderTile.id); - matrix::multiply(posMatrix, params.projMatrix, posMatrix); + matrix::multiply(posMatrix, projMatrix, posMatrix); mat4 textLabelPlaneMatrix = getLabelPlaneMatrix(posMatrix, pitchTextWithMap, rotateTextWithMap, state, pixelsToTileUnits); diff --git a/src/mbgl/text/placement.hpp b/src/mbgl/text/placement.hpp index 0d224f67de4..46d560b6735 100644 --- a/src/mbgl/text/placement.hpp +++ b/src/mbgl/text/placement.hpp @@ -89,7 +89,6 @@ class CollisionGroups { class BucketPlacementParameters { public: const RenderTile& tile; - const mat4& projMatrix; std::string sourceId; std::shared_ptr featureIndex; }; @@ -115,7 +114,7 @@ class Placement { Placement(std::shared_ptr, optional> prevPlacement = nullopt); Placement(); - void placeLayer(const RenderLayer&, const mat4&); + void placeLayer(const RenderLayer&); void commit(); void updateLayerBuckets(const RenderLayer&, const TransformState&, bool updateOpacities) const; float symbolFadeChange(TimePoint now) const; From ffa1cb3eb42d36b66d5d5ba48ddcc8ef6fc88999 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Mon, 20 Jan 2020 16:55:57 +0200 Subject: [PATCH 09/11] [core] Do not index and place the off-screen symbols for overscaled tiles For overscaled tiles the viewport might be showing only a small part of the tile, so we filter out the off-screen symbols to improve the performance. --- src/mbgl/layout/symbol_instance.hpp | 2 + src/mbgl/renderer/bucket.hpp | 2 +- src/mbgl/renderer/buckets/symbol_bucket.cpp | 8 ++-- src/mbgl/renderer/buckets/symbol_bucket.hpp | 3 +- src/mbgl/renderer/render_orchestrator.cpp | 2 +- src/mbgl/text/cross_tile_symbol_index.cpp | 44 ++++++++++++++++++--- src/mbgl/text/cross_tile_symbol_index.hpp | 5 ++- src/mbgl/text/placement.cpp | 11 +++--- test/text/cross_tile_symbol_index.test.cpp | 22 +++++------ 9 files changed, 69 insertions(+), 30 deletions(-) diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp index 2a25c655aa8..3cd67125b49 100644 --- a/src/mbgl/layout/symbol_instance.hpp +++ b/src/mbgl/layout/symbol_instance.hpp @@ -119,6 +119,8 @@ class SymbolInstance { std::array variableTextOffset; bool singleLine; uint32_t crossTileID = 0; + + static constexpr uint32_t invalidCrossTileID() { return std::numeric_limits::max(); } }; } // namespace mbgl diff --git a/src/mbgl/renderer/bucket.hpp b/src/mbgl/renderer/bucket.hpp index 3e756a06a46..fc34f55e75f 100644 --- a/src/mbgl/renderer/bucket.hpp +++ b/src/mbgl/renderer/bucket.hpp @@ -56,7 +56,7 @@ class Bucket { // Returns a pair, the first element of which is a bucket cross-tile id // on success call; `0` otherwise. The second element is `true` if // the bucket was originally registered; `false` otherwise. - virtual std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&) { + virtual std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const RenderTile&) { return std::make_pair(0u, false); } // Places this bucket to the given placement. diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp index 382aac6563a..55002e7851e 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.cpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp @@ -1,6 +1,7 @@ +#include #include #include -#include +#include #include #include #include @@ -37,6 +38,7 @@ SymbolBucket::SymbolBucket(Immutable SymbolBucket::registerAtCrossTileIndex(CrossTileSymbolLayerIndex& index, - const OverscaledTileID& tileID) { - bool firstTimeAdded = index.addBucket(tileID, *this); + const RenderTile& renderTile) { + bool firstTimeAdded = index.addBucket(renderTile.getOverscaledTileID(), renderTile.matrix, *this); return std::make_pair(bucketInstanceId, firstTimeAdded); } diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp index 432589a52bc..82d8c682aa4 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.hpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp @@ -81,7 +81,7 @@ class SymbolBucket final : public Bucket { void upload(gfx::UploadPass&) override; bool hasData() const override; - std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const OverscaledTileID&) override; + std::pair registerAtCrossTileIndex(CrossTileSymbolLayerIndex&, const RenderTile&) override; void place(Placement&, const BucketPlacementParameters&, std::set&) override; void updateVertices( const Placement&, bool updateOpacities, const TransformState&, const RenderTile&, std::set&) override; @@ -114,6 +114,7 @@ class SymbolBucket final : public Bucket { // Set and used by placement. mutable bool justReloaded : 1; bool hasVariablePlacement : 1; + bool hasUninitializedSymbols : 1; std::vector symbolInstances; diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index 79059164ae9..c771bd100b9 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -422,7 +422,7 @@ std::unique_ptr RenderOrchestrator::createRenderTree( Mutable placement = makeMutable(updateParameters); for (auto it = layersNeedPlacement.crbegin(); it != layersNeedPlacement.crend(); ++it) { const RenderLayer& layer = *it; - crossTileSymbolIndex.addLayer(layer, updateParameters->transformState); + crossTileSymbolIndex.addLayer(layer, longitude); placement->placeLayer(layer); } placement->commit(); diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp index 5b9d6ae69ce..f806c652e1b 100644 --- a/src/mbgl/text/cross_tile_symbol_index.cpp +++ b/src/mbgl/text/cross_tile_symbol_index.cpp @@ -12,6 +12,7 @@ TileLayerIndex::TileLayerIndex(OverscaledTileID coord_, std::string bucketLeaderId_) : coord(coord_), bucketInstanceId(bucketInstanceId_), bucketLeaderId(std::move(bucketLeaderId_)) { for (SymbolInstance& symbolInstance : symbolInstances) { + if (symbolInstance.crossTileID == SymbolInstance::invalidCrossTileID()) continue; indexedSymbolInstances[symbolInstance.key].emplace_back(symbolInstance.crossTileID, getScaledCoordinates(symbolInstance, coord)); } @@ -95,11 +96,29 @@ void CrossTileSymbolLayerIndex::handleWrapJump(float newLng) { lng = newLng; } -bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, SymbolBucket& bucket) { +namespace { + +bool isInVewport(const mat4& posMatrix, const Point& point) { + vec4 p = {{point.x, point.y, 0, 1}}; + matrix::transformMat4(p, p, posMatrix); + + // buffer covers area of the next zoom level (current zoom - 1 covered area). + constexpr float buffer = 1.0f; + constexpr float edge = 1.0f + buffer; + float x = p[0] / p[3]; + float y = p[1] / p[3]; + return (x > -edge && y > -edge && x < edge && y < edge); +} + +} // namespace + +bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, + const mat4& tileMatrix, + SymbolBucket& bucket) { auto& thisZoomIndexes = indexes[tileID.overscaledZ]; auto previousIndex = thisZoomIndexes.find(tileID); if (previousIndex != thisZoomIndexes.end()) { - if (previousIndex->second.bucketInstanceId == bucket.bucketInstanceId) { + if (previousIndex->second.bucketInstanceId == bucket.bucketInstanceId && !bucket.hasUninitializedSymbols) { return false; } else { // We're replacing this bucket with an updated version @@ -110,8 +129,23 @@ bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, Symbol } } - for (auto& symbolInstance: bucket.symbolInstances) { - symbolInstance.crossTileID = 0; + bucket.hasUninitializedSymbols = false; + + if (tileID.overscaleFactor() > 1u) { + // For overscaled tiles the viewport might be showing only a small part of the tile, + // so we filter out the off-screen symbols to improve the performance. + for (auto& symbolInstance : bucket.symbolInstances) { + if (isInVewport(tileMatrix, symbolInstance.anchor.point)) { + symbolInstance.crossTileID = 0u; + } else { + symbolInstance.crossTileID = SymbolInstance::invalidCrossTileID(); + bucket.hasUninitializedSymbols = true; + } + } + } else { + for (auto& symbolInstance : bucket.symbolInstances) { + symbolInstance.crossTileID = 0u; + } } auto& thisZoomUsedCrossTileIDs = usedCrossTileIDs[tileID.overscaledZ]; @@ -195,7 +229,7 @@ auto CrossTileSymbolIndex::addLayer(const RenderLayer& layer, float lng) -> AddL for (const auto& item : layer.getPlacementData()) { const RenderTile& renderTile = item.tile; Bucket& bucket = item.bucket; - auto pair = bucket.registerAtCrossTileIndex(layerIndex, renderTile.getOverscaledTileID()); + auto pair = bucket.registerAtCrossTileIndex(layerIndex, renderTile); assert(pair.first != 0u); if (pair.second) result |= AddLayerResult::BucketsAdded; currentBucketIDs.insert(pair.first); diff --git a/src/mbgl/text/cross_tile_symbol_index.hpp b/src/mbgl/text/cross_tile_symbol_index.hpp index 3b7c3677269..8dfdb8466f9 100644 --- a/src/mbgl/text/cross_tile_symbol_index.hpp +++ b/src/mbgl/text/cross_tile_symbol_index.hpp @@ -2,8 +2,9 @@ #include #include -#include #include +#include +#include #include #include @@ -48,7 +49,7 @@ class TileLayerIndex { class CrossTileSymbolLayerIndex { public: CrossTileSymbolLayerIndex(uint32_t& maxCrossTileID); - bool addBucket(const OverscaledTileID&, SymbolBucket&); + bool addBucket(const OverscaledTileID&, const mat4& tileMatrix, SymbolBucket&); bool removeStaleBuckets(const std::unordered_set& currentIDs); void handleWrapJump(float newLng); private: diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index 43d97137173..32edf679e83 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -157,10 +157,7 @@ void Placement::placeBucket(const SymbolBucket& bucket, const bool rotateIconWithMap = layout.get() == style::AlignmentType::Map; const bool pitchIconWithMap = layout.get() == style::AlignmentType::Map; - mat4 posMatrix; - const auto& projMatrix = state.getProjectionMatrix(); - state.matrixFor(posMatrix, renderTile.id); - matrix::multiply(posMatrix, projMatrix, posMatrix); + const mat4& posMatrix = renderTile.matrix; mat4 textLabelPlaneMatrix = getLabelPlaneMatrix(posMatrix, pitchTextWithMap, rotateTextWithMap, state, pixelsToTileUnits); @@ -212,8 +209,10 @@ void Placement::placeBucket(const SymbolBucket& bucket, std::vector textBoxes; std::vector iconBoxes; - auto placeSymbol = [&] (const SymbolInstance& symbolInstance) { - if (seenCrossTileIDs.count(symbolInstance.crossTileID) != 0u) return; + auto placeSymbol = [&](const SymbolInstance& symbolInstance) { + if (symbolInstance.crossTileID == SymbolInstance::invalidCrossTileID() || + seenCrossTileIDs.count(symbolInstance.crossTileID) != 0u) + return; if (renderTile.holdForFade()) { // Mark all symbols from this tile as "not placed", but don't add to seenCrossTileIDs, because we don't diff --git a/test/text/cross_tile_symbol_index.test.cpp b/test/text/cross_tile_symbol_index.test.cpp index f0c78ce6329..61597e021ff 100644 --- a/test/text/cross_tile_symbol_index.test.cpp +++ b/test/text/cross_tile_symbol_index.test.cpp @@ -62,7 +62,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; mainBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(mainID, mainBucket); + index.addBucket(mainID, mat4{}, mainBucket); // Assigned new IDs ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); @@ -89,7 +89,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; childBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(childID, childBucket); + index.addBucket(childID, mat4{}, childBucket); // matched parent tile ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 1u); @@ -117,7 +117,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; parentBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(parentID, parentBucket); + index.addBucket(parentID, mat4{}, parentBucket); // matched child tile ASSERT_EQ(parentBucket.symbolInstances.at(0).crossTileID, 1u); @@ -145,7 +145,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { {}, false /*iconsInText*/}; grandchildBucket.bucketInstanceId = ++maxBucketInstanceId; - index.addBucket(grandchildID, grandchildBucket); + index.addBucket(grandchildID, mat4{}, grandchildBucket); // Matches the symbol in `mainBucket` ASSERT_EQ(grandchildBucket.symbolInstances.at(0).crossTileID, 1u); @@ -203,7 +203,7 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { childBucket.bucketInstanceId = ++maxBucketInstanceId; // assigns a new id - index.addBucket(mainID, mainBucket); + index.addBucket(mainID, mat4{}, mainBucket); ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); // removes the tile @@ -211,11 +211,11 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { index.removeStaleBuckets(currentIDs); // assigns a new id - index.addBucket(childID, childBucket); + index.addBucket(childID, mat4{}, childBucket); ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 2u); // overwrites the old id to match the already-added tile - index.addBucket(mainID, mainBucket); + index.addBucket(mainID, mat4{}, mainBucket); ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 2u); } @@ -270,12 +270,12 @@ TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) { childBucket.bucketInstanceId = ++maxBucketInstanceId; // assigns new ids - index.addBucket(mainID, mainBucket); + index.addBucket(mainID, mat4{}, mainBucket); ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); ASSERT_EQ(mainBucket.symbolInstances.at(1).crossTileID, 2u); // copies parent ids without duplicate ids in this tile - index.addBucket(childID, childBucket); + index.addBucket(childID, mat4{}, childBucket); ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 1u); // A' copies from A ASSERT_EQ(childBucket.symbolInstances.at(1).crossTileID, 2u); // B' copies from B ASSERT_EQ(childBucket.symbolInstances.at(2).crossTileID, 3u); // C' gets new ID @@ -331,12 +331,12 @@ TEST(CrossTileSymbolLayerIndex, bucketReplacement) { secondBucket.bucketInstanceId = ++maxBucketInstanceId; // assigns new ids - index.addBucket(tileID, firstBucket); + index.addBucket(tileID, mat4{}, firstBucket); ASSERT_EQ(firstBucket.symbolInstances.at(0).crossTileID, 1u); ASSERT_EQ(firstBucket.symbolInstances.at(1).crossTileID, 2u); // copies parent ids without duplicate ids in this tile - index.addBucket(tileID, secondBucket); + index.addBucket(tileID, mat4{}, secondBucket); ASSERT_EQ(secondBucket.symbolInstances.at(0).crossTileID, 1u); // A' copies from A ASSERT_EQ(secondBucket.symbolInstances.at(1).crossTileID, 2u); // B' copies from B ASSERT_EQ(secondBucket.symbolInstances.at(2).crossTileID, 3u); // C' gets new ID From b905f260c64ec4ea88a6d098fd5f87c9b67c6a2d Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Wed, 22 Jan 2020 13:34:22 +0200 Subject: [PATCH 10/11] Add CrossTileSymbolLayerIndex.offscreenSymbols test --- test/text/cross_tile_symbol_index.test.cpp | 55 +++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/test/text/cross_tile_symbol_index.test.cpp b/test/text/cross_tile_symbol_index.test.cpp index 61597e021ff..6119392fd23 100644 --- a/test/text/cross_tile_symbol_index.test.cpp +++ b/test/text/cross_tile_symbol_index.test.cpp @@ -1,6 +1,7 @@ -#include +#include #include #include +#include using namespace mbgl; @@ -342,3 +343,55 @@ TEST(CrossTileSymbolLayerIndex, bucketReplacement) { ASSERT_EQ(secondBucket.symbolInstances.at(2).crossTileID, 3u); // C' gets new ID } +namespace { + +void populatePosMatrix(mat4& posMatrix, const OverscaledTileID& tileId, double lat, double lon, double zoom) { + TransformState transformState; + transformState.setSize({512, 512}); + transformState.setLatLngZoom(LatLng(lat, lon), zoom); + transformState.matrixFor(posMatrix, tileId.toUnwrapped()); + matrix::multiply(posMatrix, transformState.getProjectionMatrix(), posMatrix); +} + +} // namespace + +TEST(CrossTileSymbolLayerIndex, offscreenSymbols) { + uint32_t maxCrossTileID = 0; + CrossTileSymbolLayerIndex index(maxCrossTileID); + + Immutable layout = + makeMutable(); + bool iconsNeedLinear = false; + bool sortFeaturesByY = false; + std::string bucketLeaderID = "test"; + + OverscaledTileID tileId(7, 0, 6, 18, 24); + std::vector mainInstances; + mainInstances.push_back(makeSymbolInstance(1000, 1000, u"Washington")); + mainInstances.push_back(makeSymbolInstance(2000, 2000, u"Richmond")); + SymbolBucket symbolBucket{layout, + {}, + 16.0f, + 1.0f, + 0, + iconsNeedLinear, + sortFeaturesByY, + bucketLeaderID, + std::move(mainInstances), + 1.0f, + false, + {}, + false /*iconsInText*/}; + mat4 posMatrix; + populatePosMatrix(posMatrix, tileId, 60.0, 25.0, 7.0); + index.addBucket(tileId, posMatrix, symbolBucket); + + EXPECT_EQ(symbolBucket.symbolInstances.at(0).crossTileID, SymbolInstance::invalidCrossTileID()); + EXPECT_EQ(symbolBucket.symbolInstances.at(1).crossTileID, SymbolInstance::invalidCrossTileID()); + + populatePosMatrix(posMatrix, tileId, 39.0, -76.0, 7.0); + index.addBucket(tileId, posMatrix, symbolBucket); + + EXPECT_EQ(symbolBucket.symbolInstances.at(0).crossTileID, 1u); + EXPECT_EQ(symbolBucket.symbolInstances.at(1).crossTileID, 2u); +} \ No newline at end of file From 688f545f1e0a69dd4dfe5f14d8e3b294a888bc4a Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Wed, 22 Jan 2020 13:37:39 +0200 Subject: [PATCH 11/11] Add change log entry --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39558e2c589..fbb51ec5441 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,12 @@ ### 🏁 Performance improvements +- [core] Cross tile index performance ([#16127](https://github.com/mapbox/mapbox-gl-native/pull/16127)) + + For overscaled tiles, index only the symbols inside the viewport. + + Find matches only among the buckets that have the same leader Id. + - [core] Calculate GeoJSON tile geometries in a background thread ([#15953](https://github.com/mapbox/mapbox-gl-native/pull/15953)) Call `mapbox::geojsonvt::GeoJSONVT::getTile()` in a background thread, so that the rendering thread is not blocked.