fix firing onStyleImageMissing after sprite has loaded (#14369)

mapbox · Apr 8, 2019 · 104091c · 104091c
1 parent 37a8865
commit 104091c
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 6 deletions.
diff --git a/src/mbgl/renderer/image_manager.cpp b/src/mbgl/renderer/image_manager.cpp
@@ -90,19 +90,20 @@ void ImageManager::getImages(ImageRequestor& requestor, ImageRequestPair&& pair)
     // if all icons are available. If any are missing, call `onStyleImageMissing`
     // to give the user a chance to provide the icon. If they are not provided
     // by the next frame we'll assume they are permanently missing.
-    bool hasAllDependencies = true;
     if (!isLoaded()) {
+        bool hasAllDependencies = true;
         for (const auto& dependency : pair.first) {
             if (images.find(dependency.first) == images.end()) {
                 hasAllDependencies = false;
+                break;
             }
         }
-    }
 
-    if (hasAllDependencies) {
-        notify(requestor, pair);
-    } else if (!isLoaded()) {
-        requestors.emplace(&requestor, std::move(pair));
+        if (hasAllDependencies) {
+            notify(requestor, pair);
+        } else {
+            requestors.emplace(&requestor, std::move(pair));
+        }
     } else {
         checkMissingAndNotify(requestor, std::move(pair));
     }

diff --git a/test/renderer/image_manager.test.cpp b/test/renderer/image_manager.test.cpp
@@ -157,3 +157,78 @@ TEST(ImageManager, NotifiesRequestorImmediatelyIfDependenciesAreSatisfied) {
 
     ASSERT_TRUE(notified);
 }
+
+
+class StubImageManagerObserver : public ImageManagerObserver {
+    public:
+    int count = 0;
+    virtual void onStyleImageMissing(const std::string&, std::function<void()> done) override {
+        count++;
+        done();
+    }
+};
+
+TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) {
+    ImageManager imageManager;
+    StubImageRequestor requestor;
+    StubImageManagerObserver observer;
+
+    imageManager.setObserver(&observer);
+
+    bool notified = false;
+
+    requestor.imagesAvailable = [&] (ImageMap, ImageMap, std::unordered_map<std::string, uint32_t>) {
+        notified = true;
+    };
+
+    uint64_t imageCorrelationID = 0;
+    ImageDependencies dependencies;
+    dependencies.emplace("pre", ImageType::Icon);
+    imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID));
+
+    EXPECT_EQ(observer.count, 0);
+    ASSERT_FALSE(notified);
+
+    imageManager.setLoaded(true);
+
+    EXPECT_EQ(observer.count, 1);
+    ASSERT_FALSE(notified);
+
+    imageManager.notifyIfMissingImageAdded();
+
+    EXPECT_EQ(observer.count, 1);
+    ASSERT_TRUE(notified);
+
+}
+
+TEST(ImageManager, OnStyleImageMissingAfterSpriteLoaded) {
+    ImageManager imageManager;
+    StubImageRequestor requestor;
+    StubImageManagerObserver observer;
+
+    imageManager.setObserver(&observer);
+
+    bool notified = false;
+
+    requestor.imagesAvailable = [&] (ImageMap, ImageMap, std::unordered_map<std::string, uint32_t>) {
+        notified = true;
+    };
+
+    EXPECT_EQ(observer.count, 0);
+    ASSERT_FALSE(notified);
+
+    imageManager.setLoaded(true);
+
+    uint64_t imageCorrelationID = 0;
+    ImageDependencies dependencies;
+    dependencies.emplace("after", ImageType::Icon);
+    imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID));
+
+    EXPECT_EQ(observer.count, 1);
+    ASSERT_FALSE(notified);
+
+    imageManager.notifyIfMissingImageAdded();
+
+    EXPECT_EQ(observer.count, 1);
+    ASSERT_TRUE(notified);
+}