[core] Loading images to style optimization

This change enables attaching images to the style with batches and
avoids massive re-allocations. Thus, it improves UI performance
especially at start-up time.
This commit is contained in:
Mikhail Pozdnyakov
2020-02-11 01:54:05 +02:00
parent 146e0ac8ed
commit 64e2bcdf43
15 changed files with 121 additions and 89 deletions

View File

@@ -1,8 +1,10 @@
#pragma once
#include <mbgl/style/transition_options.hpp>
#include <mbgl/map/camera.hpp>
#include <mbgl/style/transition_options.hpp>
#include <mbgl/util/geo.hpp>
#include <mbgl/util/image.hpp>
#include <mbgl/util/immutable.hpp>
#include <string>
#include <vector>
@@ -45,7 +47,7 @@ public:
void setLight(std::unique_ptr<Light>);
// Images
const Image* getImage(const std::string&) const;
const PremultipliedImage* getImage(const std::string&) const;
void addImage(std::unique_ptr<Image>);
void removeImage(const std::string&);

View File

@@ -1099,12 +1099,10 @@ void NativeMapView::removeImage(JNIEnv& env, const jni::String& name) {
}
jni::Local<jni::Object<Bitmap>> NativeMapView::getImage(JNIEnv& env, const jni::String& name) {
const mbgl::style::Image *image = map->getStyle().getImage(jni::Make<std::string>(env, name));
if (image) {
return Bitmap::CreateBitmap(env, image->getImage());
} else {
return jni::Local<jni::Object<Bitmap>>();
if (auto* image = map->getStyle().getImage(jni::Make<std::string>(env, name))) {
return Bitmap::CreateBitmap(env, *image);
}
return jni::Local<jni::Object<Bitmap>>();
}
void NativeMapView::setPrefetchTiles(JNIEnv&, jni::jboolean enable) {

View File

@@ -170,7 +170,6 @@ void Map::Impl::jumpTo(const CameraOptions& camera) {
}
void Map::Impl::onStyleImageMissing(const std::string& id, std::function<void()> done) {
if (style->getImage(id) == nullptr) {
observer.onStyleImageMissing(id);
}

View File

@@ -90,7 +90,7 @@ void SpriteLoader::emitSpriteLoadedIfComplete() {
loader->worker.self().invoke(&SpriteLoaderWorker::parse, loader->image, loader->json);
}
void SpriteLoader::onParsed(std::vector<std::unique_ptr<style::Image>>&& result) {
void SpriteLoader::onParsed(std::vector<Immutable<style::Image::Impl>> result) {
observer->onSpriteLoaded(std::move(result));
}

View File

@@ -1,6 +1,5 @@
#pragma once
#include <mbgl/util/noncopyable.hpp>
#include <mbgl/style/image.hpp>
#include <string>
@@ -15,7 +14,7 @@ namespace mbgl {
class FileSource;
class SpriteLoaderObserver;
class SpriteLoader : public util::noncopyable {
class SpriteLoader {
public:
SpriteLoader(float pixelRatio);
~SpriteLoader();
@@ -29,7 +28,7 @@ private:
// Invoked by SpriteAtlasWorker
friend class SpriteLoaderWorker;
void onParsed(std::vector<std::unique_ptr<style::Image>>&&);
void onParsed(std::vector<Immutable<style::Image::Impl>>);
void onError(std::exception_ptr);
const float pixelRatio;

View File

@@ -1,7 +1,9 @@
#pragma once
#include <mbgl/style/image.hpp>
#include <mbgl/util/immutable.hpp>
#include <exception>
#include <memory>
#include <vector>
namespace mbgl {
@@ -13,8 +15,7 @@ class Image;
class SpriteLoaderObserver {
public:
virtual ~SpriteLoaderObserver() = default;
virtual void onSpriteLoaded(std::vector<std::unique_ptr<style::Image>>&&) {}
virtual void onSpriteLoaded(std::vector<Immutable<style::Image::Impl>>) {}
virtual void onSpriteError(std::exception_ptr) {}
};

View File

@@ -149,50 +149,54 @@ optional<style::ImageContent> getContent(const JSValue& value, const char* prope
} // namespace
std::vector<std::unique_ptr<style::Image>> parseSprite(const std::string& encodedImage, const std::string& json) {
std::vector<Immutable<style::Image::Impl>> parseSprite(const std::string& encodedImage, const std::string& json) {
const PremultipliedImage raster = decodeImage(encodedImage);
JSDocument doc;
doc.Parse<0>(json.c_str());
if (doc.HasParseError()) {
throw std::runtime_error("Failed to parse JSON: " + formatJSONParseError(doc));
} else if (!doc.IsObject()) {
}
if (!doc.IsObject()) {
throw std::runtime_error("Sprite JSON root must be an object");
} else {
std::vector<std::unique_ptr<style::Image>> images;
for (const auto& property : doc.GetObject()) {
const std::string name = { property.name.GetString(), property.name.GetStringLength() };
const JSValue& value = property.value;
}
if (value.IsObject()) {
const uint16_t x = getUInt16(value, "x", name.c_str(), 0);
const uint16_t y = getUInt16(value, "y", name.c_str(), 0);
const uint16_t width = getUInt16(value, "width", name.c_str(), 0);
const uint16_t height = getUInt16(value, "height", name.c_str(), 0);
const double pixelRatio = getDouble(value, "pixelRatio", name.c_str(), 1);
const bool sdf = getBoolean(value, "sdf", name.c_str(), false);
style::ImageStretches stretchX = getStretches(value, "stretchX", name.c_str());
style::ImageStretches stretchY = getStretches(value, "stretchY", name.c_str());
optional<style::ImageContent> content = getContent(value, "content", name.c_str());
const auto& properties = doc.GetObject();
std::vector<Immutable<style::Image::Impl>> images;
images.reserve(properties.MemberCount());
for (const auto& property : properties) {
const std::string name = {property.name.GetString(), property.name.GetStringLength()};
const JSValue& value = property.value;
auto image = createStyleImage(name,
raster,
x,
y,
width,
height,
pixelRatio,
sdf,
std::move(stretchX),
std::move(stretchY),
std::move(content));
if (image) {
images.push_back(std::move(image));
}
if (value.IsObject()) {
const uint16_t x = getUInt16(value, "x", name.c_str(), 0);
const uint16_t y = getUInt16(value, "y", name.c_str(), 0);
const uint16_t width = getUInt16(value, "width", name.c_str(), 0);
const uint16_t height = getUInt16(value, "height", name.c_str(), 0);
const double pixelRatio = getDouble(value, "pixelRatio", name.c_str(), 1);
const bool sdf = getBoolean(value, "sdf", name.c_str(), false);
style::ImageStretches stretchX = getStretches(value, "stretchX", name.c_str());
style::ImageStretches stretchY = getStretches(value, "stretchY", name.c_str());
optional<style::ImageContent> content = getContent(value, "content", name.c_str());
auto image = createStyleImage(name,
raster,
x,
y,
width,
height,
pixelRatio,
sdf,
std::move(stretchX),
std::move(stretchY),
std::move(content));
if (image) {
images.push_back(std::move(image->baseImpl));
}
}
return images;
}
return images;
}
} // namespace mbgl

View File

@@ -20,6 +20,6 @@ std::unique_ptr<style::Image> createStyleImage(const std::string& id,
optional<style::ImageContent> content = nullopt);
// Parses an image and an associated JSON file and returns the sprite objects.
std::vector<std::unique_ptr<style::Image>> parseSprite(const std::string& image, const std::string& json);
std::vector<Immutable<style::Image::Impl>> parseSprite(const std::string& image, const std::string& json);
} // namespace mbgl

View File

@@ -1,9 +1,10 @@
#include <mbgl/style/image.hpp>
#include <mbgl/style/image_impl.hpp>
#include <mbgl/style/layer.hpp>
#include <mbgl/style/light.hpp>
#include <mbgl/style/source.hpp>
#include <mbgl/style/style.hpp>
#include <mbgl/style/style_impl.hpp>
#include <mbgl/style/light.hpp>
#include <mbgl/style/image.hpp>
#include <mbgl/style/source.hpp>
#include <mbgl/style/layer.hpp>
namespace mbgl {
namespace style {
@@ -59,8 +60,11 @@ const Light* Style::getLight() const {
return impl->getLight();
}
const Image* Style::getImage(const std::string& name) const {
return impl->getImage(name);
const PremultipliedImage* Style::getImage(const std::string& name) const {
if (auto* image = impl->getImage(name)) {
return &(image->image);
}
return nullptr;
}
void Style::addImage(std::unique_ptr<Image> image) {

View File

@@ -98,7 +98,7 @@ void Style::Impl::parse(const std::string& json_) {
sources.clear();
layers.clear();
images.clear();
images = makeMutable<ImageImpls>();
transitionOptions = parser.transition;
@@ -274,17 +274,37 @@ bool Style::Impl::isLoaded() const {
}
void Style::Impl::addImage(std::unique_ptr<style::Image> image) {
images.remove(image->getID()); // We permit using addImage to update.
images.add(std::move(image));
auto newImages = makeMutable<ImageImpls>(*images);
auto it =
std::lower_bound(newImages->begin(), newImages->end(), image->getID(), [](const auto& a, const std::string& b) {
return a->id < b;
});
if (it != newImages->end() && (*it)->id == image->getID()) {
// We permit using addImage to update.
*it = std::move(image->baseImpl);
} else {
newImages->insert(it, std::move(image->baseImpl));
}
images = std::move(newImages);
observer->onUpdate();
}
void Style::Impl::removeImage(const std::string& id) {
images.remove(id);
auto newImages = makeMutable<ImageImpls>(*images);
auto found =
std::find_if(newImages->begin(), newImages->end(), [&id](const auto& image) { return image->id == id; });
if (found == images->end()) {
Log::Warning(Event::General, "Image '%s' is not present in style, cannot remove", id.c_str());
return;
}
newImages->erase(found);
images = std::move(newImages);
}
const style::Image* Style::Impl::getImage(const std::string& id) const {
return images.get(id);
const style::Image::Impl* Style::Impl::getImage(const std::string& id) const {
auto found = std::find_if(images->begin(), images->end(), [&id](const auto& image) { return image->id == id; });
if (found == images->end()) return nullptr;
return found->get();
}
void Style::Impl::setObserver(style::Observer* observer_) {
@@ -319,10 +339,12 @@ void Style::Impl::onSourceDescriptionChanged(Source& source) {
}
}
void Style::Impl::onSpriteLoaded(std::vector<std::unique_ptr<Image>>&& images_) {
for (auto& image : images_) {
addImage(std::move(image));
}
void Style::Impl::onSpriteLoaded(std::vector<Immutable<style::Image::Impl>> images_) {
auto newImages = makeMutable<ImageImpls>(*images);
newImages->insert(
newImages->end(), std::make_move_iterator(images_.begin()), std::make_move_iterator(images_.end()));
std::sort(newImages->begin(), newImages->end(), [](const auto& a, const auto& b) { return a->id < b->id; });
images = std::move(newImages);
spriteLoaded = true;
observer->onUpdate(); // For *-pattern properties.
}
@@ -357,7 +379,7 @@ const std::string& Style::Impl::getGlyphURL() const {
}
Immutable<std::vector<Immutable<Image::Impl>>> Style::Impl::getImageImpls() const {
return images.getImpls();
return images;
}
Immutable<std::vector<Immutable<Source::Impl>>> Style::Impl::getSourceImpls() const {

View File

@@ -78,13 +78,14 @@ public:
void setLight(std::unique_ptr<Light>);
Light* getLight() const;
const style::Image* getImage(const std::string&) const;
const style::Image::Impl* getImage(const std::string&) const;
void addImage(std::unique_ptr<style::Image>);
void removeImage(const std::string&);
const std::string& getGlyphURL() const;
Immutable<std::vector<Immutable<Image::Impl>>> getImageImpls() const;
using ImageImpls = std::vector<Immutable<Image::Impl>>;
Immutable<ImageImpls> getImageImpls() const;
Immutable<std::vector<Immutable<Source::Impl>>> getSourceImpls() const;
Immutable<std::vector<Immutable<Layer::Impl>>> getLayerImpls() const;
@@ -106,7 +107,7 @@ private:
std::unique_ptr<SpriteLoader> spriteLoader;
std::string glyphURL;
CollectionWithPersistentOrder<style::Image> images;
Immutable<ImageImpls> images = makeMutable<ImageImpls>();
CollectionWithPersistentOrder<Source> sources;
Collection<Layer> layers;
TransitionOptions transitionOptions;
@@ -117,7 +118,7 @@ private:
CameraOptions defaultCamera;
// SpriteLoaderObserver implementation.
void onSpriteLoaded(std::vector<std::unique_ptr<Image>>&&) override;
void onSpriteLoaded(std::vector<Immutable<style::Image::Impl>>) override;
void onSpriteError(std::exception_ptr) override;
// SourceObserver implementation.

View File

@@ -27,10 +27,10 @@ TEST(ImageManager, Basic) {
auto images = parseSprite(util::read_file("test/fixtures/annotations/emerald.png"),
util::read_file("test/fixtures/annotations/emerald.json"));
for (auto& image : images) {
imageManager.addImage(image->baseImpl);
auto* stored = imageManager.getImage(image->getID());
imageManager.addImage(image);
auto* stored = imageManager.getImage(image->id);
ASSERT_TRUE(stored);
EXPECT_EQ(image->getImage().size, stored->image.size);
EXPECT_EQ(image->image.size, stored->image.size);
}
}

View File

@@ -21,8 +21,8 @@ TEST(PatternAtlas, Basic) {
auto images = parseSprite(util::read_file("test/fixtures/annotations/emerald.png"),
util::read_file("test/fixtures/annotations/emerald.json"));
for (auto& image : images) {
if (image->getID() == "metro") {
ASSERT_TRUE(patternAtlas.addPattern(*image->baseImpl));
if (image->id == "metro") {
ASSERT_TRUE(patternAtlas.addPattern(*image));
}
}
auto found = patternAtlas.getPattern("metro");

View File

@@ -17,7 +17,7 @@ using namespace mbgl::style;
class StubSpriteLoaderObserver : public SpriteLoaderObserver {
public:
void onSpriteLoaded(std::vector<std::unique_ptr<style::Image>>&& images) override {
void onSpriteLoaded(std::vector<Immutable<style::Image::Impl>> images) override {
if (spriteLoaded) spriteLoaded(std::move(images));
}
@@ -25,7 +25,7 @@ public:
if (spriteError) spriteError(error);
}
std::function<void (std::vector<std::unique_ptr<style::Image>>&&)> spriteLoaded;
std::function<void(std::vector<Immutable<style::Image::Impl>>)> spriteLoaded;
std::function<void (std::exception_ptr)> spriteError;
};
@@ -92,7 +92,7 @@ TEST(SpriteLoader, LoadingSuccess) {
test.end();
};
test.observer.spriteLoaded = [&] (std::vector<std::unique_ptr<style::Image>>&& images) {
test.observer.spriteLoaded = [&](std::vector<Immutable<style::Image::Impl>> images) {
EXPECT_EQ(images.size(), 367u);
test.end();
};
@@ -169,7 +169,7 @@ TEST(SpriteLoader, LoadingCancel) {
return optional<Response>();
};
test.observer.spriteLoaded = [&] (const std::vector<std::unique_ptr<style::Image>>&) {
test.observer.spriteLoaded = [&](std::vector<Immutable<style::Image::Impl>>) {
FAIL() << "Should never be called";
};

View File

@@ -3,6 +3,7 @@
#include <mbgl/sprite/sprite_parser.hpp>
#include <mbgl/style/image.hpp>
#include <mbgl/style/image_impl.hpp>
#include <mbgl/util/image.hpp>
#include <mbgl/util/io.hpp>
#include <mbgl/util/string.hpp>
@@ -242,8 +243,8 @@ TEST(Sprite, SpriteParsing) {
const auto images = parseSprite(image_1x, json_1x);
std::set<std::string> names;
std::transform(images.begin(), images.end(), std::inserter(names, names.begin()),
[](const auto& image) { return image->getID(); });
std::transform(
images.begin(), images.end(), std::inserter(names, names.begin()), [](const auto& image) { return image->id; });
EXPECT_EQ(std::set<std::string>({ "airfield_icon",
"airport_icon",
@@ -321,11 +322,12 @@ TEST(Sprite, SpriteParsing) {
names);
{
auto& sprite = *std::find_if(images.begin(), images.end(), [] (const auto& image) { return image->getID() == "generic-metro"; });
EXPECT_EQ(18u, sprite->getImage().size.width);
EXPECT_EQ(18u, sprite->getImage().size.height);
EXPECT_EQ(1, sprite->getPixelRatio());
EXPECT_EQ(readImage("test/fixtures/annotations/result-spriteparsing.png"), sprite->getImage());
auto& sprite =
*std::find_if(images.begin(), images.end(), [](const auto& image) { return image->id == "generic-metro"; });
EXPECT_EQ(18u, sprite->image.size.width);
EXPECT_EQ(18u, sprite->image.size.height);
EXPECT_EQ(1, sprite->pixelRatio);
EXPECT_EQ(readImage("test/fixtures/annotations/result-spriteparsing.png"), sprite->image);
}
}
@@ -460,10 +462,10 @@ TEST(Sprite, SpriteParsingStretchAndContent) {
}
})JSON");
EXPECT_EQ(1u, images.size());
EXPECT_EQ("image", images[0]->getID());
EXPECT_EQ((style::ImageStretches{{2, 14}}), images[0]->getStretchX());
EXPECT_EQ((style::ImageStretches{{0, 4}, {12, 16}}), images[0]->getStretchY());
EXPECT_EQ((style::ImageContent{2, 2, 14, 14}), images[0]->getContent());
EXPECT_EQ("image", images[0]->id);
EXPECT_EQ((style::ImageStretches{{2, 14}}), images[0]->stretchX);
EXPECT_EQ((style::ImageStretches{{0, 4}, {12, 16}}), images[0]->stretchY);
EXPECT_EQ((style::ImageContent{2, 2, 14, 14}), images[0]->content);
}
TEST(Sprite, SpriteParsingEmptyImage) {