Always request the SymbolBucket dependencies at the first pass

When processing a layer, we request the dependencies of all the
SymbolBuckets even if we are not processing it completely.
By doing that we are now requesting glyphs in parallel instead of
sequentially as we were doing before.

This patch is also simplifying the SymbolBucket creation code by getting
rid of the ugly "needsResources" return value passed by reference as
argument.

src/mbgl/map/tile_parser.cpp

@@ -118,18 +118,7 @@ std::unique_ptr<Bucket> TileParser::createBucket(const StyleBucket &bucketDesc)
         } else if (bucketDesc.type == StyleLayerType::Line) {
             return createLineBucket(*layer, bucketDesc);
         } else if (bucketDesc.type == StyleLayerType::Symbol) {
-            if (partialParse) {
-                return nullptr;
-            }
-
-            bool needsResources = false;
-            auto symbolBucket = createSymbolBucket(*layer, bucketDesc, needsResources);
-            if (needsResources) {
-                partialParse = true;
-                return nullptr;
-            } else {
-                return std::move(symbolBucket);
-            }
+            return createSymbolBucket(*layer, bucketDesc);
         } else if (bucketDesc.type == StyleLayerType::Raster) {
             return nullptr;
         } else {
@@ -190,13 +179,7 @@ std::unique_ptr<Bucket> TileParser::createLineBucket(const GeometryTileLayer& la
 }
 
 std::unique_ptr<Bucket> TileParser::createSymbolBucket(const GeometryTileLayer& layer,
-                                                       const StyleBucket& bucket_desc,
-                                                       bool& needsResources) {
-    if (!sprite->isLoaded()) {
-        needsResources = true;
-        return nullptr;
-    }
-
+                                                       const StyleBucket& bucket_desc) {
     auto bucket = util::make_unique<SymbolBucket>(*tile.getCollision());
 
     const float z = tile.id.z;
@@ -240,14 +223,21 @@ std::unique_ptr<Bucket> TileParser::createSymbolBucket(const GeometryTileLayer&
     applyLayoutProperty(PropertyKey::TextOffset, bucket_desc.layout, layout.text.offset, z);
     applyLayoutProperty(PropertyKey::TextAllowOverlap, bucket_desc.layout, layout.text.allow_overlap, z);
 
-    bucket->addFeatures(
-        layer, bucket_desc.filter, reinterpret_cast<uintptr_t>(&tile), spriteAtlas, *sprite, glyphAtlas, glyphStore);
+    if (bucket->needsDependencies(layer, bucket_desc.filter, glyphStore, *sprite)) {
+        partialParse = true;
+    }
 
-    if (bucket->needsGlyphs()) {
-        needsResources = true;
+    // We do not proceed if the parser is in a "partial" state because
+    // the layer ordering needs to be respected when calculating text
+    // collisions. Although, at this point, we requested all the resources
+    // needed by this tile.
+    if (partialParse) {
         return nullptr;
     }
 
+    bucket->addFeatures(
+        reinterpret_cast<uintptr_t>(&tile), spriteAtlas, *sprite, glyphAtlas, glyphStore);
+
     return bucket->hasData() ? std::move(bucket) : nullptr;
 }
 

src/mbgl/map/tile_parser.hpp

@@ -53,7 +53,7 @@ private:
     std::unique_ptr<Bucket> createBucket(const StyleBucket&);
     std::unique_ptr<Bucket> createFillBucket(const GeometryTileLayer&, const StyleBucket&);
     std::unique_ptr<Bucket> createLineBucket(const GeometryTileLayer&, const StyleBucket&);
-    std::unique_ptr<Bucket> createSymbolBucket(const GeometryTileLayer&, const StyleBucket&, bool& needsResources);
+    std::unique_ptr<Bucket> createSymbolBucket(const GeometryTileLayer&, const StyleBucket&);
 
     template <class Bucket>
     void addBucketGeometries(Bucket&, const GeometryTileLayer&, const FilterExpression&);

src/mbgl/renderer/symbol_bucket.cpp

@@ -29,7 +29,7 @@
 namespace mbgl {
 
 SymbolBucket::SymbolBucket(Collision &collision_)
-    : collision(collision_), needsGlyphs_(false) {
+    : collision(collision_) {
 }
 
 SymbolBucket::~SymbolBucket() {
@@ -62,16 +62,15 @@ bool SymbolBucket::hasTextData() const { return !text.groups.empty(); }
 
 bool SymbolBucket::hasIconData() const { return !icon.groups.empty(); }
 
-std::vector<SymbolFeature> SymbolBucket::processFeatures(const GeometryTileLayer& layer,
-                                                         const FilterExpression& filter,
-                                                         GlyphStore &glyphStore) {
+bool SymbolBucket::needsDependencies(const GeometryTileLayer& layer,
+                                     const FilterExpression& filter,
+                                     GlyphStore& glyphStore,
+                                     Sprite& sprite) {
     const bool has_text = !layout.text.field.empty() && !layout.text.font.empty();
     const bool has_icon = !layout.icon.image.empty();
 
-    std::vector<SymbolFeature> features;
-
     if (!has_text && !has_icon) {
-        return features;
+        return false;
     }
 
     // Determine and load glyph ranges
@@ -135,28 +134,21 @@ std::vector<SymbolFeature> SymbolBucket::processFeatures(const GeometryTileLayer
     }
 
     if (glyphStore.requestGlyphRangesIfNeeded(layout.text.font, ranges)) {
-        needsGlyphs_ = true;
-        return {};
+        return true;
     }
 
-    return features;
+    if (!sprite.isLoaded()) {
+        return true;
+    }
+
+    return false;
 }
 
-void SymbolBucket::addFeatures(const GeometryTileLayer& layer,
-                               const FilterExpression& filter,
-                               uintptr_t tileUID,
+void SymbolBucket::addFeatures(uintptr_t tileUID,
                                SpriteAtlas& spriteAtlas,
                                Sprite& sprite,
                                GlyphAtlas& glyphAtlas,
                                GlyphStore& glyphStore) {
-    const std::vector<SymbolFeature> features = processFeatures(layer, filter, glyphStore);
-
-    // Stop if we still need glyphs because the
-    // bucket will be discarded.
-    if (needsGlyphs()) {
-        return;
-    }
-
     float horizontalAlign = 0.5;
     float verticalAlign = 0.5;
 
@@ -244,6 +236,8 @@ void SymbolBucket::addFeatures(const GeometryTileLayer& layer,
             }
         }
     }
+
+    features.clear();
 }
 
 bool byScale(const Anchor &a, const Anchor &b) { return a.scale < b.scale; }

src/mbgl/renderer/symbol_bucket.hpp

@@ -61,25 +61,22 @@ public:
     bool hasTextData() const;
     bool hasIconData() const;
 
-    void addFeatures(const GeometryTileLayer&,
-                     const FilterExpression&,
-                     uintptr_t tileUID,
+    void addFeatures(uintptr_t tileUID,
                      SpriteAtlas&,
                      Sprite&,
                      GlyphAtlas&,
                      GlyphStore&);
 
-    inline bool needsGlyphs() const { return needsGlyphs_; }
-
     void drawGlyphs(SDFShader& shader);
     void drawIcons(SDFShader& shader);
     void drawIcons(IconShader& shader);
 
-private:
-    std::vector<SymbolFeature> processFeatures(const GeometryTileLayer&,
-                                               const FilterExpression&,
-                                               GlyphStore&);
+    bool needsDependencies(const GeometryTileLayer&,
+                           const FilterExpression&,
+                           GlyphStore&,
+                           Sprite&);
 
+private:
     void addFeature(const std::vector<Coordinate> &line, const Shaping &shaping, const GlyphPositions &face, const Rect<uint16_t> &image);
 
     // Adds placed items to the buffer.
@@ -92,6 +89,7 @@ public:
 
 private:
     Collision &collision;
+    std::vector<SymbolFeature> features;
 
     struct TextBuffer {
         TextVertexBuffer vertices;
@@ -104,9 +102,8 @@ private:
         TriangleElementsBuffer triangles;
         std::vector<std::unique_ptr<IconElementGroup>> groups;
     } icon;
-
-    bool needsGlyphs_;
 };
+
 }
 
 #endif