Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Schedule invocation of onStyleImageMissing completion callback on the same thread #14620

Merged
merged 1 commit into from
May 9, 2019
Merged

[core] Schedule invocation of onStyleImageMissing completion callback on the same thread #14620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions src/mbgl/renderer/image_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#include <mbgl/renderer/image_manager.hpp>
#include <mbgl/actor/actor.hpp>
#include <mbgl/actor/scheduler.hpp>
#include <mbgl/util/logging.hpp>
#include <mbgl/gfx/context.hpp>
#include <mbgl/renderer/image_manager_observer.hpp>
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<ActorCallback>(
*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());
});

}
}

Expand Down
7 changes: 6 additions & 1 deletion src/mbgl/renderer/image_manager.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

namespace mbgl {

template <class T>
class Actor;

namespace gfx {
class Context;
} // namespace gfx
Expand Down Expand Up @@ -64,9 +67,11 @@ class ImageManager : public util::noncopyable {
bool loaded = false;

std::map<ImageRequestor*, ImageRequestPair> requestors;
using Callback = std::function<void()>;
using ActorCallback = Actor<Callback>;
struct MissingImageRequestPair {
ImageRequestPair pair;
unsigned int callbacksRemaining;
std::map<std::string, std::unique_ptr<ActorCallback>> callbacks;
};
std::map<ImageRequestor*, MissingImageRequestPair> missingImageRequestors;
std::map<std::string, std::set<ImageRequestor*>> requestedImages;
Expand Down
22 changes: 22 additions & 0 deletions test/renderer/image_manager.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ class StubImageRequestor : public ImageRequestor {
};

TEST(ImageManager, NotifiesRequestorWhenSpriteIsLoaded) {
util::RunLoop runLoop;
ImageManager imageManager;
StubImageRequestor requestor(imageManager);
bool notified = false;
Expand All @@ -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);
}

Expand Down Expand Up @@ -173,6 +178,7 @@ class StubImageManagerObserver : public ImageManagerObserver {
};

TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) {
util::RunLoop runLoop;
ImageManager imageManager;
StubImageRequestor requestor(imageManager);
StubImageManagerObserver observer;
Expand All @@ -189,23 +195,27 @@ 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);

}

TEST(ImageManager, OnStyleImageMissingAfterSpriteLoaded) {
util::RunLoop runLoop;
ImageManager imageManager;
StubImageRequestor requestor(imageManager);
StubImageManagerObserver observer;
Expand All @@ -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;

Expand All @@ -247,17 +261,20 @@ TEST(ImageManager, ReduceMemoryUsage) {

imageManager.setObserver(&observer);
imageManager.setLoaded(true);
runLoop.runOnce();

// Single requestor
{
std::unique_ptr<StubImageRequestor> requestor = std::make_unique<StubImageRequestor>(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
Expand All @@ -266,12 +283,14 @@ TEST(ImageManager, ReduceMemoryUsage) {
std::unique_ptr<StubImageRequestor> requestor2 = std::make_unique<StubImageRequestor>(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.
Expand All @@ -280,16 +299,19 @@ TEST(ImageManager, ReduceMemoryUsage) {
std::unique_ptr<StubImageRequestor> requestor1 = std::make_unique<StubImageRequestor>(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);
}