From e4a31e3554c34af88df72722313a761d1b9cd46b Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 17 Oct 2019 15:13:30 +0300 Subject: [PATCH 1/2] [core] Fix image requests for already obtained images Before this change, repeated request for an already obtained image was erroneously treated as pending. --- src/mbgl/renderer/image_manager.cpp | 9 ++++++--- src/mbgl/renderer/image_manager.hpp | 3 +-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/mbgl/renderer/image_manager.cpp b/src/mbgl/renderer/image_manager.cpp index 0920f5a6594..7b51fb9a2ef 100644 --- a/src/mbgl/renderer/image_manager.cpp +++ b/src/mbgl/renderer/image_manager.cpp @@ -189,9 +189,12 @@ void ImageManager::checkMissingAndNotify(ImageRequestor& requestor, const ImageR auto existingRequestorsIt = requestedImages.find(missingImage); if (existingRequestorsIt != requestedImages.end()) { // Already asked client about this image. - if (!existingRequestorsIt->second.empty()) { // Still waiting for the client response. - existingRequestorsIt->second.emplace(requestorPtr); - requestor.addPendingRequest(missingImage); + std::set& existingRequestors = existingRequestorsIt->second; + if (!existingRequestors.empty() && + (*existingRequestors.begin()) + ->hasPendingRequest(missingImage)) { // Still waiting for the client response for this image. + requestorPtr->addPendingRequest(missingImage); + existingRequestors.emplace(requestorPtr); continue; } // Unlike icons, pattern changes are not caught diff --git a/src/mbgl/renderer/image_manager.hpp b/src/mbgl/renderer/image_manager.hpp index 98b42da8386..5ed6e237f03 100644 --- a/src/mbgl/renderer/image_manager.hpp +++ b/src/mbgl/renderer/image_manager.hpp @@ -73,9 +73,8 @@ class ImageRequestor { virtual void onImagesAvailable(ImageMap icons, ImageMap patterns, ImageVersionMap versionMap, uint64_t imageCorrelationID) = 0; void addPendingRequest(const std::string& imageId) { pendingRequests.insert(imageId); } - + bool hasPendingRequest(const std::string& imageId) const { return pendingRequests.count(imageId); } bool hasPendingRequests() const { return !pendingRequests.empty(); } - void removePendingRequest(const std::string& imageId) { pendingRequests.erase(imageId); } private: From a89294b3a593f1c9e4f903e6ebb388a4de7908ce Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 17 Oct 2019 15:42:30 +0300 Subject: [PATCH 2/2] [core] Update ImageManager.OnStyleImageMissingBeforeSpriteLoaded So that it checks pending image requests for a different requestor. --- test/renderer/image_manager.test.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/renderer/image_manager.test.cpp b/test/renderer/image_manager.test.cpp index f1061ce59ef..20c0a3a7f32 100644 --- a/test/renderer/image_manager.test.cpp +++ b/test/renderer/image_manager.test.cpp @@ -189,7 +189,7 @@ TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) { // Repeated request of the same image shall not result another // `ImageManagerObserver.onStyleImageMissing()` call. - imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID)); + imageManager.getImages(requestor, std::make_pair(dependencies, ++imageCorrelationID)); runLoop.runOnce(); EXPECT_EQ(observer.count, 1); @@ -197,10 +197,20 @@ TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) { // Request for updated dependencies must be dispatched to the // observer. dependencies.emplace("post", ImageType::Icon); - imageManager.getImages(requestor, std::make_pair(dependencies, imageCorrelationID)); + imageManager.getImages(requestor, std::make_pair(dependencies, ++imageCorrelationID)); runLoop.runOnce(); EXPECT_EQ(observer.count, 2); + + // Another requestor shall not have pending requests for already obtained images. + StubImageRequestor anotherRequestor(imageManager); + imageManager.getImages(anotherRequestor, std::make_pair(dependencies, ++imageCorrelationID)); + ASSERT_FALSE(anotherRequestor.hasPendingRequests()); + + dependencies.emplace("unfamiliar", ImageType::Icon); + imageManager.getImages(anotherRequestor, std::make_pair(dependencies, ++imageCorrelationID)); + EXPECT_TRUE(anotherRequestor.hasPendingRequests()); + EXPECT_TRUE(anotherRequestor.hasPendingRequest("unfamiliar")); } TEST(ImageManager, OnStyleImageMissingAfterSpriteLoaded) {