[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.

6 files changed, 48 insertions, 52 deletions

diff --git a/src/mbgl/tile/geojson_tile.cpp b/src/mbgl/tile/geojson_tile.cpp
index 5a2a72f50b..00e10d148a 100644
--- a/src/mbgl/tile/geojson_tile.cpp
+++ b/src/mbgl/tile/geojson_tile.cpp
@@ -1,6 +1,8 @@
 #include <mbgl/tile/geojson_tile.hpp>
 #include <mbgl/tile/geometry_tile_data.hpp>
 #include <mbgl/style/query.hpp>
+#include <mbgl/style/style.hpp>
+#include <mbgl/style/update_parameters.hpp>
 
 #include <mapbox/geojsonvt.hpp>
 #include <supercluster.hpp>
@@ -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<int16_t>& features) {
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index b47519d95c..6b768d9d5b 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -9,6 +9,7 @@
 #include <mbgl/renderer/render_background_layer.hpp>
 #include <mbgl/renderer/render_custom_layer.hpp>
 #include <mbgl/renderer/render_symbol_layer.hpp>
+#include <mbgl/renderer/symbol_bucket.hpp>
 #include <mbgl/style/style.hpp>
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/geometry/feature_index.hpp>
@@ -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<SymbolBucket*>(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 3913fca812..8aedd67060 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<GeometryTileWorker> worker;
 
     GlyphAtlas& glyphAtlas;
-    std::set<SpriteAtlas*> pendingSpriteAtlases;
-    IconAtlasMap iconAtlasMap;
-    
+    SpriteAtlas& spriteAtlas;
+
     uint64_t correlationID = 0;
     optional<PlacementConfig> requestedConfig;
 
diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp
index b745cccc89..550e06bae2 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<std::string, std::shared_ptr<Bucket>> buckets;
     auto featureIndex = std::make_unique<FeatureIndex>();
     BucketParameters parameters { id, mode };
-    
+
     GlyphDependencies glyphDependencies;
-    IconDependencyMap iconDependencyMap;
+    IconDependencies iconDependencies;
 
     // Create render layers and group by layout
     std::vector<std::unique_ptr<RenderLayer>> renderLayers = toRenderLayers(*layers, id.overscaledZ);
@@ -323,7 +314,7 @@ void GeometryTileWorker::redoLayout() {
 
         if (leader.is<RenderSymbolLayer>()) {
             symbolLayoutMap.emplace(leader.getID(),
-                leader.as<RenderSymbolLayer>()->createLayout(parameters, group, *geometryLayer, glyphDependencies, iconDependencyMap));
+                leader.as<RenderSymbolLayer>()->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 ab3b9d7403..1df1ef43c4 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<std::unique_ptr<SymbolLayout>> 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 4bb4d830ec..277a176670 100644
--- a/src/mbgl/tile/vector_tile.cpp
+++ b/src/mbgl/tile/vector_tile.cpp
@@ -1,6 +1,8 @@
 #include <mbgl/tile/vector_tile.hpp>
 #include <mbgl/tile/tile_loader_impl.hpp>
 #include <mbgl/tile/geometry_tile_data.hpp>
+#include <mbgl/style/style.hpp>
+#include <mbgl/style/update_parameters.hpp>
 
 #include <protozero/pbf_reader.hpp>
 
@@ -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) {
 }