From 1c42daa152c1e7302c0997c00e206cf7c4b33d73 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 26 Apr 2017 12:42:17 -0700 Subject: [PATCH] [core] Tweak handling of annotation special case SpriteAtlas * Simplify SymbolLayout; it never needs to care about more than one SpriteAtlas. * Move the reference from SymbolLayer::Impl to SymbolBucket. This is a prerequisite for making layer Impls immutable. --- src/mbgl/annotation/annotation_manager.cpp | 2 -- src/mbgl/annotation/annotation_manager.hpp | 2 ++ src/mbgl/annotation/annotation_tile.cpp | 5 ++- src/mbgl/layout/symbol_layout.cpp | 31 ++++++++---------- src/mbgl/layout/symbol_layout.hpp | 5 +-- src/mbgl/renderer/painter_symbol.cpp | 2 +- src/mbgl/renderer/render_symbol_layer.cpp | 5 ++- src/mbgl/renderer/render_symbol_layer.hpp | 2 +- src/mbgl/renderer/symbol_bucket.hpp | 2 ++ src/mbgl/sprite/sprite_atlas.cpp | 4 +-- src/mbgl/sprite/sprite_atlas.hpp | 8 ++--- src/mbgl/style/layers/symbol_layer_impl.hpp | 2 -- src/mbgl/style/style.cpp | 6 ---- src/mbgl/tile/geojson_tile.cpp | 6 +++- src/mbgl/tile/geometry_tile.cpp | 32 +++++++++---------- src/mbgl/tile/geometry_tile.hpp | 13 ++++---- src/mbgl/tile/geometry_tile_worker.cpp | 35 ++++++++------------- src/mbgl/tile/geometry_tile_worker.hpp | 8 ++--- src/mbgl/tile/vector_tile.cpp | 6 +++- 19 files changed, 78 insertions(+), 98 deletions(-) diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp index 049c900b9..88153f5fb 100644 --- a/src/mbgl/annotation/annotation_manager.cpp +++ b/src/mbgl/annotation/annotation_manager.cpp @@ -175,8 +175,6 @@ void AnnotationManager::updateStyle(Style& style) { layer->setIconAllowOverlap(true); layer->setIconIgnorePlacement(true); - layer->impl->spriteAtlas = &spriteAtlas; - style.addLayer(std::move(layer)); } diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp index 5e2994c44..0ab43bec1 100644 --- a/src/mbgl/annotation/annotation_manager.hpp +++ b/src/mbgl/annotation/annotation_manager.hpp @@ -75,6 +75,8 @@ private: std::unordered_set obsoleteShapeAnnotationLayers; std::unordered_set tiles; SpriteAtlas spriteAtlas; + + friend class AnnotationTile; }; } // namespace mbgl diff --git a/src/mbgl/annotation/annotation_tile.cpp b/src/mbgl/annotation/annotation_tile.cpp index 9d28f6778..fd182b70b 100644 --- a/src/mbgl/annotation/annotation_tile.cpp +++ b/src/mbgl/annotation/annotation_tile.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include @@ -10,7 +11,9 @@ namespace mbgl { AnnotationTile::AnnotationTile(const OverscaledTileID& overscaledTileID, const style::UpdateParameters& parameters) - : GeometryTile(overscaledTileID, AnnotationManager::SourceID, parameters), + : GeometryTile(overscaledTileID, AnnotationManager::SourceID, parameters, + *parameters.style.glyphAtlas, + parameters.annotationManager.spriteAtlas), annotationManager(parameters.annotationManager) { annotationManager.addTile(*this); } diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index 9aca316a0..aa91eb168 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -42,14 +42,12 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters, const std::vector& layers, const GeometryTileLayer& sourceLayer, IconDependencies& iconDependencies, - uintptr_t _spriteAtlasMapIndex, GlyphDependencies& glyphDependencies) : sourceLayerName(sourceLayer.getName()), bucketName(layers.at(0)->getID()), overscaling(parameters.tileID.overscaleFactor()), zoom(parameters.tileID.overscaledZ), mode(parameters.mode), - spriteAtlasMapIndex(_spriteAtlasMapIndex), tileSize(util::tileSize * overscaling), tilePixelRatio(float(util::EXTENT) / tileSize), textSize(layers.at(0)->as()->impl->layout.unevaluated.get()), @@ -171,7 +169,7 @@ bool SymbolLayout::hasSymbolInstances() const { return !symbolInstances.empty(); } -void SymbolLayout::prepare(const GlyphPositionMap& glyphs, const IconAtlasMap& iconMap) { +void SymbolLayout::prepare(const GlyphPositionMap& glyphs, const IconMap& icons) { float horizontalAlign = 0.5; float verticalAlign = 0.5; @@ -259,21 +257,18 @@ void SymbolLayout::prepare(const GlyphPositionMap& glyphs, const IconAtlasMap& i // if feature has icon, get sprite atlas position if (feature.icon) { - auto icons = iconMap.find(spriteAtlasMapIndex); - if (icons != iconMap.end()) { - auto image = icons->second.find(*feature.icon); - if (image != icons->second.end()) { - shapedIcon = PositionedIcon::shapeIcon(image->second, - layout.evaluate(zoom, feature), - layout.evaluate(zoom, feature) * util::DEG2RAD); - if (image->second.sdf) { - sdfIcons = true; - } - if (image->second.relativePixelRatio != 1.0f) { - iconsNeedLinear = true; - } else if (layout.get().constantOr(1) != 0) { - iconsNeedLinear = true; - } + auto image = icons.find(*feature.icon); + if (image != icons.end()) { + shapedIcon = PositionedIcon::shapeIcon(image->second, + layout.evaluate(zoom, feature), + layout.evaluate(zoom, feature) * util::DEG2RAD); + if (image->second.sdf) { + sdfIcons = true; + } + if (image->second.relativePixelRatio != 1.0f) { + iconsNeedLinear = true; + } else if (layout.get().constantOr(1) != 0) { + iconsNeedLinear = true; } } } diff --git a/src/mbgl/layout/symbol_layout.hpp b/src/mbgl/layout/symbol_layout.hpp index 55e409845..7d19aba67 100644 --- a/src/mbgl/layout/symbol_layout.hpp +++ b/src/mbgl/layout/symbol_layout.hpp @@ -31,10 +31,9 @@ public: const std::vector&, const GeometryTileLayer&, IconDependencies&, - uintptr_t, GlyphDependencies&); - void prepare(const GlyphPositionMap& glyphs, const IconAtlasMap& icons); + void prepare(const GlyphPositionMap& glyphs, const IconMap& icons); std::unique_ptr place(CollisionTile&); @@ -81,8 +80,6 @@ private: const MapMode mode; style::SymbolLayoutProperties::PossiblyEvaluated layout; - - uintptr_t spriteAtlasMapIndex; // Actually a pointer to the SpriteAtlas for this symbol's layer, but don't use it from worker threads! const uint32_t tileSize; const float tilePixelRatio; diff --git a/src/mbgl/renderer/painter_symbol.cpp b/src/mbgl/renderer/painter_symbol.cpp index 9bafb5df8..5a66d1e62 100644 --- a/src/mbgl/renderer/painter_symbol.cpp +++ b/src/mbgl/renderer/painter_symbol.cpp @@ -67,7 +67,7 @@ void Painter::renderSymbol(PaintParameters& parameters, auto values = layer.iconPropertyValues(layout); auto paintPropertyValues = layer.iconPaintProperties(); - SpriteAtlas& atlas = *layer.impl->spriteAtlas; + SpriteAtlas& atlas = *bucket.spriteAtlas; const bool iconScaled = layout.get().constantOr(1.0) != 1.0 || frame.pixelRatio != atlas.getPixelRatio() || bucket.iconsNeedLinear; diff --git a/src/mbgl/renderer/render_symbol_layer.cpp b/src/mbgl/renderer/render_symbol_layer.cpp index 90cc0a761..d46dc614f 100644 --- a/src/mbgl/renderer/render_symbol_layer.cpp +++ b/src/mbgl/renderer/render_symbol_layer.cpp @@ -26,12 +26,11 @@ std::unique_ptr RenderSymbolLayer::createLayout(const BucketParame const std::vector& group, const GeometryTileLayer& layer, GlyphDependencies& glyphDependencies, - IconDependencyMap& iconDependencyMap) const { + IconDependencies& iconDependencies) const { return std::make_unique(parameters, group, layer, - iconDependencyMap[impl->spriteAtlas], - (uintptr_t) impl->spriteAtlas, + iconDependencies, glyphDependencies); } diff --git a/src/mbgl/renderer/render_symbol_layer.hpp b/src/mbgl/renderer/render_symbol_layer.hpp index dbe7c4ea8..b19b236e9 100644 --- a/src/mbgl/renderer/render_symbol_layer.hpp +++ b/src/mbgl/renderer/render_symbol_layer.hpp @@ -77,7 +77,7 @@ public: std::unique_ptr createBucket(const BucketParameters&, const std::vector&) const override; std::unique_ptr createLayout(const BucketParameters&, const std::vector&, - const GeometryTileLayer&, GlyphDependencies&, IconDependencyMap&) const; + const GeometryTileLayer&, GlyphDependencies&, IconDependencies&) const; // Paint properties style::SymbolPaintProperties::Unevaluated unevaluated; diff --git a/src/mbgl/renderer/symbol_bucket.hpp b/src/mbgl/renderer/symbol_bucket.hpp index e00de03fd..f7e4bcfa2 100644 --- a/src/mbgl/renderer/symbol_bucket.hpp +++ b/src/mbgl/renderer/symbol_bucket.hpp @@ -70,6 +70,8 @@ public: optional> vertexBuffer; optional> indexBuffer; } collisionBox; + + SpriteAtlas* spriteAtlas = nullptr; }; } // namespace mbgl diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp index 9d499411f..fed8451ae 100644 --- a/src/mbgl/sprite/sprite_atlas.cpp +++ b/src/mbgl/sprite/sprite_atlas.cpp @@ -123,7 +123,7 @@ void SpriteAtlas::onParsed(Images&& result) { } observer->onSpriteLoaded(); for (auto requestor : requestors) { - requestor->onIconsAvailable(this, buildIconMap()); + requestor->onIconsAvailable(buildIconMap()); } requestors.clear(); } @@ -202,7 +202,7 @@ const style::Image* SpriteAtlas::getImage(const std::string& id) const { void SpriteAtlas::getIcons(IconRequestor& requestor) { if (isLoaded()) { - requestor.onIconsAvailable(this, buildIconMap()); + requestor.onIconsAvailable(buildIconMap()); } else { requestors.insert(&requestor); } diff --git a/src/mbgl/sprite/sprite_atlas.hpp b/src/mbgl/sprite/sprite_atlas.hpp index 15bce919b..5afddaebc 100644 --- a/src/mbgl/sprite/sprite_atlas.hpp +++ b/src/mbgl/sprite/sprite_atlas.hpp @@ -38,16 +38,12 @@ public: float height; }; -class SpriteAtlas; - -typedef std::map IconMap; +typedef std::map IconMap; typedef std::set IconDependencies; -typedef std::map IconAtlasMap; -typedef std::map IconDependencyMap; class IconRequestor { public: - virtual void onIconsAvailable(SpriteAtlas*, IconMap) = 0; + virtual void onIconsAvailable(IconMap) = 0; }; class SpriteAtlas : public util::noncopyable { diff --git a/src/mbgl/style/layers/symbol_layer_impl.hpp b/src/mbgl/style/layers/symbol_layer_impl.hpp index 28079fa73..df145647a 100644 --- a/src/mbgl/style/layers/symbol_layer_impl.hpp +++ b/src/mbgl/style/layers/symbol_layer_impl.hpp @@ -19,8 +19,6 @@ public: SymbolLayoutProperties layout; SymbolPaintProperties::Cascading cascading; - - SpriteAtlas* spriteAtlas = nullptr; }; } // namespace style diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index 14efd6a07..e8141c3c6 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -223,12 +223,6 @@ Layer* Style::addLayer(std::unique_ptr layer, optional befor throw std::runtime_error(std::string{"Layer "} + layer->getID() + " already exists"); } - if (SymbolLayer* symbolLayer = layer->as()) { - if (!symbolLayer->impl->spriteAtlas) { - symbolLayer->impl->spriteAtlas = spriteAtlas.get(); - } - } - if (CustomLayer* customLayer = layer->as()) { customLayer->impl->initialize(); } diff --git a/src/mbgl/tile/geojson_tile.cpp b/src/mbgl/tile/geojson_tile.cpp index 5a2a72f50..00e10d148 100644 --- a/src/mbgl/tile/geojson_tile.cpp +++ b/src/mbgl/tile/geojson_tile.cpp @@ -1,6 +1,8 @@ #include #include #include +#include +#include #include #include @@ -83,7 +85,9 @@ public: GeoJSONTile::GeoJSONTile(const OverscaledTileID& overscaledTileID, std::string sourceID_, const style::UpdateParameters& parameters) - : GeometryTile(overscaledTileID, sourceID_, parameters) { + : GeometryTile(overscaledTileID, sourceID_, parameters, + *parameters.style.glyphAtlas, + *parameters.style.spriteAtlas) { } void GeoJSONTile::updateData(const mapbox::geometry::feature_collection& features) { diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index b47519d95..6b768d9d5 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -28,7 +29,9 @@ using namespace style; GeometryTile::GeometryTile(const OverscaledTileID& id_, std::string sourceID_, - const style::UpdateParameters& parameters) + const style::UpdateParameters& parameters, + GlyphAtlas& glyphAtlas_, + SpriteAtlas& spriteAtlas_) : Tile(id_), sourceID(std::move(sourceID_)), style(parameters.style), @@ -38,14 +41,13 @@ GeometryTile::GeometryTile(const OverscaledTileID& id_, id_, obsolete, parameters.mode), - glyphAtlas(*parameters.style.glyphAtlas) { + glyphAtlas(glyphAtlas_), + spriteAtlas(spriteAtlas_) { } GeometryTile::~GeometryTile() { glyphAtlas.removeGlyphs(*this); - for (auto spriteAtlas : pendingSpriteAtlases) { - spriteAtlas->removeRequestor(*this); - } + spriteAtlas.removeRequestor(*this); cancel(); } @@ -125,6 +127,9 @@ void GeometryTile::onPlacement(PlacementResult result) { pending = false; } symbolBuckets = std::move(result.symbolBuckets); + for (auto& entry : symbolBuckets) { + dynamic_cast(entry.second.get())->spriteAtlas = &spriteAtlas; + } collisionTile = std::move(result.collisionTile); observer->onTileChanged(*this); } @@ -144,21 +149,12 @@ void GeometryTile::getGlyphs(GlyphDependencies glyphDependencies) { glyphAtlas.getGlyphs(*this, std::move(glyphDependencies)); } -void GeometryTile::onIconsAvailable(SpriteAtlas* spriteAtlas, IconMap icons) { - iconAtlasMap[(uintptr_t)spriteAtlas] = icons; - pendingSpriteAtlases.erase(spriteAtlas); - if (pendingSpriteAtlases.empty()) { - worker.invoke(&GeometryTileWorker::onIconsAvailable, std::move(iconAtlasMap)); - } +void GeometryTile::onIconsAvailable(IconMap icons) { + worker.invoke(&GeometryTileWorker::onIconsAvailable, std::move(icons)); } -// TODO: If there's any value to be gained by it, we can narrow our request to just the sprites -// we need, but SpriteAtlases are just "loaded" or "not loaded" -void GeometryTile::getIcons(IconDependencyMap iconDependencyMap) { - for (auto dependency : iconDependencyMap) { - pendingSpriteAtlases.insert(dependency.first); - dependency.first->getIcons(*this); - } +void GeometryTile::getIcons(IconDependencies) { + spriteAtlas.getIcons(*this); } Bucket* GeometryTile::getBucket(const RenderLayer& layer) const { diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 3913fca81..8aedd6706 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -30,7 +30,9 @@ class GeometryTile : public Tile, public GlyphRequestor, IconRequestor { public: GeometryTile(const OverscaledTileID&, std::string sourceID, - const style::UpdateParameters&); + const style::UpdateParameters&, + GlyphAtlas&, + SpriteAtlas&); ~GeometryTile() override; @@ -41,10 +43,10 @@ public: void redoLayout() override; void onGlyphsAvailable(GlyphPositionMap) override; - void onIconsAvailable(SpriteAtlas*, IconMap) override; + void onIconsAvailable(IconMap) override; void getGlyphs(GlyphDependencies); - void getIcons(IconDependencyMap); + void getIcons(IconDependencies); Bucket* getBucket(const RenderLayer&) const override; @@ -95,9 +97,8 @@ private: Actor worker; GlyphAtlas& glyphAtlas; - std::set pendingSpriteAtlases; - IconAtlasMap iconAtlasMap; - + SpriteAtlas& spriteAtlas; + uint64_t correlationID = 0; optional requestedConfig; diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index b745cccc8..550e06bae 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -216,14 +216,9 @@ void GeometryTileWorker::onGlyphsAvailable(GlyphPositionMap newGlyphPositions) { symbolDependenciesChanged(); } -void GeometryTileWorker::onIconsAvailable(IconAtlasMap newIcons) { - for (auto& atlasIcons : newIcons) { - auto pendingAtlasIcons = pendingIconDependencies.find((SpriteAtlas*)atlasIcons.first); - if (pendingAtlasIcons != pendingIconDependencies.end()) { - icons[atlasIcons.first] = std::move(newIcons[atlasIcons.first]); - pendingIconDependencies.erase((SpriteAtlas*)atlasIcons.first); - } - } +void GeometryTileWorker::onIconsAvailable(IconMap newIcons) { + icons = std::move(newIcons); + pendingIconDependencies.clear(); symbolDependenciesChanged(); } @@ -241,12 +236,8 @@ void GeometryTileWorker::requestNewGlyphs(const GlyphDependencies& glyphDependen } } -void GeometryTileWorker::requestNewIcons(const IconDependencyMap &iconDependencies) { - for (auto& atlasDependency : iconDependencies) { - if (icons.find((uintptr_t)atlasDependency.first) == icons.end()) { - pendingIconDependencies[atlasDependency.first] = IconDependencies(); - } - } +void GeometryTileWorker::requestNewIcons(const IconDependencies& iconDependencies) { + pendingIconDependencies = iconDependencies; if (!pendingIconDependencies.empty()) { parent.invoke(&GeometryTile::getIcons, pendingIconDependencies); } @@ -290,9 +281,9 @@ void GeometryTileWorker::redoLayout() { std::unordered_map> buckets; auto featureIndex = std::make_unique(); BucketParameters parameters { id, mode }; - + GlyphDependencies glyphDependencies; - IconDependencyMap iconDependencyMap; + IconDependencies iconDependencies; // Create render layers and group by layout std::vector> renderLayers = toRenderLayers(*layers, id.overscaledZ); @@ -323,7 +314,7 @@ void GeometryTileWorker::redoLayout() { if (leader.is()) { symbolLayoutMap.emplace(leader.getID(), - leader.as()->createLayout(parameters, group, *geometryLayer, glyphDependencies, iconDependencyMap)); + leader.as()->createLayout(parameters, group, *geometryLayer, glyphDependencies, iconDependencies)); } else { const Filter& filter = leader.baseImpl.filter; const std::string& sourceLayerID = leader.baseImpl.sourceLayer; @@ -357,9 +348,9 @@ void GeometryTileWorker::redoLayout() { symbolLayouts.push_back(std::move(it->second)); } } - + requestNewGlyphs(glyphDependencies); - requestNewIcons(iconDependencyMap); + requestNewIcons(iconDependencies); parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult { std::move(buckets), @@ -403,12 +394,12 @@ void GeometryTileWorker::attemptPlacement() { if (obsolete) { return; } - + if (symbolLayout->state == SymbolLayout::Pending) { - symbolLayout->prepare(glyphPositions,icons); + symbolLayout->prepare(glyphPositions, icons); symbolLayout->state = SymbolLayout::Placed; } - + if (!symbolLayout->hasSymbolInstances()) { continue; } diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp index ab3b9d740..1df1ef43c 100644 --- a/src/mbgl/tile/geometry_tile_worker.hpp +++ b/src/mbgl/tile/geometry_tile_worker.hpp @@ -37,7 +37,7 @@ public: void setPlacementConfig(PlacementConfig, uint64_t correlationID); void onGlyphsAvailable(GlyphPositionMap glyphs); - void onIconsAvailable(IconAtlasMap icons); + void onIconsAvailable(IconMap icons); private: void coalesced(); @@ -47,7 +47,7 @@ private: void coalesce(); void requestNewGlyphs(const GlyphDependencies&); - void requestNewIcons(const IconDependencyMap&); + void requestNewIcons(const IconDependencies&); void symbolDependenciesChanged(); bool hasPendingSymbolDependencies() const; @@ -77,9 +77,9 @@ private: std::vector> symbolLayouts; GlyphDependencies pendingGlyphDependencies; - IconDependencyMap pendingIconDependencies; + IconDependencies pendingIconDependencies; GlyphPositionMap glyphPositions; - IconAtlasMap icons; + IconMap icons; }; } // namespace mbgl diff --git a/src/mbgl/tile/vector_tile.cpp b/src/mbgl/tile/vector_tile.cpp index 4bb4d830e..277a17667 100644 --- a/src/mbgl/tile/vector_tile.cpp +++ b/src/mbgl/tile/vector_tile.cpp @@ -1,6 +1,8 @@ #include #include #include +#include +#include #include @@ -83,7 +85,9 @@ VectorTile::VectorTile(const OverscaledTileID& id_, std::string sourceID_, const style::UpdateParameters& parameters, const Tileset& tileset) - : GeometryTile(id_, sourceID_, parameters), + : GeometryTile(id_, sourceID_, parameters, + *parameters.style.glyphAtlas, + *parameters.style.spriteAtlas), loader(*this, id_, parameters, tileset) { }