From e44db93f1cb3276dcdc7de8400ca96beda1b1d30 Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Wed, 13 Jan 2016 21:39:42 -0800 Subject: [PATCH] [core] place symbol layers in correct order Previously, the placement of symbols depended on the order in which the glyphs they need were loaded. The placement order should be based on the style. To fix this, symbol layers are placed in `placementPending` after they are parsed. After all layers are parsed, symbol layers (stored in `placementPending`) are placed and returned. --- src/mbgl/layer/symbol_layer.cpp | 3 +- src/mbgl/map/tile_worker.cpp | 46 +++++++++++++++------- src/mbgl/map/tile_worker.hpp | 8 +++- src/mbgl/map/vector_tile_data.cpp | 6 ++- src/mbgl/renderer/symbol_bucket.cpp | 11 +----- src/mbgl/renderer/symbol_bucket.hpp | 4 +- src/mbgl/style/style_bucket_parameters.hpp | 3 -- src/mbgl/util/worker.cpp | 6 ++- src/mbgl/util/worker.hpp | 1 + 9 files changed, 51 insertions(+), 37 deletions(-) diff --git a/src/mbgl/layer/symbol_layer.cpp b/src/mbgl/layer/symbol_layer.cpp index e05b27bff..77254d6f2 100644 --- a/src/mbgl/layer/symbol_layer.cpp +++ b/src/mbgl/layer/symbol_layer.cpp @@ -177,8 +177,7 @@ std::unique_ptr SymbolLayer::createBucket(StyleBucketParameters& paramet bucket->addFeatures(parameters.tileUID, *spriteAtlas, parameters.glyphAtlas, - parameters.glyphStore, - parameters.collisionTile); + parameters.glyphStore); } return std::move(bucket); diff --git a/src/mbgl/map/tile_worker.cpp b/src/mbgl/map/tile_worker.cpp index f483082cf..a7f733eed 100644 --- a/src/mbgl/map/tile_worker.cpp +++ b/src/mbgl/map/tile_worker.cpp @@ -41,14 +41,12 @@ TileParseResult TileWorker::parseAllLayers(std::vector(config); - // We're storing a set of bucket names we've parsed to avoid parsing a bucket twice that is // referenced from more than one layer std::set parsed; @@ -62,10 +60,15 @@ TileParseResult TileWorker::parseAllLayers(std::vectoraddFeatures(reinterpret_cast(this), *layer.spriteAtlas, glyphAtlas, - glyphStore, - *collisionTile); - insertBucket(layer.bucketName(), std::move(it->second)); + glyphStore); + placementPending.emplace(layer.bucketName(), std::move(it->second)); pending.erase(it++); continue; } @@ -89,20 +91,33 @@ TileParseResult TileWorker::parsePendingLayers() { } result.state = pending.empty() ? TileData::State::parsed : TileData::State::partial; + + if (result.state == TileData::State::parsed) { + placeLayers(config); + } + return std::move(result); } +void TileWorker::placeLayers(const PlacementConfig config) { + redoPlacement(&placementPending, config); + for (auto &p : placementPending) { + p.second->swapRenderData(); + insertBucket(p.first, std::move(p.second)); + } + placementPending.clear(); +} + void TileWorker::redoPlacement( const std::unordered_map>* buckets, PlacementConfig config) { - // Reset the collision tile so we have a clean slate; we're placing all features anyway. - collisionTile = std::make_unique(config); + CollisionTile collisionTile(config); for (auto i = layers.rbegin(); i != layers.rend(); i++) { const auto it = buckets->find((*i)->id); if (it != buckets->end()) { - it->second->placeFeatures(*collisionTile); + it->second->placeFeatures(collisionTile); } } } @@ -142,14 +157,17 @@ void TileWorker::parseLayer(const StyleLayer* layer, const GeometryTile& geometr spriteStore, glyphAtlas, glyphStore, - *collisionTile, mode); std::unique_ptr bucket = layer->createBucket(parameters); - if (layer->is() && partialParse) { - // We cannot parse this bucket yet. Instead, we're saving it for later. - pending.emplace_back(layer->as(), std::move(bucket)); + if (layer->is()) { + if (partialParse) { + // We cannot parse this bucket yet. Instead, we're saving it for later. + pending.emplace_back(layer->as(), std::move(bucket)); + } else { + placementPending.emplace(layer->bucketName(), std::move(bucket)); + } } else { insertBucket(layer->bucketName(), std::move(bucket)); } diff --git a/src/mbgl/map/tile_worker.hpp b/src/mbgl/map/tile_worker.hpp index 74894a9eb..a5e892e2b 100644 --- a/src/mbgl/map/tile_worker.hpp +++ b/src/mbgl/map/tile_worker.hpp @@ -53,7 +53,7 @@ public: const GeometryTile&, PlacementConfig); - TileParseResult parsePendingLayers(); + TileParseResult parsePendingLayers(PlacementConfig); void redoPlacement(const std::unordered_map>*, PlacementConfig); @@ -61,6 +61,7 @@ public: private: void parseLayer(const StyleLayer*, const GeometryTile&); void insertBucket(const std::string& name, std::unique_ptr); + void placeLayers(PlacementConfig); const TileID id; const std::string sourceID; @@ -74,12 +75,15 @@ private: bool partialParse = false; std::vector> layers; - std::unique_ptr collisionTile; // Contains buckets that we couldn't parse so far due to missing resources. // They will be attempted on subsequent parses. std::list>> pending; + // Contains buckets that have been parsed, but still need placement. + // They will be placed when all buckets have been parsed. + std::unordered_map> placementPending; + // Temporary holder TileParseResultBuckets result; }; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 048cadc98..f44dc7598 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -102,7 +102,7 @@ bool VectorTileData::parsePending(std::function callback) { } workRequest.reset(); - workRequest = worker.parsePendingGeometryTileLayers(tileWorker, [this, callback] (TileParseResult result) { + workRequest = worker.parsePendingGeometryTileLayers(tileWorker, targetConfig, [this, callback, config = targetConfig] (TileParseResult result) { workRequest.reset(); if (state == State::obsolete) { return; @@ -118,6 +118,10 @@ bool VectorTileData::parsePending(std::function callback) { buckets[bucket.first] = std::move(bucket.second); } + // Persist the configuration we just placed so that we can later check whether we need to + // place again in case the configuration has changed. + placedConfig = config; + // The target configuration could have changed since we started placement. In this case, // we're starting another placement run. if (placedConfig != targetConfig) { diff --git a/src/mbgl/renderer/symbol_bucket.cpp b/src/mbgl/renderer/symbol_bucket.cpp index 67539637b..a34cbcd02 100644 --- a/src/mbgl/renderer/symbol_bucket.cpp +++ b/src/mbgl/renderer/symbol_bucket.cpp @@ -173,8 +173,7 @@ bool SymbolBucket::needsDependencies(GlyphStore& glyphStore, SpriteStore& sprite void SymbolBucket::addFeatures(uintptr_t tileUID, SpriteAtlas& spriteAtlas, GlyphAtlas& glyphAtlas, - GlyphStore& glyphStore, - CollisionTile& collisionTile) { + GlyphStore& glyphStore) { float horizontalAlign = 0.5; float verticalAlign = 0.5; @@ -266,8 +265,6 @@ void SymbolBucket::addFeatures(uintptr_t tileUID, } features.clear(); - - placeFeatures(collisionTile, true); } @@ -358,10 +355,6 @@ bool SymbolBucket::anchorIsTooClose(const std::u32string &text, const float repe } void SymbolBucket::placeFeatures(CollisionTile& collisionTile) { - placeFeatures(collisionTile, false); -} - -void SymbolBucket::placeFeatures(CollisionTile& collisionTile, bool swapImmediately) { renderDataInProgress = std::make_unique(); @@ -448,8 +441,6 @@ void SymbolBucket::placeFeatures(CollisionTile& collisionTile, bool swapImmediat if (collisionTile.config.debug) { addToDebugBuffers(collisionTile); } - - if (swapImmediately) swapRenderData(); } template diff --git a/src/mbgl/renderer/symbol_bucket.hpp b/src/mbgl/renderer/symbol_bucket.hpp index 4f52f2300..68a622bb8 100644 --- a/src/mbgl/renderer/symbol_bucket.hpp +++ b/src/mbgl/renderer/symbol_bucket.hpp @@ -79,8 +79,7 @@ public: void addFeatures(uintptr_t tileUID, SpriteAtlas&, GlyphAtlas&, - GlyphStore&, - CollisionTile&); + GlyphStore&); void drawGlyphs(SDFShader& shader); void drawIcons(SDFShader& shader); @@ -101,7 +100,6 @@ private: void addToDebugBuffers(CollisionTile &collisionTile); - void placeFeatures(CollisionTile& collisionTile, bool swapImmediately); void swapRenderData() override; // Adds placed items to the buffer. diff --git a/src/mbgl/style/style_bucket_parameters.hpp b/src/mbgl/style/style_bucket_parameters.hpp index 732ebade1..3329cac03 100644 --- a/src/mbgl/style/style_bucket_parameters.hpp +++ b/src/mbgl/style/style_bucket_parameters.hpp @@ -27,7 +27,6 @@ public: SpriteStore& spriteStore_, GlyphAtlas& glyphAtlas_, GlyphStore& glyphStore_, - CollisionTile& collisionTile_, const MapMode mode_) : tileID(tileID_), layer(layer_), @@ -37,7 +36,6 @@ public: spriteStore(spriteStore_), glyphAtlas(glyphAtlas_), glyphStore(glyphStore_), - collisionTile(collisionTile_), mode(mode_) {} bool cancelled() const { @@ -54,7 +52,6 @@ public: SpriteStore& spriteStore; GlyphAtlas& glyphAtlas; GlyphStore& glyphStore; - CollisionTile& collisionTile; const MapMode mode; }; diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp index 49d1c2bc5..d1e54fa37 100644 --- a/src/mbgl/util/worker.cpp +++ b/src/mbgl/util/worker.cpp @@ -39,9 +39,10 @@ public: } void parsePendingGeometryTileLayers(TileWorker* worker, + PlacementConfig config, std::function callback) { try { - callback(worker->parsePendingLayers()); + callback(worker->parsePendingLayers(config)); } catch (...) { callback(std::current_exception()); } @@ -87,10 +88,11 @@ Worker::parseGeometryTile(TileWorker& worker, std::unique_ptr Worker::parsePendingGeometryTileLayers(TileWorker& worker, + PlacementConfig config, std::function callback) { current = (current + 1) % threads.size(); return threads[current]->invokeWithCallback(&Worker::Impl::parsePendingGeometryTileLayers, - callback, &worker); + callback, &worker, config); } std::unique_ptr diff --git a/src/mbgl/util/worker.hpp b/src/mbgl/util/worker.hpp index d4660d9e7..d72438bc7 100644 --- a/src/mbgl/util/worker.hpp +++ b/src/mbgl/util/worker.hpp @@ -46,6 +46,7 @@ public: std::function callback); Request parsePendingGeometryTileLayers(TileWorker&, + PlacementConfig config, std::function callback); Request redoPlacement(TileWorker&,