Only emit tile updated signals when the tile really changes

We only emit signals when the number of buckets increase (i.e. the
new data that arrived added a new bucket to the tile).

Also, if we try to reparse a partial tile but it is not possible, we
keep the flag up for trying to reparse it again on the next update,
because the data might have arrived after the bucket that is waiting
for the data was processed but before the end of the parsing.

Fixes #1458

7 files changed, 59 insertions, 17 deletions

diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp
index 7b5ebd19df..2be86dc5e2 100644
--- a/src/mbgl/map/resource_loader.cpp
+++ b/src/mbgl/map/resource_loader.cpp
@@ -83,12 +83,19 @@ void ResourceLoader::update(MapData& data,
         spriteAtlas.setSprite(sprite_);
     }
 
+    bool allTilesUpdated = true;
     for (const auto& source : style_->sources) {
-        source->update(data, transform, *style_, glyphAtlas, *glyphStore_,
-                       spriteAtlas, sprite_, texturePool, shouldReparsePartialTiles_);
+        if (!source->update(data, transform, *style_, glyphAtlas, *glyphStore_,
+                       spriteAtlas, sprite_, texturePool, shouldReparsePartialTiles_)) {
+            allTilesUpdated = false;
+        }
     }
 
-    shouldReparsePartialTiles_ = false;
+    // We can only stop updating "partial" tiles when all of them
+    // were notified of the arrival of the new resources.
+    if (allTilesUpdated) {
+        shouldReparsePartialTiles_ = false;
+    }
 }
 
 void ResourceLoader::onGlyphRangeLoaded() {
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index e40cb0f16b..e8dc3468d3 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -219,21 +219,30 @@ TileData::State Source::hasTile(const TileID& id) {
     return TileData::State::invalid;
 }
 
-void Source::handlePartialTile(const TileID& id, Worker& worker) {
+bool Source::handlePartialTile(const TileID& id, Worker& worker) {
     const TileID normalized_id = id.normalized();
 
     auto it = tile_data.find(normalized_id);
     if (it == tile_data.end()) {
-        return;
+        return true;
     }
 
     util::ptr<TileData> data = it->second.lock();
     if (!data) {
-        return;
+        return true;
     }
 
-    auto callback = std::bind(&Source::emitTileLoaded, this, false);
-    data->reparse(worker, callback);
+    // The signal is only emitted if there was an actual change on the tile. The
+    // tile can be in a "partial" state waiting for resources and get reparsed on
+    // the arrival of new resources that were needed by another tile.
+    size_t bucketCount = static_cast<VectorTileData*>(data.get())->countBuckets();
+    auto callback = [this, data, bucketCount]() {
+        if (static_cast<VectorTileData*>(data.get())->countBuckets() > bucketCount) {
+            emitTileLoaded(false);
+        }
+    };
+
+    return data->reparse(worker, callback);
 }
 
 TileData::State Source::addTile(MapData& data,
@@ -377,7 +386,7 @@ bool Source::findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::fo
     return false;
 }
 
-void Source::update(MapData& data,
+bool Source::update(MapData& data,
                     const TransformState& transformState,
                     Style& style,
                     GlyphAtlas& glyphAtlas,
@@ -386,8 +395,10 @@ void Source::update(MapData& data,
                     util::ptr<Sprite> sprite,
                     TexturePool& texturePool,
                     bool shouldReparsePartialTiles) {
+    bool allTilesUpdated = true;
+
     if (!loaded || data.getAnimationTime() <= updated) {
-        return;
+        return allTilesUpdated;
     }
 
     int32_t zoom = std::floor(getZoom(transformState));
@@ -408,8 +419,11 @@ void Source::update(MapData& data,
 
         switch (state) {
         case TileData::State::partial:
-            if (shouldReparsePartialTiles)
-                handlePartialTile(id, style.workers);
+            if (shouldReparsePartialTiles) {
+                if (!handlePartialTile(id, style.workers)) {
+                    allTilesUpdated = false;
+                }
+            }
             break;
         case TileData::State::invalid:
             state = addTile(data, transformState, style, glyphAtlas, glyphStore,
@@ -484,6 +498,8 @@ void Source::update(MapData& data,
     updateTilePtrs();
 
     updated = data.getAnimationTime();
+
+    return allTilesUpdated;
 }
 
 void Source::invalidateTiles(const std::vector<TileID>& ids) {
diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp
index fcb1b41839..9d3de81b70 100644
--- a/src/mbgl/map/source.hpp
+++ b/src/mbgl/map/source.hpp
@@ -68,7 +68,12 @@ public:
     bool isLoaded() const;
 
     void load(MapData&, Environment&, std::function<void()> callback);
-    void update(MapData&,
+
+    // Request or parse all the tiles relevant for the "TransformState". This method
+    // will return true if all the tiles were scheduled for updating of false if
+    // they were not. shouldReparsePartialTiles must be set to "true" if there is
+    // new data available that a tile in the "partial" state might be interested at.
+    bool update(MapData&,
                 const TransformState&,
                 Style&,
                 GlyphAtlas&,
@@ -99,7 +104,7 @@ private:
     void emitSourceLoaded();
     void emitTileLoaded(bool isNewTile);
 
-    void handlePartialTile(const TileID &id, Worker &worker);
+    bool handlePartialTile(const TileID &id, Worker &worker);
     bool findLoadedChildren(const TileID& id, int32_t maxCoveringZoom, std::forward_list<TileID>& retain);
     bool findLoadedParent(const TileID& id, int32_t minCoveringZoom, std::forward_list<TileID>& retain);
     int32_t coveringZoomLevel(const TransformState&) const;
diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp
index aa66ceb80b..e9084bdff2 100644
--- a/src/mbgl/map/tile_data.cpp
+++ b/src/mbgl/map/tile_data.cpp
@@ -73,10 +73,11 @@ void TileData::endParsing() {
     parsing.clear(std::memory_order_release);
 }
 
-void TileData::reparse(Worker& worker, std::function<void()> callback) {
+bool TileData::reparse(Worker& worker, std::function<void()> callback) {
     if (!mayStartParsing()) {
-        return;
+        return false;
     }
 
     workRequest = worker.send([this] { parse(); endParsing(); }, callback);
+    return true;
 }
diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp
index 7f16e7c413..3945a32147 100644
--- a/src/mbgl/map/tile_data.hpp
+++ b/src/mbgl/map/tile_data.hpp
@@ -45,7 +45,13 @@ public:
     ~TileData();
 
     void request(Worker&, float pixelRatio, std::function<void ()> callback);
-    void reparse(Worker&, std::function<void ()> callback);
+
+    // Schedule a tile reparse on a worker thread and call the callback on
+    // completion. It will return true if the work was schedule or false it was
+    // not, which can occur if the tile is already being parsed by another
+    // worker (see "mayStartParsing()").
+    bool reparse(Worker&, std::function<void ()> callback);
+
     void cancel();
     const std::string toString() const;
 
diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp
index 263ad52b9d..801d9a7c57 100644
--- a/src/mbgl/map/vector_tile_data.cpp
+++ b/src/mbgl/map/vector_tile_data.cpp
@@ -82,6 +82,12 @@ Bucket* VectorTileData::getBucket(StyleLayer const& layer) {
     return it->second.get();
 }
 
+size_t VectorTileData::countBuckets() const {
+    std::lock_guard<std::mutex> lock(bucketsMutex);
+
+    return buckets.size();
+}
+
 void VectorTileData::setBucket(StyleLayer const& layer, std::unique_ptr<Bucket> bucket) {
     assert(layer.bucket);
 
diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp
index 5fe351ca0c..6549c106e4 100644
--- a/src/mbgl/map/vector_tile_data.hpp
+++ b/src/mbgl/map/vector_tile_data.hpp
@@ -44,6 +44,7 @@ public:
     void parse() override;
     virtual Bucket* getBucket(StyleLayer const &layer_desc) override;
 
+    size_t countBuckets() const;
     void setBucket(StyleLayer const &layer_desc, std::unique_ptr<Bucket> bucket);
 
     void setState(const State& state) override;