From cc9f040a2d35293c51dcc5be9c7affea7f1263bd Mon Sep 17 00:00:00 2001 From: Ivo van Dongen Date: Fri, 12 May 2017 23:19:00 +0300 Subject: [core] Split style image collection from SpriteAtlas --- cmake/core-files.cmake | 13 +- cmake/test-files.cmake | 2 + include/mbgl/style/image.hpp | 17 ++- platform/ios/src/UIImage+MGLAdditions.mm | 6 +- platform/macos/src/NSImage+MGLAdditions.mm | 4 +- src/mbgl/annotation/annotation_manager.cpp | 13 +- src/mbgl/annotation/annotation_manager.hpp | 3 + src/mbgl/map/map.cpp | 8 +- src/mbgl/sprite/sprite_atlas.cpp | 115 ++-------------- src/mbgl/sprite/sprite_atlas.hpp | 28 +--- src/mbgl/sprite/sprite_atlas_observer.hpp | 15 --- src/mbgl/sprite/sprite_atlas_worker.cpp | 29 ----- src/mbgl/sprite/sprite_atlas_worker.hpp | 23 ---- src/mbgl/sprite/sprite_image_collection.cpp | 40 ++++++ src/mbgl/sprite/sprite_image_collection.hpp | 25 ++++ src/mbgl/sprite/sprite_loader.cpp | 107 +++++++++++++++ src/mbgl/sprite/sprite_loader.hpp | 46 +++++++ src/mbgl/sprite/sprite_loader_observer.hpp | 24 ++++ src/mbgl/sprite/sprite_loader_worker.cpp | 29 +++++ src/mbgl/sprite/sprite_loader_worker.hpp | 23 ++++ src/mbgl/sprite/sprite_parser.hpp | 4 +- src/mbgl/style/image.cpp | 38 ++++-- src/mbgl/style/image_impl.cpp | 22 ++++ src/mbgl/style/image_impl.hpp | 25 ++++ src/mbgl/style/layers/symbol_layer_impl.hpp | 1 - src/mbgl/style/observer.hpp | 4 +- src/mbgl/style/style.cpp | 41 +++++- src/mbgl/style/style.hpp | 15 ++- test/map/map.test.cpp | 2 +- test/sprite/sprite_atlas.test.cpp | 187 --------------------------- test/sprite/sprite_image_collection.test.cpp | 45 +++++++ test/sprite/sprite_loader.test.cpp | 164 +++++++++++++++++++++++ test/sprite/sprite_parser.test.cpp | 40 +++--- test/src/mbgl/test/stub_style_observer.hpp | 6 +- test/style/style_image.test.cpp | 12 +- 35 files changed, 720 insertions(+), 456 deletions(-) delete mode 100644 src/mbgl/sprite/sprite_atlas_observer.hpp delete mode 100644 src/mbgl/sprite/sprite_atlas_worker.cpp delete mode 100644 src/mbgl/sprite/sprite_atlas_worker.hpp create mode 100644 src/mbgl/sprite/sprite_image_collection.cpp create mode 100644 src/mbgl/sprite/sprite_image_collection.hpp create mode 100644 src/mbgl/sprite/sprite_loader.cpp create mode 100644 src/mbgl/sprite/sprite_loader.hpp create mode 100644 src/mbgl/sprite/sprite_loader_observer.hpp create mode 100644 src/mbgl/sprite/sprite_loader_worker.cpp create mode 100644 src/mbgl/sprite/sprite_loader_worker.hpp create mode 100644 src/mbgl/style/image_impl.cpp create mode 100644 src/mbgl/style/image_impl.hpp create mode 100644 test/sprite/sprite_image_collection.test.cpp create mode 100644 test/sprite/sprite_loader.test.cpp diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index f83a0413b9..fbe43bef3b 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -283,9 +283,13 @@ set(MBGL_CORE_FILES # sprite src/mbgl/sprite/sprite_atlas.cpp src/mbgl/sprite/sprite_atlas.hpp - src/mbgl/sprite/sprite_atlas_observer.hpp - src/mbgl/sprite/sprite_atlas_worker.cpp - src/mbgl/sprite/sprite_atlas_worker.hpp + src/mbgl/sprite/sprite_image_collection.cpp + src/mbgl/sprite/sprite_image_collection.hpp + src/mbgl/sprite/sprite_loader.cpp + src/mbgl/sprite/sprite_loader.hpp + src/mbgl/sprite/sprite_loader_observer.hpp + src/mbgl/sprite/sprite_loader_worker.cpp + src/mbgl/sprite/sprite_loader_worker.hpp src/mbgl/sprite/sprite_parser.cpp src/mbgl/sprite/sprite_parser.hpp @@ -322,6 +326,8 @@ set(MBGL_CORE_FILES src/mbgl/style/class_dictionary.cpp src/mbgl/style/class_dictionary.hpp src/mbgl/style/image.cpp + src/mbgl/style/image_impl.cpp + src/mbgl/style/image_impl.hpp src/mbgl/style/layer.cpp src/mbgl/style/layer_impl.cpp src/mbgl/style/layer_impl.hpp @@ -363,6 +369,7 @@ set(MBGL_CORE_FILES include/mbgl/style/conversion/tileset.hpp include/mbgl/style/conversion/transition_options.hpp src/mbgl/style/conversion/geojson.cpp + src/mbgl/style/conversion/json.hpp src/mbgl/style/conversion/stringify.hpp # style/function diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake index 6026374de6..5eb9d8d9f8 100644 --- a/cmake/test-files.cmake +++ b/cmake/test-files.cmake @@ -46,6 +46,8 @@ set(MBGL_TEST_FILES # sprite test/sprite/sprite_atlas.test.cpp + test/sprite/sprite_image_collection.test.cpp + test/sprite/sprite_loader.test.cpp test/sprite/sprite_parser.test.cpp # src/mbgl/test diff --git a/include/mbgl/style/image.hpp b/include/mbgl/style/image.hpp index 499377467e..4abd86f1be 100644 --- a/include/mbgl/style/image.hpp +++ b/include/mbgl/style/image.hpp @@ -2,6 +2,8 @@ #include +#include + namespace mbgl { namespace style { @@ -9,16 +11,21 @@ class Image { public: Image(PremultipliedImage&&, float pixelRatio, bool sdf = false); - PremultipliedImage image; + PremultipliedImage& getImage() const; // Pixel ratio of the sprite image. - const float pixelRatio; + float getPixelRatio() const; // Whether this image should be interpreted as a signed distance field icon. - const bool sdf; + bool isSdf() const; + + float getWidth() const; + float getHeight() const; + + class Impl; - float getWidth() const { return image.size.width / pixelRatio; } - float getHeight() const { return image.size.height / pixelRatio; } +private: + const std::shared_ptr impl; }; } // namespace style diff --git a/platform/ios/src/UIImage+MGLAdditions.mm b/platform/ios/src/UIImage+MGLAdditions.mm index db64d78232..4ced1e30d5 100644 --- a/platform/ios/src/UIImage+MGLAdditions.mm +++ b/platform/ios/src/UIImage+MGLAdditions.mm @@ -6,14 +6,14 @@ - (nullable instancetype)initWithMGLStyleImage:(const mbgl::style::Image *)styleImage { - CGImageRef image = CGImageFromMGLPremultipliedImage(styleImage->image.clone()); + CGImageRef image = CGImageFromMGLPremultipliedImage(styleImage->getImage().clone()); if (!image) { return nil; } - if (self = [self initWithCGImage:image scale:styleImage->pixelRatio orientation:UIImageOrientationUp]) + if (self = [self initWithCGImage:image scale:styleImage->getPixelRatio() orientation:UIImageOrientationUp]) { - if (styleImage->sdf) + if (styleImage->isSdf()) { self = [self imageWithRenderingMode:UIImageRenderingModeAlwaysTemplate]; } diff --git a/platform/macos/src/NSImage+MGLAdditions.mm b/platform/macos/src/NSImage+MGLAdditions.mm index 91c4f7bf66..fee3fe2751 100644 --- a/platform/macos/src/NSImage+MGLAdditions.mm +++ b/platform/macos/src/NSImage+MGLAdditions.mm @@ -16,7 +16,7 @@ } - (nullable instancetype)initWithMGLStyleImage:(const mbgl::style::Image *)styleImage { - CGImageRef image = CGImageFromMGLPremultipliedImage(styleImage->image.clone()); + CGImageRef image = CGImageFromMGLPremultipliedImage(styleImage->getImage().clone()); if (!image) { return nil; } @@ -25,7 +25,7 @@ CGImageRelease(image); if (self = [self initWithSize:NSMakeSize(styleImage->getWidth(), styleImage->getHeight())]) { [self addRepresentation:rep]; - [self setTemplate:styleImage->sdf]; + [self setTemplate:styleImage->isSdf()]; } return self; } diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp index e11ecff30c..96cdd96750 100644 --- a/src/mbgl/annotation/annotation_manager.cpp +++ b/src/mbgl/annotation/annotation_manager.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -19,7 +20,7 @@ const std::string AnnotationManager::SourceID = "com.mapbox.annotations"; const std::string AnnotationManager::PointLayerID = "com.mapbox.annotations.points"; AnnotationManager::AnnotationManager(float pixelRatio) - : spriteAtlas({ 1024, 1024 }, pixelRatio) { + : spriteAtlas({ 1024, 1024 }, pixelRatio){ // This is a special atlas, holding only images added via addIcon, so we always treat it as // loaded. spriteAtlas.markAsLoaded(); @@ -190,16 +191,20 @@ void AnnotationManager::removeTile(AnnotationTile& tile) { } void AnnotationManager::addImage(const std::string& id, std::unique_ptr image) { - spriteAtlas.addImage(id, std::move(image)); + addSpriteImage(spriteImages, id, std::move(image), [&](style::Image& added) { + spriteAtlas.addImage(id, std::make_unique(added)); + }); } void AnnotationManager::removeImage(const std::string& id) { - spriteAtlas.removeImage(id); + removeSpriteImage(spriteImages, id, [&] () { + spriteAtlas.removeImage(id); + }); } double AnnotationManager::getTopOffsetPixelsForImage(const std::string& id) { const style::Image* image = spriteAtlas.getImage(id); - return image ? -(image->image.size.height / image->pixelRatio) / 2 : 0; + return image ? -(image->getImage().size.height / image->getPixelRatio()) / 2 : 0; } } // namespace mbgl diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp index de213c830d..69232677f9 100644 --- a/src/mbgl/annotation/annotation_manager.hpp +++ b/src/mbgl/annotation/annotation_manager.hpp @@ -9,6 +9,7 @@ #include #include #include +#include namespace mbgl { @@ -72,6 +73,8 @@ private: ShapeAnnotationMap shapeAnnotations; std::unordered_set obsoleteShapeAnnotationLayers; std::unordered_set tiles; + + std::unordered_map> spriteImages; SpriteAtlas spriteAtlas; friend class AnnotationTile; diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index 4ef422a2de..16254f6e9b 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -942,8 +942,7 @@ void Map::addImage(const std::string& id, std::unique_ptr image) { } impl->styleMutated = true; - impl->style->spriteAtlas->addImage(id, std::move(image)); - impl->onUpdate(Update::Repaint); + impl->style->addImage(id, std::move(image)); } void Map::removeImage(const std::string& id) { @@ -952,13 +951,12 @@ void Map::removeImage(const std::string& id) { } impl->styleMutated = true; - impl->style->spriteAtlas->removeImage(id); - impl->onUpdate(Update::Repaint); + impl->style->removeImage(id); } const style::Image* Map::getImage(const std::string& id) { if (impl->style) { - return impl->style->spriteAtlas->getImage(id); + return impl->style->getImage(id); } return nullptr; } diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp index fed8451aed..bef74b7ce5 100644 --- a/src/mbgl/sprite/sprite_atlas.cpp +++ b/src/mbgl/sprite/sprite_atlas.cpp @@ -1,7 +1,4 @@ #include -#include -#include -#include #include #include #include @@ -9,11 +6,6 @@ #include #include #include -#include -#include -#include -#include -#include #include #include @@ -21,28 +13,12 @@ namespace mbgl { -static SpriteAtlasObserver nullObserver; - -struct SpriteAtlas::Loader { - Loader(Scheduler& scheduler, SpriteAtlas& spriteAtlas) - : mailbox(std::make_shared(*util::RunLoop::Get())), - worker(scheduler, ActorRef(spriteAtlas, mailbox)) { - } - - std::shared_ptr image; - std::shared_ptr json; - std::unique_ptr jsonRequest; - std::unique_ptr spriteRequest; - std::shared_ptr mailbox; - Actor worker; -}; - SpriteAtlasElement::SpriteAtlasElement(Rect rect_, const style::Image& image, Size size_, float pixelRatio) : pos(std::move(rect_)), - sdf(image.sdf), - relativePixelRatio(image.pixelRatio / pixelRatio), + sdf(image.isSdf()), + relativePixelRatio(image.getPixelRatio() / pixelRatio), width(image.getWidth()), height(image.getHeight()) { @@ -59,87 +35,25 @@ SpriteAtlasElement::SpriteAtlasElement(Rect rect_, SpriteAtlas::SpriteAtlas(Size size_, float pixelRatio_) : size(std::move(size_)), pixelRatio(pixelRatio_), - observer(&nullObserver), bin(size.width, size.height), dirty(true) { } SpriteAtlas::~SpriteAtlas() = default; -void SpriteAtlas::load(const std::string& url, Scheduler& scheduler, FileSource& fileSource) { - if (url.empty()) { - // Treat a non-existent sprite as a successfully loaded empty sprite. - markAsLoaded(); - return; - } - - loader = std::make_unique(scheduler, *this); - - loader->jsonRequest = fileSource.request(Resource::spriteJSON(url, pixelRatio), [this](Response res) { - if (res.error) { - observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message))); - } else if (res.notModified) { - return; - } else if (res.noContent) { - loader->json = std::make_shared(); - emitSpriteLoadedIfComplete(); - } else { - // Only trigger a sprite loaded event we got new data. - loader->json = res.data; - emitSpriteLoadedIfComplete(); - } - }); - - loader->spriteRequest = fileSource.request(Resource::spriteImage(url, pixelRatio), [this](Response res) { - if (res.error) { - observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message))); - } else if (res.notModified) { - return; - } else if (res.noContent) { - loader->image = std::make_shared(); - emitSpriteLoadedIfComplete(); - } else { - loader->image = res.data; - emitSpriteLoadedIfComplete(); - } - }); -} - -void SpriteAtlas::emitSpriteLoadedIfComplete() { - assert(loader); - - if (!loader->image || !loader->json) { - return; - } - - loader->worker.invoke(&SpriteAtlasWorker::parse, loader->image, loader->json); - // TODO: delete the loader? -} - -void SpriteAtlas::onParsed(Images&& result) { +void SpriteAtlas::onSpriteLoaded(Images&& result) { markAsLoaded(); + for (auto& pair : result) { addImage(pair.first, std::move(pair.second)); } - observer->onSpriteLoaded(); + for (auto requestor : requestors) { requestor->onIconsAvailable(buildIconMap()); } requestors.clear(); } -void SpriteAtlas::onError(std::exception_ptr err) { - observer->onSpriteError(err); -} - -void SpriteAtlas::setObserver(SpriteAtlasObserver* observer_) { - observer = observer_; -} - -void SpriteAtlas::dumpDebugLogs() const { - Log::Info(Event::General, "SpriteAtlas::loaded: %d", loaded); -} - void SpriteAtlas::addImage(const std::string& id, std::unique_ptr image_) { icons.clear(); @@ -152,10 +66,7 @@ void SpriteAtlas::addImage(const std::string& id, std::unique_ptr Entry& entry = it->second; // There is already a sprite with that name in our store. - if (entry.image->image.size != image_->image.size) { - Log::Warning(Event::Sprite, "Can't change sprite dimensions for '%s'", id.c_str()); - return; - } + assert(entry.image->getImage().size == image_->getImage().size); entry.image = std::move(image_); @@ -172,9 +83,7 @@ void SpriteAtlas::removeImage(const std::string& id) { icons.clear(); auto it = entries.find(id); - if (it == entries.end()) { - return; - } + assert(it != entries.end()); Entry& entry = it->second; @@ -243,8 +152,8 @@ optional SpriteAtlas::getImage(const std::string& id, }; } - const uint16_t pixelWidth = std::ceil(entry.image->image.size.width / pixelRatio); - const uint16_t pixelHeight = std::ceil(entry.image->image.size.height / pixelRatio); + const uint16_t pixelWidth = std::ceil(entry.image->getImage().size.width / pixelRatio); + const uint16_t pixelHeight = std::ceil(entry.image->getImage().size.height / pixelRatio); // Increase to next number divisible by 4, but at least 1. // This is so we can scale down the texture coordinates and pack them @@ -279,7 +188,7 @@ void SpriteAtlas::copy(const Entry& entry, optional> Entry::*entr image.fill(0); } - const PremultipliedImage& src = entry.image->image; + const PremultipliedImage& src = entry.image->getImage(); const Rect& rect = *(entry.*entryRect); const uint32_t padding = 1; @@ -337,4 +246,8 @@ void SpriteAtlas::bind(bool linear, gl::Context& context, gl::TextureUnit unit) linear ? gl::TextureFilter::Linear : gl::TextureFilter::Nearest); } +void SpriteAtlas::dumpDebugLogs() const { + Log::Info(Event::General, "SpriteAtlas::loaded: %d", loaded); +} + } // namespace mbgl diff --git a/src/mbgl/sprite/sprite_atlas.hpp b/src/mbgl/sprite/sprite_atlas.hpp index 5d99677726..7ae583dcba 100644 --- a/src/mbgl/sprite/sprite_atlas.hpp +++ b/src/mbgl/sprite/sprite_atlas.hpp @@ -7,7 +7,6 @@ #include #include -#include #include #include #include @@ -15,10 +14,6 @@ namespace mbgl { -class Scheduler; -class FileSource; -class SpriteAtlasObserver; - namespace gl { class Context; } // namespace gl @@ -38,7 +33,7 @@ public: float height; }; -using IconMap = std::map; +using IconMap = std::unordered_map; using IconDependencies = std::set; class IconRequestor { @@ -48,12 +43,12 @@ public: class SpriteAtlas : public util::noncopyable { public: - using Images = std::map>; + using Images = std::unordered_map>; SpriteAtlas(Size, float pixelRatio); ~SpriteAtlas(); - void load(const std::string& url, Scheduler&, FileSource&); + void onSpriteLoaded(Images&&); void markAsLoaded() { loaded = true; @@ -65,8 +60,6 @@ public: void dumpDebugLogs() const; - void setObserver(SpriteAtlasObserver*); - const style::Image* getImage(const std::string&) const; void addImage(const std::string&, std::unique_ptr); void removeImage(const std::string&); @@ -93,25 +86,12 @@ public: } private: - void emitSpriteLoadedIfComplete(); - - // Invoked by SpriteAtlasWorker - friend class SpriteAtlasWorker; - void onParsed(Images&& result); - void onError(std::exception_ptr); - const Size size; const float pixelRatio; - - struct Loader; - std::unique_ptr loader; - bool loaded = false; - SpriteAtlasObserver* observer = nullptr; - struct Entry { - std::unique_ptr image; + std::unique_ptr image; // One sprite image might be used as both an icon image and a pattern image. If so, // it must have two distinct entries in the texture. The one for the icon image has diff --git a/src/mbgl/sprite/sprite_atlas_observer.hpp b/src/mbgl/sprite/sprite_atlas_observer.hpp deleted file mode 100644 index 263c95b0e8..0000000000 --- a/src/mbgl/sprite/sprite_atlas_observer.hpp +++ /dev/null @@ -1,15 +0,0 @@ -#pragma once - -#include - -namespace mbgl { - -class SpriteAtlasObserver { -public: - virtual ~SpriteAtlasObserver() = default; - - virtual void onSpriteLoaded() {} - virtual void onSpriteError(std::exception_ptr) {} -}; - -} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_atlas_worker.cpp b/src/mbgl/sprite/sprite_atlas_worker.cpp deleted file mode 100644 index da507aabab..0000000000 --- a/src/mbgl/sprite/sprite_atlas_worker.cpp +++ /dev/null @@ -1,29 +0,0 @@ -#include -#include -#include - -namespace mbgl { - -SpriteAtlasWorker::SpriteAtlasWorker(ActorRef, ActorRef parent_) - : parent(std::move(parent_)) { -} - -void SpriteAtlasWorker::parse(std::shared_ptr image, - std::shared_ptr json) { - try { - if (!image) { - // This shouldn't happen, since we always invoke it with a non-empty pointer. - throw std::runtime_error("missing sprite image"); - } - if (!json) { - // This shouldn't happen, since we always invoke it with a non-empty pointer. - throw std::runtime_error("missing sprite metadata"); - } - - parent.invoke(&SpriteAtlas::onParsed, parseSprite(*image, *json)); - } catch (...) { - parent.invoke(&SpriteAtlas::onError, std::current_exception()); - } -} - -} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_atlas_worker.hpp b/src/mbgl/sprite/sprite_atlas_worker.hpp deleted file mode 100644 index 70ca0d52e5..0000000000 --- a/src/mbgl/sprite/sprite_atlas_worker.hpp +++ /dev/null @@ -1,23 +0,0 @@ -#pragma once - -#include -#include - -#include -#include - -namespace mbgl { - -class SpriteAtlas; - -class SpriteAtlasWorker { -public: - SpriteAtlasWorker(ActorRef, ActorRef); - - void parse(std::shared_ptr image, std::shared_ptr json); - -private: - ActorRef parent; -}; - -} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_image_collection.cpp b/src/mbgl/sprite/sprite_image_collection.cpp new file mode 100644 index 0000000000..787ba83db2 --- /dev/null +++ b/src/mbgl/sprite/sprite_image_collection.cpp @@ -0,0 +1,40 @@ +#include +#include + +namespace mbgl { + +void addSpriteImage(Images& images, const std::string& id, + std::unique_ptr image_, + std::function onAdded) { + + auto it = images.find(id); + if (it == images.end()) { + // Add new + it = images.emplace(id, std::move(image_)).first; + onAdded(*it->second.get()); + return; + } + + std::unique_ptr& image = it->second; + + // There is already a sprite with that name in our store. + if (image->getImage().size != image_->getImage().size) { + Log::Warning(Event::Sprite, "Can't change sprite dimensions for '%s'", id.c_str()); + } + + // Update existing + image = std::move(image_); + onAdded(*it->second.get()); +} + +void removeSpriteImage(Images& images, + const std::string& id, + std::function onRemoved) { + if (images.erase(id) > 0) { + onRemoved(); + } +} + + + +} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_image_collection.hpp b/src/mbgl/sprite/sprite_image_collection.hpp new file mode 100644 index 0000000000..00e252f74f --- /dev/null +++ b/src/mbgl/sprite/sprite_image_collection.hpp @@ -0,0 +1,25 @@ +#pragma once + +#include + +#include +#include +#include +#include + +namespace mbgl { + +using Images = std::unordered_map>; + +void addSpriteImage(Images&, + const std::string&, + std::unique_ptr, + std::function onAdded = [] (style::Image&){}); + +void removeSpriteImage(Images&, + const std::string&, + std::function onRemoved = [] (){}); + + + +} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_loader.cpp b/src/mbgl/sprite/sprite_loader.cpp new file mode 100644 index 0000000000..86325bba35 --- /dev/null +++ b/src/mbgl/sprite/sprite_loader.cpp @@ -0,0 +1,107 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +namespace mbgl { + +static SpriteLoaderObserver nullObserver; + +struct SpriteLoader::Loader { + Loader(Scheduler& scheduler, SpriteLoader& spriteAtlas) + : mailbox(std::make_shared(*util::RunLoop::Get())), + worker(scheduler, ActorRef(spriteAtlas, mailbox)) { + } + + std::shared_ptr image; + std::shared_ptr json; + std::unique_ptr jsonRequest; + std::unique_ptr spriteRequest; + std::shared_ptr mailbox; + Actor worker; +}; + +SpriteLoader::SpriteLoader(float pixelRatio_) + : pixelRatio(pixelRatio_) + , observer(&nullObserver) { +} + +SpriteLoader::~SpriteLoader() = default; + +void SpriteLoader::load(const std::string& url, Scheduler& scheduler, FileSource& fileSource) { + if (url.empty()) { + // Treat a non-existent sprite as a successfully loaded empty sprite. + observer->onSpriteLoaded({}); + return; + } + + loader = std::make_unique(scheduler, *this); + + loader->jsonRequest = fileSource.request(Resource::spriteJSON(url, pixelRatio), [this](Response res) { + if (res.error) { + observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message))); + } else if (res.notModified) { + return; + } else if (res.noContent) { + loader->json = std::make_shared(); + emitSpriteLoadedIfComplete(); + } else { + // Only trigger a sprite loaded event we got new data. + loader->json = res.data; + emitSpriteLoadedIfComplete(); + } + }); + + loader->spriteRequest = fileSource.request(Resource::spriteImage(url, pixelRatio), [this](Response res) { + if (res.error) { + observer->onSpriteError(std::make_exception_ptr(std::runtime_error(res.error->message))); + } else if (res.notModified) { + return; + } else if (res.noContent) { + loader->image = std::make_shared(); + emitSpriteLoadedIfComplete(); + } else { + loader->image = res.data; + emitSpriteLoadedIfComplete(); + } + }); +} + +void SpriteLoader::emitSpriteLoadedIfComplete() { + assert(loader); + + if (!loader->image || !loader->json) { + return; + } + + loader->worker.invoke(&SpriteLoaderWorker::parse, loader->image, loader->json); +} + +void SpriteLoader::onParsed(Images&& result) { + + + + observer->onSpriteLoaded(std::move(result)); +} + +void SpriteLoader::onError(std::exception_ptr err) { + observer->onSpriteError(err); +} + +void SpriteLoader::setObserver(SpriteLoaderObserver* observer_) { + observer = observer_; +} + +} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_loader.hpp b/src/mbgl/sprite/sprite_loader.hpp new file mode 100644 index 0000000000..dbebede03b --- /dev/null +++ b/src/mbgl/sprite/sprite_loader.hpp @@ -0,0 +1,46 @@ +#pragma once + +#include +#include + +#include +#include +#include +#include +#include +#include + +namespace mbgl { + +class Scheduler; +class FileSource; +class SpriteLoaderObserver; + +class SpriteLoader : public util::noncopyable { +public: + using Images = std::unordered_map>; + + SpriteLoader(float pixelRatio); + ~SpriteLoader(); + + void load(const std::string& url, Scheduler&, FileSource&); + + void setObserver(SpriteLoaderObserver*); + +private: + void emitSpriteLoadedIfComplete(); + + // Invoked by SpriteAtlasWorker + friend class SpriteLoaderWorker; + void onParsed(Images&& result); + void onError(std::exception_ptr); + + const float pixelRatio; + + struct Loader; + std::unique_ptr loader; + + SpriteLoaderObserver* observer = nullptr; +}; + +} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_loader_observer.hpp b/src/mbgl/sprite/sprite_loader_observer.hpp new file mode 100644 index 0000000000..632c48ea56 --- /dev/null +++ b/src/mbgl/sprite/sprite_loader_observer.hpp @@ -0,0 +1,24 @@ +#pragma once + +#include +#include +#include +#include + +namespace mbgl { + +namespace style { +class Image; +} // namespace style + +class SpriteLoaderObserver { +public: + using Images = std::unordered_map>; + + virtual ~SpriteLoaderObserver() = default; + + virtual void onSpriteLoaded(Images&&) {} + virtual void onSpriteError(std::exception_ptr) {} +}; + +} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_loader_worker.cpp b/src/mbgl/sprite/sprite_loader_worker.cpp new file mode 100644 index 0000000000..4bded33d53 --- /dev/null +++ b/src/mbgl/sprite/sprite_loader_worker.cpp @@ -0,0 +1,29 @@ +#include +#include +#include + +namespace mbgl { + +SpriteLoaderWorker::SpriteLoaderWorker(ActorRef, ActorRef parent_) + : parent(std::move(parent_)) { +} + +void SpriteLoaderWorker::parse(std::shared_ptr image, + std::shared_ptr json) { + try { + if (!image) { + // This shouldn't happen, since we always invoke it with a non-empty pointer. + throw std::runtime_error("missing sprite image"); + } + if (!json) { + // This shouldn't happen, since we always invoke it with a non-empty pointer. + throw std::runtime_error("missing sprite metadata"); + } + + parent.invoke(&SpriteLoader::onParsed, parseSprite(*image, *json)); + } catch (...) { + parent.invoke(&SpriteLoader::onError, std::current_exception()); + } +} + +} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_loader_worker.hpp b/src/mbgl/sprite/sprite_loader_worker.hpp new file mode 100644 index 0000000000..d61e07d14f --- /dev/null +++ b/src/mbgl/sprite/sprite_loader_worker.hpp @@ -0,0 +1,23 @@ +#pragma once + +#include +#include + +#include +#include + +namespace mbgl { + +class SpriteLoader; + +class SpriteLoaderWorker { +public: + SpriteLoaderWorker(ActorRef, ActorRef); + + void parse(std::shared_ptr image, std::shared_ptr json); + +private: + ActorRef parent; +}; + +} // namespace mbgl diff --git a/src/mbgl/sprite/sprite_parser.hpp b/src/mbgl/sprite/sprite_parser.hpp index 5be8435ebb..175ec8a883 100644 --- a/src/mbgl/sprite/sprite_parser.hpp +++ b/src/mbgl/sprite/sprite_parser.hpp @@ -7,7 +7,7 @@ #include #include -#include +#include namespace mbgl { @@ -24,7 +24,7 @@ std::unique_ptr createStyleImage(const PremultipliedImage&, double ratio, bool sdf); -using Images = std::map>; +using Images = std::unordered_map>; // Parses an image and an associated JSON file and returns the sprite objects. Images parseSprite(const std::string& image, const std::string& json); diff --git a/src/mbgl/style/image.cpp b/src/mbgl/style/image.cpp index 4c0c6a859b..c5fecec2b2 100644 --- a/src/mbgl/style/image.cpp +++ b/src/mbgl/style/image.cpp @@ -1,21 +1,35 @@ #include +#include #include namespace mbgl { namespace style { -Image::Image(PremultipliedImage&& image_, - const float pixelRatio_, - bool sdf_) - : image(std::move(image_)), - pixelRatio(pixelRatio_), - sdf(sdf_) { - - if (!image.valid()) { - throw util::SpriteImageException("Sprite image dimensions may not be zero"); - } else if (pixelRatio <= 0) { - throw util::SpriteImageException("Sprite pixelRatio may not be <= 0"); - } +Image::Image(PremultipliedImage &&image, + const float pixelRatio, + bool sdf) + : impl(std::make_shared(std::move(image), pixelRatio, sdf)) { +} + +PremultipliedImage& Image::getImage() const { + assert(impl); + return impl->image; +} + +bool Image::isSdf() const { + return impl->sdf; +} + +float Image::getPixelRatio() const { + return impl->pixelRatio; +} + +float Image::getWidth() const { + return impl->getWidth(); +} + +float Image::getHeight() const { + return impl->getHeight(); } } // namespace style diff --git a/src/mbgl/style/image_impl.cpp b/src/mbgl/style/image_impl.cpp new file mode 100644 index 0000000000..910bffa905 --- /dev/null +++ b/src/mbgl/style/image_impl.cpp @@ -0,0 +1,22 @@ +#include +#include + +namespace mbgl { +namespace style { + +Image::Impl::Impl(PremultipliedImage&& image_, + const float pixelRatio_, + bool sdf_) + : image(std::move(image_)), + pixelRatio(pixelRatio_), + sdf(sdf_) { + + if (!image.valid()) { + throw util::SpriteImageException("Sprite image dimensions may not be zero"); + } else if (pixelRatio <= 0) { + throw util::SpriteImageException("Sprite pixelRatio may not be <= 0"); + } +} + +} // namespace style +} // namespace mbgl diff --git a/src/mbgl/style/image_impl.hpp b/src/mbgl/style/image_impl.hpp new file mode 100644 index 0000000000..dce4a6e4c0 --- /dev/null +++ b/src/mbgl/style/image_impl.hpp @@ -0,0 +1,25 @@ +#pragma once + +#include + +namespace mbgl { +namespace style { + +class Image::Impl { +public: + Impl(PremultipliedImage&&, float pixelRatio, bool sdf = false); + + PremultipliedImage image; + + // Pixel ratio of the sprite image. + const float pixelRatio; + + // Whether this image should be interpreted as a signed distance field icon. + const bool sdf; + + float getWidth() const { return image.size.width / pixelRatio; } + float getHeight() const { return image.size.height / pixelRatio; } +}; + +} // namespace style +} // namespace mbgl diff --git a/src/mbgl/style/layers/symbol_layer_impl.hpp b/src/mbgl/style/layers/symbol_layer_impl.hpp index 5e04834583..a20d001e0e 100644 --- a/src/mbgl/style/layers/symbol_layer_impl.hpp +++ b/src/mbgl/style/layers/symbol_layer_impl.hpp @@ -1,6 +1,5 @@ #pragma once -#include #include #include #include diff --git a/src/mbgl/style/observer.hpp b/src/mbgl/style/observer.hpp index 60432334e2..77d97685be 100644 --- a/src/mbgl/style/observer.hpp +++ b/src/mbgl/style/observer.hpp @@ -1,7 +1,7 @@ #pragma once #include -#include +#include #include #include @@ -11,7 +11,7 @@ namespace mbgl { namespace style { class Observer : public GlyphAtlasObserver, - public SpriteAtlasObserver, + public SpriteLoaderObserver, public SourceObserver { public: virtual void onUpdate(Update) {} diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index 6af876d740..0dcb110443 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -17,6 +17,8 @@ #include #include #include +#include +#include #include #include #include @@ -72,13 +74,14 @@ Style::Style(Scheduler& scheduler_, FileSource& fileSource_, float pixelRatio) : scheduler(scheduler_), fileSource(fileSource_), glyphAtlas(std::make_unique(Size{ 2048, 2048 }, fileSource)), + spriteLoader(std::make_unique(pixelRatio)), spriteAtlas(std::make_unique(Size{ 1024, 1024 }, pixelRatio)), lineAtlas(std::make_unique(Size{ 256, 512 })), light(std::make_unique()), renderLight(light->impl), observer(&nullObserver) { glyphAtlas->setObserver(this); - spriteAtlas->setObserver(this); + spriteLoader->setObserver(this); light->setObserver(this); } @@ -160,7 +163,7 @@ void Style::setJSON(const std::string& json) { setLight(std::make_unique(parser.light)); glyphAtlas->setURL(parser.glyphURL); - spriteAtlas->load(parser.spriteURL, scheduler, fileSource); + spriteLoader->load(parser.spriteURL, scheduler, fileSource); loaded = true; @@ -539,6 +542,24 @@ bool Style::isLoaded() const { return true; } +void Style::addImage(const std::string& id, std::unique_ptr image) { + addSpriteImage(spriteImages, id, std::move(image), [&](style::Image& added) { + spriteAtlas->addImage(id, std::make_unique(added)); + observer->onUpdate(Update::Repaint); + }); +} + +void Style::removeImage(const std::string& id) { + removeSpriteImage(spriteImages, id, [&] () { + spriteAtlas->removeImage(id); + observer->onUpdate(Update::Repaint); + }); +} + +const style::Image* Style::getImage(const std::string& id) const { + return spriteAtlas->getImage(id); +} + RenderData Style::getRenderData(MapDebugOptions debugOptions, float angle) const { RenderData result; @@ -755,8 +776,20 @@ void Style::onTileError(RenderSource& source, const OverscaledTileID& tileID, st observer->onResourceError(error); } -void Style::onSpriteLoaded() { - observer->onSpriteLoaded(); +void Style::onSpriteLoaded(SpriteLoader::Images&& images) { + // Add images to collection + Images addedImages; + for (auto& entry : images) { + addSpriteImage(spriteImages, entry.first, std::move(entry.second), [&] (style::Image& added) { + addedImages.emplace(entry.first, std::make_unique(added)); + }); + } + + // Update render sprite atlas + spriteAtlas->onSpriteLoaded(std::move(addedImages)); + + // Update observer + observer->onSpriteLoaded(std::move(images)); observer->onUpdate(Update::Repaint); // For *-pattern properties. } diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp index 04d3a11d55..5953d823d9 100644 --- a/src/mbgl/style/style.hpp +++ b/src/mbgl/style/style.hpp @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include #include @@ -31,6 +31,7 @@ namespace mbgl { class FileSource; class GlyphAtlas; class SpriteAtlas; +class SpriteLoader; class LineAtlas; class RenderData; class TransformState; @@ -44,7 +45,7 @@ class Layer; class QueryParameters; class Style : public GlyphAtlasObserver, - public SpriteAtlasObserver, + public SpriteLoaderObserver, public SourceObserver, public RenderSourceObserver, public LayerObserver, @@ -106,6 +107,10 @@ public: Light* getLight() const; const RenderLight& getRenderLight() const; + const style::Image* getImage(const std::string&) const; + void addImage(const std::string&, std::unique_ptr); + void removeImage(const std::string&); + RenderData getRenderData(MapDebugOptions, float angle) const; std::vector queryRenderedFeatures(const ScreenLineString& geometry, @@ -120,6 +125,7 @@ public: Scheduler& scheduler; FileSource& fileSource; std::unique_ptr glyphAtlas; + std::unique_ptr spriteLoader; std::unique_ptr spriteAtlas; std::unique_ptr lineAtlas; @@ -152,8 +158,9 @@ private: void onGlyphsLoaded(const FontStack&, const GlyphRange&) override; void onGlyphsError(const FontStack&, const GlyphRange&, std::exception_ptr) override; - // SpriteStoreObserver implementation. - void onSpriteLoaded() override; + // SpriteLoaderObserver implementation. + std::unordered_map> spriteImages; + void onSpriteLoaded(SpriteLoaderObserver::Images&&) override; void onSpriteError(std::exception_ptr) override; // SourceObserver implementation. diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp index c24f736fcd..6809fefb65 100644 --- a/test/map/map.test.cpp +++ b/test/map/map.test.cpp @@ -512,7 +512,7 @@ TEST(Map, GetImage) { map.setStyleJSON(util::read_file("test/fixtures/api/icon_style.json")); map.addImage("test-icon", std::move(image)); - test::checkImage("test/fixtures/map/get_icon", map.getImage("test-icon")->image); + test::checkImage("test/fixtures/map/get_icon", map.getImage("test-icon")->getImage()); } TEST(Map, DontLoadUnneededTiles) { diff --git a/test/sprite/sprite_atlas.test.cpp b/test/sprite/sprite_atlas.test.cpp index 08388f0a93..0cbd211de8 100644 --- a/test/sprite/sprite_atlas.test.cpp +++ b/test/sprite/sprite_atlas.test.cpp @@ -3,7 +3,6 @@ #include #include -#include #include #include #include @@ -185,189 +184,3 @@ TEST(SpriteAtlas, RemoveReleasesBinPackRect) { EXPECT_TRUE(atlas.getIcon("big")); EXPECT_TRUE(log.empty()); } - -TEST(SpriteAtlas, OtherPixelRatio) { - FixtureLog log; - SpriteAtlas atlas({ 32, 32 }, 1); - - // Adding mismatched sprite image - atlas.addImage("one", std::make_unique(PremultipliedImage({ 8, 8 }), 2)); -} - -TEST(SpriteAtlas, Replace) { - FixtureLog log; - SpriteAtlas atlas({ 32, 32 }, 1); - - atlas.addImage("sprite", std::make_unique(PremultipliedImage({ 16, 16 }), 2)); - auto image = atlas.getImage("sprite"); - atlas.addImage("sprite", std::make_unique(PremultipliedImage({ 16, 16 }), 2)); - EXPECT_NE(image, atlas.getImage("sprite")); -} - -TEST(SpriteAtlas, ReplaceWithDifferentDimensions) { - FixtureLog log; - SpriteAtlas atlas({ 32, 32 }, 1); - - atlas.addImage("sprite", std::make_unique(PremultipliedImage({ 16, 16 }), 2)); - atlas.addImage("sprite", std::make_unique(PremultipliedImage({ 18, 18 }), 2)); - - EXPECT_EQ(1u, log.count({ - EventSeverity::Warning, - Event::Sprite, - int64_t(-1), - "Can't change sprite dimensions for 'sprite'", - })); -} - -class SpriteAtlasTest { -public: - SpriteAtlasTest() = default; - - util::RunLoop loop; - StubFileSource fileSource; - StubStyleObserver observer; - ThreadPool threadPool { 1 }; - SpriteAtlas spriteAtlas{ { 32, 32 }, 1 }; - - void run() { - // Squelch logging. - Log::setObserver(std::make_unique()); - - spriteAtlas.setObserver(&observer); - spriteAtlas.load("test/fixtures/resources/sprite", threadPool, fileSource); - - loop.run(); - } - - void end() { - loop.stop(); - } -}; - -Response successfulSpriteImageResponse(const Resource& resource) { - EXPECT_EQ("test/fixtures/resources/sprite.png", resource.url); - Response response; - response.data = std::make_unique(util::read_file(resource.url)); - return response; -} - -Response successfulSpriteJSONResponse(const Resource& resource) { - EXPECT_EQ("test/fixtures/resources/sprite.json", resource.url); - Response response; - response.data = std::make_unique(util::read_file(resource.url)); - return response; -} - -Response failedSpriteResponse(const Resource&) { - Response response; - response.error = std::make_unique( - Response::Error::Reason::Other, - "Failed by the test case"); - return response; -} - -Response corruptSpriteResponse(const Resource&) { - Response response; - response.data = std::make_unique("CORRUPT"); - return response; -} - -TEST(SpriteAtlas, LoadingSuccess) { - SpriteAtlasTest test; - - test.fileSource.spriteImageResponse = successfulSpriteImageResponse; - test.fileSource.spriteJSONResponse = successfulSpriteJSONResponse; - - test.observer.spriteError = [&] (std::exception_ptr error) { - FAIL() << util::toString(error); - test.end(); - }; - - test.observer.spriteLoaded = [&] () { - EXPECT_EQ(1.0, test.spriteAtlas.getPixelRatio()); - EXPECT_TRUE(test.spriteAtlas.isLoaded()); - test.end(); - }; - - test.run(); -} - -TEST(SpriteAtlas, JSONLoadingFail) { - SpriteAtlasTest test; - - test.fileSource.spriteImageResponse = successfulSpriteImageResponse; - test.fileSource.spriteJSONResponse = failedSpriteResponse; - - test.observer.spriteError = [&] (std::exception_ptr error) { - EXPECT_TRUE(error != nullptr); - EXPECT_EQ("Failed by the test case", util::toString(error)); - EXPECT_FALSE(test.spriteAtlas.isLoaded()); - test.end(); - }; - - test.run(); -} - -TEST(SpriteAtlas, ImageLoadingFail) { - SpriteAtlasTest test; - - test.fileSource.spriteImageResponse = failedSpriteResponse; - test.fileSource.spriteJSONResponse = successfulSpriteJSONResponse; - - test.observer.spriteError = [&] (std::exception_ptr error) { - EXPECT_TRUE(error != nullptr); - EXPECT_EQ("Failed by the test case", util::toString(error)); - EXPECT_FALSE(test.spriteAtlas.isLoaded()); - test.end(); - }; - - test.run(); -} - -TEST(SpriteAtlas, JSONLoadingCorrupted) { - SpriteAtlasTest test; - - test.fileSource.spriteImageResponse = successfulSpriteImageResponse; - test.fileSource.spriteJSONResponse = corruptSpriteResponse; - - test.observer.spriteError = [&] (std::exception_ptr error) { - EXPECT_TRUE(error != nullptr); - EXPECT_EQ("Failed to parse JSON: Invalid value. at offset 0", util::toString(error)); - EXPECT_FALSE(test.spriteAtlas.isLoaded()); - test.end(); - }; - - test.run(); -} - -TEST(SpriteAtlas, ImageLoadingCorrupted) { - SpriteAtlasTest test; - - test.fileSource.spriteImageResponse = corruptSpriteResponse; - test.fileSource.spriteJSONResponse = successfulSpriteJSONResponse; - - test.observer.spriteError = [&] (std::exception_ptr error) { - EXPECT_TRUE(error != nullptr); - // Not asserting on platform-specific error text. - EXPECT_FALSE(test.spriteAtlas.isLoaded()); - test.end(); - }; - - test.run(); -} - -TEST(SpriteAtlas, LoadingCancel) { - SpriteAtlasTest test; - - test.fileSource.spriteImageResponse = - test.fileSource.spriteJSONResponse = [&] (const Resource&) { - test.end(); - return optional(); - }; - - test.observer.spriteLoaded = [&] () { - FAIL() << "Should never be called"; - }; - - test.run(); -} diff --git a/test/sprite/sprite_image_collection.test.cpp b/test/sprite/sprite_image_collection.test.cpp new file mode 100644 index 0000000000..51ea20fdf3 --- /dev/null +++ b/test/sprite/sprite_image_collection.test.cpp @@ -0,0 +1,45 @@ +#include +#include + +#include +#include +#include + +#include + +using namespace mbgl; + +TEST(SpriteImageCollection, OtherPixelRatio) { + FixtureLog log; + Images images; + + // Adding mismatched sprite image + addSpriteImage(images, "one", std::make_unique(PremultipliedImage({ 8, 8 }), 2)); +} + +TEST(SpriteImageCollection, Replace) { + FixtureLog log; + Images images; + + addSpriteImage(images, "sprite", std::make_unique(PremultipliedImage({ 16, 16 }), 2)); + auto image = images.find("sprite")->second.get(); + addSpriteImage(images, "sprite", std::make_unique(PremultipliedImage({ 16, 16 }), 2)); + EXPECT_NE(image, images.find("sprite")->second.get()); +} + +TEST(SpriteImageCollection, ReplaceWithDifferentDimensions) { + FixtureLog log; + Images images; + + addSpriteImage(images, "sprite", std::make_unique(PremultipliedImage({ 16, 16 }), 2)); + addSpriteImage(images, "sprite", std::make_unique(PremultipliedImage({ 18, 18 }), 2)); + + EXPECT_EQ(1u, log.count({ + EventSeverity::Warning, + Event::Sprite, + int64_t(-1), + "Can't change sprite dimensions for 'sprite'", + })); +} + + diff --git a/test/sprite/sprite_loader.test.cpp b/test/sprite/sprite_loader.test.cpp new file mode 100644 index 0000000000..06018cc390 --- /dev/null +++ b/test/sprite/sprite_loader.test.cpp @@ -0,0 +1,164 @@ +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include + +using namespace mbgl; + +class SpriteLoaderTest { +public: + SpriteLoaderTest() = default; + + util::RunLoop loop; + StubFileSource fileSource; + StubStyleObserver observer; + ThreadPool threadPool { 1 }; + SpriteLoader spriteLoader{ 1 }; + + void run() { + // Squelch logging. + Log::setObserver(std::make_unique()); + + spriteLoader.setObserver(&observer); + spriteLoader.load("test/fixtures/resources/sprite", threadPool, fileSource); + + loop.run(); + } + + void end() { + loop.stop(); + } +}; + +Response successfulSpriteImageResponse(const Resource& resource) { + EXPECT_EQ("test/fixtures/resources/sprite.png", resource.url); + Response response; + response.data = std::make_unique(util::read_file(resource.url)); + return response; +} + +Response successfulSpriteJSONResponse(const Resource& resource) { + EXPECT_EQ("test/fixtures/resources/sprite.json", resource.url); + Response response; + response.data = std::make_unique(util::read_file(resource.url)); + return response; +} + +Response failedSpriteResponse(const Resource&) { + Response response; + response.error = std::make_unique( + Response::Error::Reason::Other, + "Failed by the test case"); + return response; +} + +Response corruptSpriteResponse(const Resource&) { + Response response; + response.data = std::make_unique("CORRUPT"); + return response; +} + +TEST(SpriteLoader, LoadingSuccess) { + SpriteLoaderTest test; + + test.fileSource.spriteImageResponse = successfulSpriteImageResponse; + test.fileSource.spriteJSONResponse = successfulSpriteJSONResponse; + + test.observer.spriteError = [&] (std::exception_ptr error) { + FAIL() << util::toString(error); + test.end(); + }; + + test.observer.spriteLoaded = [&] (SpriteLoaderObserver::Images&& images) { + EXPECT_EQ(images.size(), Images::size_type(367)); + test.end(); + }; + + test.run(); +} + +TEST(SpriteLoader, JSONLoadingFail) { + SpriteLoaderTest test; + + test.fileSource.spriteImageResponse = successfulSpriteImageResponse; + test.fileSource.spriteJSONResponse = failedSpriteResponse; + + test.observer.spriteError = [&] (std::exception_ptr error) { + EXPECT_TRUE(error != nullptr); + EXPECT_EQ("Failed by the test case", util::toString(error)); + test.end(); + }; + + test.run(); +} + +TEST(SpriteLoader, ImageLoadingFail) { + SpriteLoaderTest test; + + test.fileSource.spriteImageResponse = failedSpriteResponse; + test.fileSource.spriteJSONResponse = successfulSpriteJSONResponse; + + test.observer.spriteError = [&] (std::exception_ptr error) { + EXPECT_TRUE(error != nullptr); + EXPECT_EQ("Failed by the test case", util::toString(error)); + test.end(); + }; + + test.run(); +} + +TEST(SpriteLoader, JSONLoadingCorrupted) { + SpriteLoaderTest test; + + test.fileSource.spriteImageResponse = successfulSpriteImageResponse; + test.fileSource.spriteJSONResponse = corruptSpriteResponse; + + test.observer.spriteError = [&] (std::exception_ptr error) { + EXPECT_TRUE(error != nullptr); + EXPECT_EQ("Failed to parse JSON: Invalid value. at offset 0", util::toString(error)); + test.end(); + }; + + test.run(); +} + +TEST(SpriteLoader, ImageLoadingCorrupted) { + SpriteLoaderTest test; + + test.fileSource.spriteImageResponse = corruptSpriteResponse; + test.fileSource.spriteJSONResponse = successfulSpriteJSONResponse; + + test.observer.spriteError = [&] (std::exception_ptr error) { + EXPECT_TRUE(error != nullptr); + // Not asserting on platform-specific error text. + test.end(); + }; + + test.run(); +} + +TEST(SpriteLoader, LoadingCancel) { + SpriteLoaderTest test; + + test.fileSource.spriteImageResponse = + test.fileSource.spriteJSONResponse = [&] (const Resource&) { + test.end(); + return optional(); + }; + + test.observer.spriteLoaded = [&] (const SpriteLoaderObserver::Images&) { + FAIL() << "Should never be called"; + }; + + test.run(); +} diff --git a/test/sprite/sprite_parser.test.cpp b/test/sprite/sprite_parser.test.cpp index bb8e71db95..19c6b914a8 100644 --- a/test/sprite/sprite_parser.test.cpp +++ b/test/sprite/sprite_parser.test.cpp @@ -145,11 +145,11 @@ TEST(Sprite, SpriteImageCreation1x) { ASSERT_TRUE(sprite.get()); EXPECT_EQ(18, sprite->getWidth()); EXPECT_EQ(18, sprite->getHeight()); - EXPECT_EQ(18u, sprite->image.size.width); - EXPECT_EQ(18u, sprite->image.size.height); - EXPECT_EQ(1, sprite->pixelRatio); + 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-spriteimagecreation1x-museum.png"), - sprite->image); + sprite->getImage()); } } @@ -161,11 +161,11 @@ TEST(Sprite, SpriteImageCreation2x) { ASSERT_TRUE(sprite.get()); EXPECT_EQ(18, sprite->getWidth()); EXPECT_EQ(18, sprite->getHeight()); - EXPECT_EQ(36u, sprite->image.size.width); - EXPECT_EQ(36u, sprite->image.size.height); - EXPECT_EQ(2, sprite->pixelRatio); + EXPECT_EQ(36u, sprite->getImage().size.width); + EXPECT_EQ(36u, sprite->getImage().size.height); + EXPECT_EQ(2, sprite->getPixelRatio()); EXPECT_EQ(readImage("test/fixtures/annotations/result-spriteimagecreation2x.png"), - sprite->image); + sprite->getImage()); } TEST(Sprite, SpriteImageCreation1_5x) { @@ -176,22 +176,22 @@ TEST(Sprite, SpriteImageCreation1_5x) { ASSERT_TRUE(sprite.get()); EXPECT_EQ(24, sprite->getWidth()); EXPECT_EQ(24, sprite->getHeight()); - EXPECT_EQ(36u, sprite->image.size.width); - EXPECT_EQ(36u, sprite->image.size.height); - EXPECT_EQ(1.5, sprite->pixelRatio); + EXPECT_EQ(36u, sprite->getImage().size.width); + EXPECT_EQ(36u, sprite->getImage().size.height); + EXPECT_EQ(1.5, sprite->getPixelRatio()); EXPECT_EQ(readImage("test/fixtures/annotations/result-spriteimagecreation1_5x-museum.png"), - sprite->image); + sprite->getImage()); // "hospital_icon":{"x":314,"y":518,"width":36,"height":36,"pixelRatio":2,"sdf":false} const auto sprite2 = createStyleImage(image_2x, 314, 518, 35, 35, 1.5, false); ASSERT_TRUE(sprite2.get()); EXPECT_EQ(float(35 / 1.5), sprite2->getWidth()); EXPECT_EQ(float(35 / 1.5), sprite2->getHeight()); - EXPECT_EQ(35u, sprite2->image.size.width); - EXPECT_EQ(35u, sprite2->image.size.height); - EXPECT_EQ(1.5, sprite2->pixelRatio); + EXPECT_EQ(35u, sprite2->getImage().size.width); + EXPECT_EQ(35u, sprite2->getImage().size.height); + EXPECT_EQ(1.5, sprite2->getPixelRatio()); EXPECT_EQ(readImage("test/fixtures/annotations/result-spriteimagecreation1_5x-hospital.png"), - sprite2->image); + sprite2->getImage()); } TEST(Sprite, SpriteParsing) { @@ -283,10 +283,10 @@ TEST(Sprite, SpriteParsing) { auto& sprite = images.find("generic-metro")->second; EXPECT_EQ(18, sprite->getWidth()); EXPECT_EQ(18, sprite->getHeight()); - 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); + 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()); } } diff --git a/test/src/mbgl/test/stub_style_observer.hpp b/test/src/mbgl/test/stub_style_observer.hpp index 7e22c68823..a3a48b04c0 100644 --- a/test/src/mbgl/test/stub_style_observer.hpp +++ b/test/src/mbgl/test/stub_style_observer.hpp @@ -18,8 +18,8 @@ public: if (glyphsError) glyphsError(fontStack, glyphRange, error); } - void onSpriteLoaded() override { - if (spriteLoaded) spriteLoaded(); + void onSpriteLoaded(Images&& images) override { + if (spriteLoaded) spriteLoaded(std::move(images)); } void onSpriteError(std::exception_ptr error) override { @@ -48,7 +48,7 @@ public: std::function glyphsLoaded; std::function glyphsError; - std::function spriteLoaded; + std::function spriteLoaded; std::function spriteError; std::function sourceLoaded; std::function sourceChanged; diff --git a/test/style/style_image.test.cpp b/test/style/style_image.test.cpp index 319120df83..34814bee88 100644 --- a/test/style/style_image.test.cpp +++ b/test/style/style_image.test.cpp @@ -36,17 +36,17 @@ TEST(StyleImage, ZeroRatio) { TEST(StyleImage, Retina) { style::Image image(PremultipliedImage({ 32, 24 }), 2.0); EXPECT_EQ(16, image.getWidth()); - EXPECT_EQ(32u, image.image.size.width); + EXPECT_EQ(32u, image.getImage().size.width); EXPECT_EQ(12, image.getHeight()); - EXPECT_EQ(24u, image.image.size.height); - EXPECT_EQ(2, image.pixelRatio); + EXPECT_EQ(24u, image.getImage().size.height); + EXPECT_EQ(2, image.getPixelRatio()); } TEST(StyleImage, FractionalRatio) { style::Image image(PremultipliedImage({ 20, 12 }), 1.5); EXPECT_EQ(float(20.0 / 1.5), image.getWidth()); - EXPECT_EQ(20u, image.image.size.width); + EXPECT_EQ(20u, image.getImage().size.width); EXPECT_EQ(float(12.0 / 1.5), image.getHeight()); - EXPECT_EQ(12u, image.image.size.height); - EXPECT_EQ(1.5, image.pixelRatio); + EXPECT_EQ(12u, image.getImage().size.height); + EXPECT_EQ(1.5, image.getPixelRatio()); } -- cgit v1.2.1