[core] make{Glyph,Image}Atlas only once for any number of symbol layersupstream/optimze-symbol-atlases

3 files changed, 24 insertions, 34 deletions

diff --git a/src/mbgl/layout/symbol_layout.hpp b/src/mbgl/layout/symbol_layout.hpp
index 8cdaadff00..4ee52e843f 100644
--- a/src/mbgl/layout/symbol_layout.hpp
+++ b/src/mbgl/layout/symbol_layout.hpp
@@ -40,13 +40,6 @@ public:
 
     bool hasSymbolInstances() const;
 
-    enum State {
-        Pending,  // Waiting for the necessary glyphs or icons to be available.
-        Placed    // The final positions have been determined, taking into account prior layers.
-    };
-
-    State state = Pending;
-
     std::map<std::string,
         std::pair<style::IconPaintProperties::PossiblyEvaluated, style::TextPaintProperties::PossiblyEvaluated>> layerPaintProperties;
 
diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp
index 12bb84d7e3..c622d82e31 100644
--- a/src/mbgl/tile/geometry_tile_worker.cpp
+++ b/src/mbgl/tile/geometry_tile_worker.cpp
@@ -144,14 +144,14 @@ void GeometryTileWorker::symbolDependenciesChanged() {
     try {
         switch (state) {
         case Idle:
-            if (hasPendingSymbolLayouts()) {
+            if (symbolLayoutsNeedPreparation) {
                 attemptPlacement();
                 coalesce();
             }
             break;
 
         case Coalescing:
-            if (hasPendingSymbolLayouts()) {
+            if (symbolLayoutsNeedPreparation) {
                 state = NeedPlacement;
             }
             break;
@@ -312,6 +312,7 @@ void GeometryTileWorker::redoLayout() {
             auto layout = leader.as<RenderSymbolLayer>()->createLayout(
                 parameters, group, std::move(geometryLayer), glyphDependencies, imageDependencies);
             symbolLayoutMap.emplace(leader.getID(), std::move(layout));
+            symbolLayoutsNeedPreparation = true;
         } else {
             const Filter& filter = leader.baseImpl->filter;
             const std::string& sourceLayerID = leader.baseImpl->sourceLayer;
@@ -359,16 +360,6 @@ void GeometryTileWorker::redoLayout() {
     attemptPlacement();
 }
 
-bool GeometryTileWorker::hasPendingSymbolLayouts() const {
-    for (const auto& symbolLayout : symbolLayouts) {
-        if (symbolLayout->state == SymbolLayout::Pending) {
-            return true;
-        }
-    }
-
-    return false;
-}
-
 bool GeometryTileWorker::hasPendingSymbolDependencies() const {
     for (auto& glyphDependency : pendingGlyphDependencies) {
         if (!glyphDependency.second.empty()) {
@@ -378,33 +369,39 @@ bool GeometryTileWorker::hasPendingSymbolDependencies() const {
     return !pendingImageDependencies.empty();
 }
 
-
 void GeometryTileWorker::attemptPlacement() {
     if (!data || !layers || !placementConfig || hasPendingSymbolDependencies()) {
         return;
     }
     
-    auto collisionTile = std::make_unique<CollisionTile>(*placementConfig);
-    std::unordered_map<std::string, std::shared_ptr<Bucket>> buckets;
-
     optional<AlphaImage> glyphAtlasImage;
     optional<PremultipliedImage> iconAtlasImage;
 
-    for (auto& symbolLayout : symbolLayouts) {
-        if (obsolete) {
-            return;
-        }
+    if (symbolLayoutsNeedPreparation) {
+        GlyphAtlas glyphAtlas = makeGlyphAtlas(glyphMap);
+        ImageAtlas imageAtlas = makeImageAtlas(imageMap);
+
+        glyphAtlasImage = std::move(glyphAtlas.image);
+        iconAtlasImage = std::move(imageAtlas.image);
 
-        if (symbolLayout->state == SymbolLayout::Pending) {
-            GlyphAtlas glyphAtlas = makeGlyphAtlas(glyphMap);
-            ImageAtlas imageAtlas = makeImageAtlas(imageMap);
+        for (auto& symbolLayout : symbolLayouts) {
+            if (obsolete) {
+                return;
+            }
 
             symbolLayout->prepare(glyphMap, glyphAtlas.positions,
                                   imageMap, imageAtlas.positions);
-            symbolLayout->state = SymbolLayout::Placed;
+        }
+
+        symbolLayoutsNeedPreparation = false;
+    }
 
-            glyphAtlasImage = std::move(glyphAtlas.image);
-            iconAtlasImage = std::move(imageAtlas.image);
+    auto collisionTile = std::make_unique<CollisionTile>(*placementConfig);
+    std::unordered_map<std::string, std::shared_ptr<Bucket>> buckets;
+
+    for (auto& symbolLayout : symbolLayouts) {
+        if (obsolete) {
+            return;
         }
 
         if (!symbolLayout->hasSymbolInstances()) {
diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp
index 194477e7b8..7f80c3b4f7 100644
--- a/src/mbgl/tile/geometry_tile_worker.hpp
+++ b/src/mbgl/tile/geometry_tile_worker.hpp
@@ -52,7 +52,6 @@ private:
    
     void symbolDependenciesChanged();
     bool hasPendingSymbolDependencies() const;
-    bool hasPendingSymbolLayouts() const;
 
     ActorRef<GeometryTileWorker> self;
     ActorRef<GeometryTile> parent;
@@ -77,6 +76,7 @@ private:
     optional<std::unique_ptr<const GeometryTileData>> data;
     optional<PlacementConfig> placementConfig;
 
+    bool symbolLayoutsNeedPreparation = false;
     std::vector<std::unique_ptr<SymbolLayout>> symbolLayouts;
     GlyphDependencies pendingGlyphDependencies;
     ImageDependencies pendingImageDependencies;