tei/MapLibreParticleLab
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
Before this change, ImageManger's 'done' callback for onStyleImageMissing observer
notification that was created on renderer thread, could be called from different thread,
therefore, is not thread safe. For example, on Android platform, callback was invoked
from UI thread. This change makes callback to be scheduled on originating thread.
This commit is contained in:
Alexander Shalamov
2019-05-09 10:49:10 +03:00
parent 6fa1d357b3
commit a53c4593d6
3 changed files with 48 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
src/mbgl/renderer/image_manager.cpp
View File

@@ -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>
@@ -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<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());
});
}
}

7
src/mbgl/renderer/image_manager.hpp
View File

@@ -15,6 +15,9 @@
namespace mbgl {
template <class T>
class Actor;
namespace gfx {
class Context;
} // namespace gfx
@@ -64,9 +67,11 @@ private:
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;

22
test/renderer/image_manager.test.cpp
View File

@@ -119,6 +119,7 @@ public:
};
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<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
@@ -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.
@@ -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);
}