diff --git a/src/mbgl/renderer/image_manager.cpp b/src/mbgl/renderer/image_manager.cpp index 8e584ffd34c..860de487e70 100644 --- a/src/mbgl/renderer/image_manager.cpp +++ b/src/mbgl/renderer/image_manager.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include #include #include @@ -128,7 +130,7 @@ void ImageManager::removeRequestor(ImageRequestor& requestor) { void ImageManager::notifyIfMissingImageAdded() { for (auto it = missingImageRequestors.begin(); it != missingImageRequestors.end();) { - if (it->second.callbacksRemaining == 0) { + if (it->second.callbacks.empty()) { notify(*it->first, it->second.pair); it = missingImageRequestors.erase(it); } else { @@ -162,19 +164,29 @@ void ImageManager::checkMissingAndNotify(ImageRequestor& requestor, const ImageR if (missing > 0) { ImageRequestor* requestorPtr = &requestor; - missingImageRequestors.emplace(requestorPtr, MissingImageRequestPair { std::move(pair), missing }); + auto emplaced = missingImageRequestors.emplace(requestorPtr, MissingImageRequestPair { pair, {} }); + assert(emplaced.second); for (const auto& dependency : pair.first) { auto it = images.find(dependency.first); if (it == images.end()) { assert(observer != nullptr); - observer->onStyleImageMissing(dependency.first, [this, requestorPtr]() { - auto requestorIt = missingImageRequestors.find(requestorPtr); - if (requestorIt != missingImageRequestors.end()) { - assert(requestorIt->second.callbacksRemaining > 0); - requestorIt->second.callbacksRemaining--; - } + auto callback = std::make_unique( + *Scheduler::GetCurrent(), + [this, requestorPtr, imageId = dependency.first] { + auto requestorIt = missingImageRequestors.find(requestorPtr); + if (requestorIt != missingImageRequestors.end()) { + assert(requestorIt->second.callbacks.find(imageId) != requestorIt->second.callbacks.end()); + requestorIt->second.callbacks.erase(imageId); + } + }); + + auto actorRef = callback->self(); + emplaced.first->second.callbacks.emplace(dependency.first, std::move(callback)); + observer->onStyleImageMissing(dependency.first, [actorRef]() mutable { + actorRef.invoke(&Callback::operator()); }); + } } diff --git a/src/mbgl/renderer/image_manager.hpp b/src/mbgl/renderer/image_manager.hpp index 61f3f3c2763..c2d6466b3ad 100644 --- a/src/mbgl/renderer/image_manager.hpp +++ b/src/mbgl/renderer/image_manager.hpp @@ -15,6 +15,9 @@ namespace mbgl { +template +class Actor; + namespace gfx { class Context; } // namespace gfx @@ -64,9 +67,11 @@ class ImageManager : public util::noncopyable { bool loaded = false; std::map requestors; + using Callback = std::function; + using ActorCallback = Actor; struct MissingImageRequestPair { ImageRequestPair pair; - unsigned int callbacksRemaining; + std::map> callbacks; }; std::map missingImageRequestors; std::map> requestedImages; diff --git a/test/renderer/image_manager.test.cpp b/test/renderer/image_manager.test.cpp index a5fffce4796..7f0d6708469 100644 --- a/test/renderer/image_manager.test.cpp +++ b/test/renderer/image_manager.test.cpp @@ -119,6 +119,7 @@ class StubImageRequestor : public ImageRequestor { }; TEST(ImageManager, NotifiesRequestorWhenSpriteIsLoaded) { + util::RunLoop runLoop; ImageManager imageManager; StubImageRequestor requestor(imageManager); bool notified = false; @@ -134,11 +135,15 @@ TEST(ImageManager, NotifiesRequestorWhenSpriteIsLoaded) { ImageDependencies dependencies; dependencies.emplace("one", ImageType::Icon); imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID)); + runLoop.runOnce(); + ASSERT_FALSE(notified); imageManager.setLoaded(true); + runLoop.runOnce(); ASSERT_FALSE(notified); imageManager.notifyIfMissingImageAdded(); + runLoop.runOnce(); ASSERT_TRUE(notified); } @@ -173,6 +178,7 @@ class StubImageManagerObserver : public ImageManagerObserver { }; TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) { + util::RunLoop runLoop; ImageManager imageManager; StubImageRequestor requestor(imageManager); StubImageManagerObserver observer; @@ -189,16 +195,19 @@ TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) { ImageDependencies dependencies; dependencies.emplace("pre", ImageType::Icon); imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID)); + runLoop.runOnce(); EXPECT_EQ(observer.count, 0); ASSERT_FALSE(notified); imageManager.setLoaded(true); + runLoop.runOnce(); EXPECT_EQ(observer.count, 1); ASSERT_FALSE(notified); imageManager.notifyIfMissingImageAdded(); + runLoop.runOnce(); EXPECT_EQ(observer.count, 1); ASSERT_TRUE(notified); @@ -206,6 +215,7 @@ TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) { } TEST(ImageManager, OnStyleImageMissingAfterSpriteLoaded) { + util::RunLoop runLoop; ImageManager imageManager; StubImageRequestor requestor(imageManager); StubImageManagerObserver observer; @@ -222,22 +232,26 @@ TEST(ImageManager, OnStyleImageMissingAfterSpriteLoaded) { ASSERT_FALSE(notified); imageManager.setLoaded(true); + runLoop.runOnce(); uint64_t imageCorrelationID = 0; ImageDependencies dependencies; dependencies.emplace("after", ImageType::Icon); imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID)); + runLoop.runOnce(); EXPECT_EQ(observer.count, 1); ASSERT_FALSE(notified); imageManager.notifyIfMissingImageAdded(); + runLoop.runOnce(); EXPECT_EQ(observer.count, 1); ASSERT_TRUE(notified); } TEST(ImageManager, ReduceMemoryUsage) { + util::RunLoop runLoop; ImageManager imageManager; StubImageManagerObserver observer; @@ -247,17 +261,20 @@ TEST(ImageManager, ReduceMemoryUsage) { imageManager.setObserver(&observer); imageManager.setLoaded(true); + runLoop.runOnce(); // Single requestor { std::unique_ptr requestor = std::make_unique(imageManager); imageManager.getImages(*requestor, std::make_pair(ImageDependencies{{"missing", ImageType::Icon}}, 0ull)); + runLoop.runOnce(); EXPECT_EQ(observer.count, 1); ASSERT_FALSE(imageManager.getImage("missing") == nullptr); } // Reduce memory usage and check that unused image was deleted. imageManager.reduceMemoryUse(); + runLoop.runOnce(); ASSERT_TRUE(imageManager.getImage("missing") == nullptr); // Multiple requestors @@ -266,12 +283,14 @@ TEST(ImageManager, ReduceMemoryUsage) { std::unique_ptr requestor2 = std::make_unique(imageManager); imageManager.getImages(*requestor1, std::make_pair(ImageDependencies{{"missing", ImageType::Icon}}, 0ull)); imageManager.getImages(*requestor2, std::make_pair(ImageDependencies{{"missing", ImageType::Icon}}, 1ull)); + runLoop.runOnce(); EXPECT_EQ(observer.count, 2); ASSERT_FALSE(imageManager.getImage("missing") == nullptr); } // Reduce memory usage and check that unused image was deleted when all requestors are destructed. imageManager.reduceMemoryUse(); + runLoop.runOnce(); ASSERT_TRUE(imageManager.getImage("missing") == nullptr); // Multiple requestors, check that image resource is not destroyed if there is at least 1 requestor that uses it. @@ -280,16 +299,19 @@ TEST(ImageManager, ReduceMemoryUsage) { std::unique_ptr requestor1 = std::make_unique(imageManager); imageManager.getImages(*requestor, std::make_pair(ImageDependencies{{"missing", ImageType::Icon}}, 0ull)); imageManager.getImages(*requestor1, std::make_pair(ImageDependencies{{"missing", ImageType::Icon}}, 1ull)); + runLoop.runOnce(); EXPECT_EQ(observer.count, 3); ASSERT_FALSE(imageManager.getImage("missing") == nullptr); } // Reduce memory usage and check that requested image is not destructed. imageManager.reduceMemoryUse(); + runLoop.runOnce(); ASSERT_FALSE(imageManager.getImage("missing") == nullptr); // Release last requestor and check if resource was released after reduceMemoryUse(). requestor.reset(); imageManager.reduceMemoryUse(); + runLoop.runOnce(); ASSERT_TRUE(imageManager.getImage("missing") == nullptr); }