From 7f42e3970ee8c6a57fbb4dba7c564ff17c2eb6bb Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Fri, 30 Mar 2018 16:02:56 -0700 Subject: [core] Convert GeometryTileWorker to "one-phase" loading Modest simplification refactoring (issue #10457). Also, fixes issue #11538, which was caused in part by a hole in the vestigial two-phase loading. --- src/mbgl/tile/geometry_tile.cpp | 46 ++------- src/mbgl/tile/geometry_tile.hpp | 33 +++--- src/mbgl/tile/geometry_tile_worker.cpp | 177 ++++++++++++++++++++------------- src/mbgl/tile/geometry_tile_worker.hpp | 14 ++- 4 files changed, 141 insertions(+), 129 deletions(-) (limited to 'src/mbgl/tile') diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 0c8a92d8b7..f95d7220bd 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -125,36 +125,17 @@ void GeometryTile::setShowCollisionBoxes(const bool showCollisionBoxes_) { } void GeometryTile::onLayout(LayoutResult result, const uint64_t resultCorrelationID) { - // Don't mark ourselves loaded or renderable until the first successful placement - // TODO: Ideally we'd render this tile without symbols as long as this tile wasn't - // replacing a tile at a different zoom that _did_ have symbols. - (void)resultCorrelationID; - nonSymbolBuckets = std::move(result.nonSymbolBuckets); - // It is possible for multiple onLayouts to be called before an onPlacement is called, in which - // case we will discard previous pending data - pendingFeatureIndex = {{ false, std::move(result.featureIndex) }}; - pendingData = {{ false, std::move(result.tileData) }}; - observer->onTileChanged(*this); -} - -void GeometryTile::onPlacement(PlacementResult result, const uint64_t resultCorrelationID) { loaded = true; renderable = true; if (resultCorrelationID == correlationID) { pending = false; } + + nonSymbolBuckets = std::move(result.nonSymbolBuckets); symbolBuckets = std::move(result.symbolBuckets); - // When symbolBuckets arrive, mark pendingData/FeatureIndex as "ready for commit" at the - // time of the next global placement. We are counting on these symbolBuckets being - // in sync with the data/index in the last `onLayout` call, even though the correlation IDs - // may not be the same (e.g. "setShowCollisionBoxes" could bump the correlation ID while - // waiting for glyph dependencies) - if (pendingData) { - pendingData->first = true; - } - if (pendingFeatureIndex) { - pendingFeatureIndex->first = true; - } + + dataPendingCommit = {{ std::move(result.tileData), std::move(result.featureIndex) }}; + if (result.glyphAtlasImage) { glyphAtlasImage = std::move(*result.glyphAtlasImage); } @@ -227,17 +208,12 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const { } void GeometryTile::commitFeatureIndex() { - // We commit our pending FeatureIndex and GeometryTileData when: - // 1) An `onPlacement` result has delivered us updated symbolBuckets since we received the pending data - // 2) A global placement has run, synchronizing the global CollisionIndex with the latest - // symbolBuckets (and thus with the latest FeatureIndex/GeometryTileData) - if (pendingFeatureIndex && pendingFeatureIndex->first) { - featureIndex = std::move(pendingFeatureIndex->second); - pendingFeatureIndex = nullopt; - } - if (pendingData && pendingData->first) { - data = std::move(pendingData->second); - pendingData = nullopt; + // We commit our pending FeatureIndex and GeometryTileData when a global placement has run, + // synchronizing the global CollisionIndex with the latest symbolBuckets/FeatureIndex/GeometryTileData + if (dataPendingCommit) { + data = std::move(dataPendingCommit->first); + featureIndex = std::move(dataPendingCommit->second); + dataPendingCommit = nullopt; } } diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 8843d6e579..98b5f0c490 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -68,30 +68,24 @@ public: std::unordered_map> nonSymbolBuckets; std::unique_ptr featureIndex; std::unique_ptr tileData; + std::unordered_map> symbolBuckets; + optional glyphAtlasImage; + optional iconAtlasImage; LayoutResult(std::unordered_map> nonSymbolBuckets_, std::unique_ptr featureIndex_, - std::unique_ptr tileData_) + std::unique_ptr tileData_, + std::unordered_map> symbolBuckets_, + optional glyphAtlasImage_, + optional iconAtlasImage_) : nonSymbolBuckets(std::move(nonSymbolBuckets_)), featureIndex(std::move(featureIndex_)), - tileData(std::move(tileData_)) {} - }; - void onLayout(LayoutResult, uint64_t correlationID); - - class PlacementResult { - public: - std::unordered_map> symbolBuckets; - optional glyphAtlasImage; - optional iconAtlasImage; - - PlacementResult(std::unordered_map> symbolBuckets_, - optional glyphAtlasImage_, - optional iconAtlasImage_) - : symbolBuckets(std::move(symbolBuckets_)), + tileData(std::move(tileData_)), + symbolBuckets(std::move(symbolBuckets_)), glyphAtlasImage(std::move(glyphAtlasImage_)), iconAtlasImage(std::move(iconAtlasImage_)) {} }; - void onPlacement(PlacementResult, uint64_t correlationID); + void onLayout(LayoutResult, uint64_t correlationID); void onError(std::exception_ptr, uint64_t correlationID); @@ -124,16 +118,15 @@ private: uint64_t correlationID = 0; std::unordered_map> nonSymbolBuckets; + std::unordered_map> symbolBuckets; + + optional, std::unique_ptr>> dataPendingCommit; std::unique_ptr featureIndex; - optional>> pendingFeatureIndex; std::unique_ptr data; - optional>> pendingData; optional glyphAtlasImage; optional iconAtlasImage; - std::unordered_map> symbolBuckets; - const MapMode mode; bool showCollisionBoxes; diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index 8b6f1f63bf..482cbe0053 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -41,48 +41,73 @@ GeometryTileWorker::GeometryTileWorker(ActorRef self_, GeometryTileWorker::~GeometryTileWorker() = default; /* - NOTE: The comments below are technically correct, but currently - conceptually misleading. The change to foreground label placement - means that: - (1) "placement" here is a misnomer: the remaining role of - "attemptPlacement" is symbol buffer generation - (2) Once a tile has completed layout, we will only run - "attemptPlacement" once - (3) Tiles won't be rendered until "attemptPlacement" has run once - - TODO: Simplify GeometryTileWorker to fit its new role - https://github.com/mapbox/mapbox-gl-native/issues/10457 - GeometryTileWorker is a state machine. This is its transition diagram. States are indicated by [state], lines are transitions triggered by messages, (parentheses) are actions taken on transition. - [idle] <----------------------------. - | | - set{Data,Layers,Placement}, symbolDependenciesChanged | - | | - (do layout/placement; self-send "coalesced") | - v | - [coalescing] --- coalesced ------------. - | | - .-----------------. .---------------. + [Idle] <-------------------------. + | | + set{Data,Layers}, symbolDependenciesChanged, | + setShowCollisionBoxes | + | | + (do parse and/or symbol layout; self-send "coalesced") | + v | + [Coalescing] --- coalesced ---------. + | | + .-----------. .---------------------. | | - .--- set{Data,Layers} setPlacement -----. - | | | | - | v v | - .-- [need layout] <-- set{Data,Layers} -- [need placement] --. + .--- set{Data,Layers} setShowCollisionBoxes, + | | symbolDependenciesChanged --. + | | | | + | v v | + .-- [NeedsParse] <-- set{Data,Layers} -- [NeedsSymbolLayout] ---. | | coalesced coalesced | | v v - (do layout or placement; self-send "coalesced"; goto [coalescing]) - - The idea is that in the [idle] state, layout or placement happens immediately - in response to a "set" message. During this processing, multiple "set" messages - might get queued in the mailbox. At the end of processing, we self-send "coalesced", - read all the queued messages until we get to "coalesced", and then redo either - layout or placement if there were one or more "set"s (with layout taking priority, - since it will trigger placement when complete), or return to the [idle] state if not. + (do parse or symbol layout; self-send "coalesced"; goto [coalescing]) + + The idea is that in the [idle] state, parsing happens immediately in response to + a "set" message, and symbol layout happens once all symbol dependencies are met. + During this processing, multiple "set" messages might get queued in the mailbox. + At the end of processing, we self-send "coalesced", read all the queued messages + until we get to "coalesced", and then re-parse if there were one or more "set"s or + return to the [idle] state if not. + + One important goal of the design is to prevent starvation. Under heavy load new + requests for tiles should not prevent in progress request from completing. + It is nevertheless possible to restart an in-progress request: + + - [Idle] setData -> parse() + sends getGlyphs, hasPendingSymbolDependencies() is true + enters [Coalescing], sends coalesced + - [Coalescing] coalesced -> [Idle] + - [Idle] setData -> new parse(), interrupts old parse() + sends getGlyphs, hasPendingSymbolDependencies() is true + enters [Coalescing], sends coalesced + - [Coalescing] onGlyphsAvailable -> [NeedsSymbolLayout] + hasPendingSymbolDependencies() may or may not be true + - [NeedsSymbolLayout] coalesced -> performSymbolLayout() + Generates result depending on whether dependencies are met + -> [Idle] + + In this situation, we are counting on the idea that even with rapid changes to + the tile's data, the set of glyphs/images it requires will not keep growing without + limit. + + Although parsing (which populates all non-symbol buckets and requests dependencies + for symbol buckets) is internally separate from symbol layout, we only return + results to the foreground when we have completed both steps. Because we _move_ + the result buckets to the foreground, it is necessary to re-generate all buckets from + scratch for `setShowCollisionBoxes`, even though it only affects symbol layers. + + The GL JS equivalent (in worker_tile.js and vector_tile_worker_source.js) + is somewhat simpler because it relies on getGlyphs/getImages calls that transfer + an entire set of glyphs/images on every tile load, while the native logic + maintains a local state that can be incrementally updated. Because each tile load + call becomes self-contained, the equivalent of the coalescing logic is handled by + 'reloadTile' queueing a single extra 'reloadTile' callback to run after the next + completed parse. */ void GeometryTileWorker::setData(std::unique_ptr data_, uint64_t correlationID_) { @@ -92,14 +117,14 @@ void GeometryTileWorker::setData(std::unique_ptr data_, switch (state) { case Idle: - redoLayout(); + parse(); coalesce(); break; case Coalescing: - case NeedLayout: - case NeedPlacement: - state = NeedLayout; + case NeedsParse: + case NeedsSymbolLayout: + state = NeedsParse; break; } } catch (...) { @@ -114,16 +139,16 @@ void GeometryTileWorker::setLayers(std::vector> layers_, switch (state) { case Idle: - redoLayout(); + parse(); coalesce(); break; case Coalescing: - case NeedPlacement: - state = NeedLayout; + case NeedsSymbolLayout: + state = NeedsParse; break; - case NeedLayout: + case NeedsParse: break; } } catch (...) { @@ -138,16 +163,20 @@ void GeometryTileWorker::setShowCollisionBoxes(bool showCollisionBoxes_, uint64_ switch (state) { case Idle: - attemptPlacement(); - coalesce(); + if (!hasPendingParseResult()) { + // Trigger parse if nothing is in flight, otherwise symbol layout will automatically + // pick up the change + parse(); + coalesce(); + } break; case Coalescing: - state = NeedPlacement; + state = NeedsSymbolLayout; break; - case NeedPlacement: - case NeedLayout: + case NeedsSymbolLayout: + case NeedsParse: break; } } catch (...) { @@ -160,19 +189,23 @@ void GeometryTileWorker::symbolDependenciesChanged() { switch (state) { case Idle: if (symbolLayoutsNeedPreparation) { - attemptPlacement(); + // symbolLayoutsNeedPreparation can only be set true by parsing + // and the parse result can only be cleared by performSymbolLayout + // which also clears symbolLayoutsNeedPreparation + assert(hasPendingParseResult()); + performSymbolLayout(); coalesce(); } break; case Coalescing: if (symbolLayoutsNeedPreparation) { - state = NeedPlacement; + state = NeedsSymbolLayout; } break; - case NeedPlacement: - case NeedLayout: + case NeedsSymbolLayout: + case NeedsParse: break; } } catch (...) { @@ -191,13 +224,16 @@ void GeometryTileWorker::coalesced() { state = Idle; break; - case NeedLayout: - redoLayout(); + case NeedsParse: + parse(); coalesce(); break; - case NeedPlacement: - attemptPlacement(); + case NeedsSymbolLayout: + // We may have entered NeedsSymbolLayout while coalescing + // after a performSymbolLayout. In that case, we need to + // start over with parsing in order to do another layout. + hasPendingParseResult() ? performSymbolLayout() : parse(); coalesce(); break; } @@ -279,7 +315,7 @@ static std::vector> toRenderLayers(const std::vecto return renderLayers; } -void GeometryTileWorker::redoLayout() { +void GeometryTileWorker::parse() { if (!data || !layers) { return; } @@ -292,8 +328,8 @@ void GeometryTileWorker::redoLayout() { } std::unordered_map> symbolLayoutMap; - std::unordered_map> buckets; - auto featureIndex = std::make_unique(); + nonSymbolBuckets.clear(); + featureIndex = std::make_unique(); BucketParameters parameters { id, mode, pixelRatio }; GlyphDependencies glyphDependencies; @@ -352,7 +388,7 @@ void GeometryTileWorker::redoLayout() { } for (const auto& layer : group) { - buckets.emplace(layer->getID(), bucket); + nonSymbolBuckets.emplace(layer->getID(), bucket); } } } @@ -368,13 +404,7 @@ void GeometryTileWorker::redoLayout() { requestNewGlyphs(glyphDependencies); requestNewImages(imageDependencies); - parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult { - std::move(buckets), - std::move(featureIndex), - *data ? (*data)->clone() : nullptr, - }, correlationID); - - attemptPlacement(); + performSymbolLayout(); } bool GeometryTileWorker::hasPendingSymbolDependencies() const { @@ -385,9 +415,13 @@ bool GeometryTileWorker::hasPendingSymbolDependencies() const { } return !pendingImageDependencies.empty(); } + +bool GeometryTileWorker::hasPendingParseResult() const { + return bool(featureIndex); +} -void GeometryTileWorker::attemptPlacement() { - if (!data || !layers || hasPendingSymbolDependencies()) { +void GeometryTileWorker::performSymbolLayout() { + if (!data || !layers || !hasPendingParseResult() || hasPendingSymbolDependencies()) { return; } @@ -435,11 +469,14 @@ void GeometryTileWorker::attemptPlacement() { } firstLoad = false; - - parent.invoke(&GeometryTile::onPlacement, GeometryTile::PlacementResult { + + parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult { + std::move(nonSymbolBuckets), + std::move(featureIndex), + *data ? (*data)->clone() : nullptr, std::move(buckets), std::move(glyphAtlasImage), - std::move(iconAtlasImage), + std::move(iconAtlasImage) }, correlationID); } diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp index 0276392679..458ec76988 100644 --- a/src/mbgl/tile/geometry_tile_worker.hpp +++ b/src/mbgl/tile/geometry_tile_worker.hpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -43,8 +45,8 @@ public: private: void coalesced(); - void redoLayout(); - void attemptPlacement(); + void parse(); + void performSymbolLayout(); void coalesce(); @@ -53,6 +55,7 @@ private: void symbolDependenciesChanged(); bool hasPendingSymbolDependencies() const; + bool hasPendingParseResult() const; ActorRef self; ActorRef parent; @@ -62,12 +65,15 @@ private: const std::atomic& obsolete; const MapMode mode; const float pixelRatio; + + std::unique_ptr featureIndex; + std::unordered_map> nonSymbolBuckets; enum State { Idle, Coalescing, - NeedLayout, - NeedPlacement + NeedsParse, + NeedsSymbolLayout }; State state = Idle; -- cgit v1.2.1