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

Cherry-pick to release-mojito #14635

Merged
merged 3 commits into from
May 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,6 @@ private void onSourceChanged(String sourceId) {

@Keep
private void onStyleImageMissing(String imageId) {
Logger.e(TAG, "OnStyleImageMissing: " + imageId);
if (stateCallback != null) {
stateCallback.onStyleImageMissing(imageId);
}
Expand Down
28 changes: 20 additions & 8 deletions src/mbgl/renderer/image_manager.cpp
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 @@ -116,7 +118,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);
missingImageRequestors.erase(it++);
} else {
Expand All @@ -137,19 +139,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
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 @@ -66,9 +69,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;
ImageMap images;
Expand Down
3 changes: 3 additions & 0 deletions src/mbgl/style/style_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,9 @@ void Style::Impl::onSpriteError(std::exception_ptr error) {
lastError = error;
Log::Error(Event::Style, "Failed to load sprite: %s", util::toString(error).c_str());
observer->onResourceError(error);
// Unblock rendering tiles (even though sprite request has failed).
spriteLoaded = true;
observer->onUpdate();
}

void Style::Impl::onLayerChanged(Layer& layer) {
Expand Down
13 changes: 13 additions & 0 deletions test/renderer/image_manager.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ class StubImageRequestor : public ImageRequestor {
};

TEST(ImageManager, NotifiesRequestorWhenSpriteIsLoaded) {
util::RunLoop runLoop;
ImageManager imageManager;
StubImageRequestor requestor;
bool notified = false;
Expand All @@ -132,11 +133,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 @@ -169,6 +174,7 @@ class StubImageManagerObserver : public ImageManagerObserver {
};

TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) {
util::RunLoop runLoop;
ImageManager imageManager;
StubImageRequestor requestor;
StubImageManagerObserver observer;
Expand All @@ -185,23 +191,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;
StubImageManagerObserver observer;
Expand All @@ -218,16 +228,19 @@ 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);
Expand Down