[core] trigger repaint after placement is redone

fixes #3140

This also fixes a bug where placement could be redone with a stale
PlacementConfig after the tile first finishes loading.

8 files changed, 39 insertions, 42 deletions

diff --git a/package.json b/package.json
index ee514f3d05..901519dd4c 100644
--- a/package.json
+++ b/package.json
@@ -26,7 +26,7 @@
   ],
   "devDependencies": {
     "aws-sdk": "^2.2.21",
-    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#2f4d8ed044a3c962a43d62de0608b752eb68052c",
+    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#1b9035a02f8f23bf72a197c2a8d8f910d935346f",
     "node-gyp": "^3.2.1",
     "request": "^2.67.0",
     "tape": "^4.2.2"
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index 4253a60650..15099d6425 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -216,7 +216,7 @@ TileData::State Source::hasTile(const TileID& tileID) {
     return TileData::State::invalid;
 }
 
-bool Source::handlePartialTile(const TileID& tileID, Worker&) {
+bool Source::handlePartialTile(const TileID& tileID) {
     auto it = tileDataMap.find(tileID.normalized());
     if (it == tileDataMap.end()) {
         return true;
@@ -227,14 +227,10 @@ bool Source::handlePartialTile(const TileID& tileID, Worker&) {
         return true;
     }
 
-    return tileData->parsePending([this, tileID](std::exception_ptr error) {
-        if (error) {
-            observer->onTileError(*this, tileID, error);
-            return;
-        }
+    auto callback = std::bind(&Source::tileLoadingCallback, this, tileID,
+            std::placeholders::_1, false);
 
-        observer->onTileLoaded(*this, tileID, false);
-    });
+    return tileData->parsePending(callback);
 }
 
 TileData::State Source::addTile(const TileID& tileID, const StyleUpdateParameters& parameters) {
@@ -266,9 +262,8 @@ TileData::State Source::addTile(const TileID& tileID, const StyleUpdateParameter
     }
 
     if (!newTile->data) {
-        auto callback = std::bind(&Source::tileLoadingCompleteCallback, this, normalizedID,
-                                  std::placeholders::_1, parameters.transformState,
-                                  parameters.debugOptions & MapDebugOptions::Collision);
+        auto callback = std::bind(&Source::tileLoadingCallback, this, normalizedID,
+                                  std::placeholders::_1, true);
 
         // If we don't find working tile data, we're just going to load it.
         if (type == SourceType::Raster) {
@@ -425,7 +420,7 @@ bool Source::update(const StyleUpdateParameters& parameters) {
         switch (state) {
         case TileData::State::partial:
             if (parameters.shouldReparsePartialTiles) {
-                if (!handlePartialTile(tileID, parameters.worker)) {
+                if (!handlePartialTile(tileID)) {
                     allTilesUpdated = false;
                 }
             }
@@ -502,7 +497,10 @@ bool Source::update(const StyleUpdateParameters& parameters) {
 
     for (auto& tilePtr : tilePtrs) {
         tilePtr->data->redoPlacement(
-            { parameters.transformState.getAngle(), parameters.transformState.getPitch(), parameters.debugOptions & MapDebugOptions::Collision });
+            { parameters.transformState.getAngle(), parameters.transformState.getPitch(), parameters.debugOptions & MapDebugOptions::Collision },
+            [this]() {
+                observer->onPlacementRedone();
+            });
     }
 
     updated = parameters.animationTime;
@@ -529,10 +527,9 @@ void Source::setObserver(Observer* observer_) {
     observer = observer_;
 }
 
-void Source::tileLoadingCompleteCallback(const TileID& tileID,
+void Source::tileLoadingCallback(const TileID& tileID,
                                          std::exception_ptr error,
-                                         const TransformState& transformState,
-                                         bool collisionDebug) {
+                                         bool isNewTile) {
     auto it = tileDataMap.find(tileID);
     if (it == tileDataMap.end()) {
         return;
@@ -548,8 +545,10 @@ void Source::tileLoadingCompleteCallback(const TileID& tileID,
         return;
     }
 
-    tileData->redoPlacement({ transformState.getAngle(), transformState.getPitch(), collisionDebug });
-    observer->onTileLoaded(*this, tileID, true);
+    tileData->redoPlacement([this]() {
+        observer->onPlacementRedone();
+    });
+    observer->onTileLoaded(*this, tileID, isNewTile);
 }
 
 void Source::dumpDebugLogs() const {
diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp
index 9e9b6b66ab..d9146c1fdb 100644
--- a/src/mbgl/map/source.hpp
+++ b/src/mbgl/map/source.hpp
@@ -37,6 +37,7 @@ public:
 
         virtual void onTileLoaded(Source&, const TileID&, bool /* isNewTile */) {};
         virtual void onTileError(Source&, const TileID&, std::exception_ptr) {};
+        virtual void onPlacementRedone() {};
     };
 
     Source(SourceType,
@@ -78,11 +79,10 @@ public:
     bool enabled = false;
 
 private:
-    void tileLoadingCompleteCallback(const TileID&,
-                                     std::exception_ptr,
-                                     const TransformState&,
-                                     bool collisionDebug);
-    bool handlePartialTile(const TileID&, Worker& worker);
+    void tileLoadingCallback(const TileID&,
+                             std::exception_ptr,
+                             bool isNewTile);
+    bool handlePartialTile(const TileID&);
     bool findLoadedChildren(const TileID&, int32_t maxCoveringZoom, std::forward_list<TileID>& retain);
     void findLoadedParent(const TileID&, int32_t minCoveringZoom, std::forward_list<TileID>& retain);
     int32_t coveringZoomLevel(const TransformState&) const;
diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp
index fec8ba4901..90196f8a42 100644
--- a/src/mbgl/map/tile_data.hpp
+++ b/src/mbgl/map/tile_data.hpp
@@ -79,7 +79,8 @@ public:
     virtual Bucket* getBucket(const StyleLayer&) = 0;
 
     virtual bool parsePending(std::function<void (std::exception_ptr)>) { return true; }
-    virtual void redoPlacement(PlacementConfig) {}
+    virtual void redoPlacement(PlacementConfig, const std::function<void()>&) {}
+    virtual void redoPlacement(const std::function<void()>&) {}
 
     bool isReady() const {
         return isReadyState(state);
diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp
index fa08432b20..df7923273c 100644
--- a/src/mbgl/map/vector_tile_data.cpp
+++ b/src/mbgl/map/vector_tile_data.cpp
@@ -77,11 +77,6 @@ VectorTileData::VectorTileData(const TileID& id_,
                 // existing buckets in case we got a refresh parse.
                 buckets = std::move(resultBuckets.buckets);
 
-                // The target configuration could have changed since we started placement. In this case,
-                // we're starting another placement run.
-                if (placedConfig != targetConfig) {
-                    redoPlacement();
-                }
             } else {
                 error = result.get<std::exception_ptr>();
                 state = State::obsolete;
@@ -124,11 +119,6 @@ bool VectorTileData::parsePending(std::function<void(std::exception_ptr)> callba
             // place again in case the configuration has changed.
             placedConfig = config;
 
-            // The target configuration could have changed since we started placement. In this case,
-            // we're starting another placement run.
-            if (placedConfig != targetConfig) {
-                redoPlacement();
-            }
         } else {
             error = result.get<std::exception_ptr>();
             state = State::obsolete;
@@ -150,21 +140,21 @@ Bucket* VectorTileData::getBucket(const StyleLayer& layer) {
     return it->second.get();
 }
 
-void VectorTileData::redoPlacement(const PlacementConfig newConfig) {
+void VectorTileData::redoPlacement(const PlacementConfig newConfig, const std::function<void()>& callback) {
     if (newConfig != placedConfig) {
         targetConfig = newConfig;
 
         if (!workRequest) {
             // Don't start a new placement request when the current one hasn't completed yet, or when
             // we are parsing buckets.
-            redoPlacement();
+            redoPlacement(callback);
         }
     }
 }
 
-void VectorTileData::redoPlacement() {
+void VectorTileData::redoPlacement(const std::function<void()>& callback) {
     workRequest.reset();
-    workRequest = worker.redoPlacement(tileWorker, buckets, targetConfig, [this, config = targetConfig] {
+    workRequest = worker.redoPlacement(tileWorker, buckets, targetConfig, [this, callback, config = targetConfig] {
         workRequest.reset();
 
         // Persist the configuration we just placed so that we can later check whether we need to
@@ -178,7 +168,9 @@ void VectorTileData::redoPlacement() {
         // The target configuration could have changed since we started placement. In this case,
         // we're starting another placement run.
         if (placedConfig != targetConfig) {
-            redoPlacement();
+            redoPlacement(callback);
+        } else {
+            callback();
         }
     });
 }
diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp
index 3ba572f42a..b61b2a25a0 100644
--- a/src/mbgl/map/vector_tile_data.hpp
+++ b/src/mbgl/map/vector_tile_data.hpp
@@ -31,8 +31,8 @@ public:
 
     bool parsePending(std::function<void(std::exception_ptr)> callback) override;
 
-    void redoPlacement(PlacementConfig config) override;
-    void redoPlacement();
+    void redoPlacement(PlacementConfig config, const std::function<void()>&) override;
+    void redoPlacement(const std::function<void()>&) override;
 
     void cancel() override;
 
diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp
index 58a3ae009c..ca18cd9247 100644
--- a/src/mbgl/style/style.cpp
+++ b/src/mbgl/style/style.cpp
@@ -356,6 +356,10 @@ void Style::onTileError(Source& source, const TileID& tileID, std::exception_ptr
     observer->onResourceError(error);
 }
 
+void Style::onPlacementRedone() {
+    observer->onResourceLoaded();
+}
+
 void Style::onSpriteLoaded() {
     shouldReparsePartialTiles = true;
     observer->onSpriteLoaded();
diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp
index 29510a3887..7bd98a7552 100644
--- a/src/mbgl/style/style.hpp
+++ b/src/mbgl/style/style.hpp
@@ -132,6 +132,7 @@ private:
     void onSourceError(Source&, std::exception_ptr) override;
     void onTileLoaded(Source&, const TileID&, bool isNewTile) override;
     void onTileError(Source&, const TileID&, std::exception_ptr) override;
+    void onPlacementRedone() override;
 
     bool shouldReparsePartialTiles = false;