[core] De-mutex SpriteAtlas.

 - Consolidate symbol dependency tracking code.

src/mbgl/layout/symbol_layout.cpp

@@ -32,14 +32,15 @@ using namespace style;
 SymbolLayout::SymbolLayout(const BucketParameters& parameters,
                            const std::vector<const Layer*>& layers,
                            const GeometryTileLayer& sourceLayer,
-                           SpriteAtlas& spriteAtlas_,
+                           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),
-      spriteAtlas(spriteAtlas_),
+      spriteAtlasMapIndex(_spriteAtlasMapIndex),
       tileSize(util::tileSize * overscaling),
       tilePixelRatio(float(util::EXTENT) / tileSize) {
 
@@ -148,6 +149,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters,
 
         if (hasIcon) {
             ft.icon = util::replaceTokens(layout.get<IconImage>(), getValue);
+            iconDependencies.insert(*ft.icon);
         }
 
         if (ft.text || ft.icon) {
@@ -164,24 +166,7 @@ bool SymbolLayout::hasSymbolInstances() const {
     return !symbolInstances.empty();
 }
 
-bool SymbolLayout::canPrepare(const GlyphPositionMap& glyphPositions) {
-    // TODO: This is a needlessly complex way to check if we can move to the next step, we really just want to wait until
-    // we've gotten a reply from 'getGlyphs'. I'm just keeping this in place here to reduce the number of moving parts in the refactor
-    const bool hasTextField = layout.get<TextField>().match([&] (const std::string& s) { return !s.empty(); },
-                                                            [&] (const auto&) { return true; } );
-
-    if (hasTextField && glyphPositions.empty()) {
-        return false;
-    }
-
-    if (!layout.get<IconImage>().empty() && !spriteAtlas.isLoaded()) {
-        return false;
-    }
-
-    return true;
-}
-
-void SymbolLayout::prepare(const GlyphPositionMap& glyphs) {
+void SymbolLayout::prepare(const GlyphPositionMap& glyphs, const IconAtlasMap& iconMap) {
     float horizontalAlign = 0.5;
     float verticalAlign = 0.5;
 
@@ -269,19 +254,21 @@ void SymbolLayout::prepare(const GlyphPositionMap& glyphs) {
 
         // if feature has icon, get sprite atlas position
         if (feature.icon) {
-            auto image = spriteAtlas.getIcon(*feature.icon);
-            if (image) {
-                shapedIcon = shapeIcon(*image,
-                    layout.evaluate<IconOffset>(zoom, feature),
-                    layout.evaluate<IconRotate>(zoom, feature) * util::DEG2RAD);
-                assert((*image).spriteImage);
-                if ((*image).spriteImage->sdf) {
-                    sdfIcons = true;
-                }
-                if ((*image).relativePixelRatio != 1.0f) {
-                    iconsNeedLinear = true;
-                } else if (layout.get<IconRotate>().constantOr(1) != 0) {
-                    iconsNeedLinear = true;
+            auto icons = iconMap.find(spriteAtlasMapIndex);
+            if (icons != iconMap.end()) {
+                auto image = icons->second.find(*feature.icon);
+                if (image != icons->second.end()) {
+                    shapedIcon = shapeIcon(image->second,
+                        layout.evaluate<IconOffset>(zoom, feature),
+                        layout.evaluate<IconRotate>(zoom, feature) * util::DEG2RAD);
+                    if (image->second.sdf) {
+                        sdfIcons = true;
+                    }
+                    if (image->second.relativePixelRatio != 1.0f) {
+                        iconsNeedLinear = true;
+                    } else if (layout.get<IconRotate>().constantOr(1) != 0) {
+                        iconsNeedLinear = true;
+                    }
                 }
             }
         }

src/mbgl/layout/symbol_layout.hpp

@@ -16,8 +16,6 @@ namespace mbgl {
 
 class GeometryTileLayer;
 class CollisionTile;
-class SpriteAtlas;
-class GlyphAtlas;
 class SymbolBucket;
 class Anchor;
 
@@ -32,20 +30,18 @@ class SymbolLayout {
     SymbolLayout(const style::BucketParameters&,
                  const std::vector<const style::Layer*>&,
                  const GeometryTileLayer&,
-                 SpriteAtlas&,
+                 IconDependencies&,
+                 uintptr_t,
                  GlyphDependencies&);
 
-    bool canPrepare(const GlyphPositionMap& glyphs);
-
-    void prepare(const GlyphPositionMap& glyphs);
+    void prepare(const GlyphPositionMap& glyphs, const IconAtlasMap& icons);
 
     std::unique_ptr<SymbolBucket> place(CollisionTile&);
 
     bool hasSymbolInstances() const;
 
     enum State {
         Pending,  // Waiting for the necessary glyphs or icons to be available.
-        Prepared, // The potential positions of text and icons have been determined.
         Placed    // The final positions have been determined, taking into account prior layers.
     };
 
@@ -80,8 +76,8 @@ class SymbolLayout {
 
     style::SymbolLayoutProperties::Evaluated layout;
     float textMaxSize;
-
-    SpriteAtlas& spriteAtlas;
+    
+    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;

src/mbgl/sprite/sprite_atlas.cpp

@@ -28,11 +28,13 @@ struct SpriteAtlas::Loader {
 };
 
 SpriteAtlasElement::SpriteAtlasElement(Rect<uint16_t> rect_,
-                                       std::shared_ptr<const SpriteImage> image_,
+                                       std::shared_ptr<const SpriteImage> spriteImage,
                                        Size size_, float pixelRatio)
     : pos(std::move(rect_)),
-      spriteImage(std::move(image_)),
-      relativePixelRatio(spriteImage->pixelRatio / pixelRatio) {
+      sdf(spriteImage->sdf),
+      relativePixelRatio(spriteImage->pixelRatio / pixelRatio),
+      width(spriteImage->getWidth()),
+      height(spriteImage->getHeight()) {
 
     const float padding = 1;
 
@@ -105,6 +107,10 @@ void SpriteAtlas::emitSpriteLoadedIfComplete() {
         loaded = true;
         setSprites(result.get<Sprites>());
         observer->onSpriteLoaded();
+        for (auto requestor : requestors) {
+            requestor->onIconsAvailable(this, buildIconMap());
+        }
+        requestors.clear();
     } else {
         observer->onSpriteError(result.get<std::exception_ptr>());
     }
@@ -119,20 +125,16 @@ void SpriteAtlas::dumpDebugLogs() const {
 }
 
 void SpriteAtlas::setSprites(const Sprites& newSprites) {
-    std::lock_guard<std::mutex> lock(mutex);
     for (const auto& pair : newSprites) {
         _setSprite(pair.first, pair.second);
     }
 }
 
 void SpriteAtlas::setSprite(const std::string& name, std::shared_ptr<const SpriteImage> sprite) {
-    std::lock_guard<std::mutex> lock(mutex);
     _setSprite(name, sprite);
 }
 
 void SpriteAtlas::removeSprite(const std::string& name) {
-    std::lock_guard<std::mutex> lock(mutex);
-
     auto it = entries.find(name);
     if (it == entries.end()) {
         return;
@@ -184,7 +186,6 @@ void SpriteAtlas::_setSprite(const std::string& name,
 }
 
 std::shared_ptr<const SpriteImage> SpriteAtlas::getSprite(const std::string& name) {
-    std::lock_guard<std::mutex> lock(mutex);
     const auto it = entries.find(name);
     if (it != entries.end()) {
         return it->second.spriteImage;
@@ -196,6 +197,18 @@ std::shared_ptr<const SpriteImage> SpriteAtlas::getSprite(const std::string& nam
     }
 }
 
+void SpriteAtlas::getIcons(IconRequestor& requestor) {
+    if (isLoaded()) {
+        requestor.onIconsAvailable(this, buildIconMap());
+    } else {
+        requestors.insert(&requestor);
+    }
+}
+
+void SpriteAtlas::removeRequestor(IconRequestor& requestor) {
+    requestors.erase(&requestor);
+}
+
 optional<SpriteAtlasElement> SpriteAtlas::getIcon(const std::string& name) {
     return getImage(name, &Entry::iconRect);
 }
@@ -206,7 +219,6 @@ optional<SpriteAtlasElement> SpriteAtlas::getPattern(const std::string& name) {
 
 optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::string& name,
                                                    optional<Rect<uint16_t>> Entry::*entryRect) {
-    std::lock_guard<std::mutex> lock(mutex);
 
     auto it = entries.find(name);
     if (it == entries.end()) {
@@ -219,6 +231,7 @@ optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::string& name,
     Entry& entry = it->second;
 
     if (entry.*entryRect) {
+        assert(entry.spriteImage.get());
         return SpriteAtlasElement {
             *(entry.*entryRect),
             entry.spriteImage,
@@ -285,6 +298,17 @@ void SpriteAtlas::copy(const Entry& entry, optional<Rect<uint16_t>> Entry::*entr
     dirty = true;
 }
 
+IconMap SpriteAtlas::buildIconMap() {
+    IconMap icons;
+    for (auto entry : entries) {
+        icons.emplace(std::piecewise_construct,
+                      std::forward_as_tuple(entry.first),
+                      std::forward_as_tuple(*getIcon(entry.first)));
+
+    }
+    return icons;
+}
+
 void SpriteAtlas::upload(gl::Context& context, gl::TextureUnit unit) {
     if (!texture) {
         texture = context.createTexture(image, unit);

src/mbgl/sprite/sprite_atlas.hpp

@@ -6,10 +6,9 @@
 #include <mbgl/util/optional.hpp>
 #include <mbgl/sprite/sprite_image.hpp>
 
-#include <atomic>
 #include <string>
 #include <map>
-#include <mutex>
+#include <set>
 #include <unordered_map>
 #include <array>
 #include <memory>
@@ -28,12 +27,26 @@ class SpriteAtlasElement {
     SpriteAtlasElement(Rect<uint16_t>, std::shared_ptr<const SpriteImage>, Size size, float pixelRatio);
 
     Rect<uint16_t> pos;
-    std::shared_ptr<const SpriteImage> spriteImage;
+    bool sdf;
 
     float relativePixelRatio;
     std::array<float, 2> size;
     std::array<float, 2> tl;
     std::array<float, 2> br;
+    float width;
+    float height;
+};
+
+class SpriteAtlas;
+
+typedef std::map<std::string,SpriteAtlasElement> IconMap;
+typedef std::set<std::string> IconDependencies;
+typedef std::map<uintptr_t,IconMap> IconAtlasMap;
+typedef std::map<SpriteAtlas*,IconDependencies> IconDependencyMap;
+
+class IconRequestor {
+public:
+    virtual void onIconsAvailable(SpriteAtlas*, IconMap) = 0;
 };
 
 class SpriteAtlas : public util::noncopyable {
@@ -55,8 +68,11 @@ class SpriteAtlas : public util::noncopyable {
 
     void setSprite(const std::string&, std::shared_ptr<const SpriteImage>);
     void removeSprite(const std::string&);
+
+    std::shared_ptr<const SpriteImage> getSprite(const std::string& name);
 
-    std::shared_ptr<const SpriteImage> getSprite(const std::string&);
+    void getIcons(IconRequestor& requestor);
+    void removeRequestor(IconRequestor& requestor);
 
     optional<SpriteAtlasElement> getIcon(const std::string& name);
     optional<SpriteAtlasElement> getPattern(const std::string& name);
@@ -105,13 +121,16 @@ class SpriteAtlas : public util::noncopyable {
 
     optional<SpriteAtlasElement> getImage(const std::string& name, optional<Rect<uint16_t>> Entry::*rect);
     void copy(const Entry&, optional<Rect<uint16_t>> Entry::*rect);
+
+    IconMap buildIconMap();
 
-    std::mutex mutex;
     std::unordered_map<std::string, Entry> entries;
     BinPack<uint16_t> bin;
     PremultipliedImage image;
     mbgl::optional<gl::Texture> texture;
-    std::atomic<bool> dirty;
+    bool dirty;
+
+    std::set<IconRequestor*> requestors;
 };
 
 } // namespace mbgl

src/mbgl/style/layers/symbol_layer_impl.cpp

@@ -37,11 +37,13 @@ std::unique_ptr<Bucket> SymbolLayer::Impl::createBucket(const BucketParameters&,
 std::unique_ptr<SymbolLayout> SymbolLayer::Impl::createLayout(const BucketParameters& parameters,
                                                               const std::vector<const Layer*>& group,
                                                               const GeometryTileLayer& layer,
-                                                              GlyphDependencies& glyphDependencies) const {
+                                                              GlyphDependencies& glyphDependencies,
+                                                              IconDependencyMap& iconDependencyMap) const {
     return std::make_unique<SymbolLayout>(parameters,
                                           group,
                                           layer,
-                                          *spriteAtlas,
+                                          iconDependencyMap[spriteAtlas],
+                                          (uintptr_t)spriteAtlas,
                                           glyphDependencies);
 }
 

src/mbgl/style/layers/symbol_layer_impl.hpp

@@ -2,13 +2,13 @@
 
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/util/variant.hpp>
+#include <mbgl/sprite/sprite_atlas.hpp>
 #include <mbgl/style/layer_impl.hpp>
 #include <mbgl/style/layers/symbol_layer.hpp>
 #include <mbgl/style/layers/symbol_layer_properties.hpp>
 
 namespace mbgl {
 
-class SpriteAtlas;
 class SymbolLayout;
 
 namespace style {
@@ -68,7 +68,7 @@ class SymbolLayer::Impl : public Layer::Impl {
 
     std::unique_ptr<Bucket> createBucket(const BucketParameters&, const std::vector<const Layer*>&) const override;
     std::unique_ptr<SymbolLayout> createLayout(const BucketParameters&, const std::vector<const Layer*>&,
-                                               const GeometryTileLayer&, GlyphDependencies&) const;
+                                               const GeometryTileLayer&, GlyphDependencies&, IconDependencyMap&) const;
 
     IconPaintProperties::Evaluated iconPaintProperties() const;
     TextPaintProperties::Evaluated textPaintProperties() const;

src/mbgl/style/source_impl.cpp

@@ -191,12 +191,6 @@ void Source::Impl::removeTiles() {
     }
 }
 
-void Source::Impl::updateSymbolDependentTiles() {
-    for (auto& pair : tiles) {
-        pair.second->symbolDependenciesChanged();
-    }
-}
-
 void Source::Impl::reloadTiles() {
     cache.clear();
 

src/mbgl/style/source_impl.hpp

@@ -48,10 +48,6 @@ class Source::Impl : public TileObserver, private util::noncopyable {
     // trigger re-placement of existing complete tiles.
     void updateTiles(const UpdateParameters&);
 
-    // Called when icons or glyphs are loaded. Triggers further processing of tiles which
-    // were waiting on such dependencies.
-    void updateSymbolDependentTiles();
-
     // Removes all tiles (by putting them into the cache).
     void removeTiles();