From 5dd98df50ba1210b1eef0d8d6655713a725f2995 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 21 Apr 2017 14:52:19 -0700 Subject: [all] Rationalize style::Image A style has a collection of images, just as it has collections of sources and layers. * Name things appropriately * Use std::unique_ptr --- src/mbgl/sprite/sprite_atlas.cpp | 119 +++++++++++++++++--------------------- src/mbgl/sprite/sprite_atlas.hpp | 19 +++--- src/mbgl/sprite/sprite_image.cpp | 23 -------- src/mbgl/sprite/sprite_parser.cpp | 20 +++---- src/mbgl/sprite/sprite_parser.hpp | 12 ++-- 5 files changed, 77 insertions(+), 116 deletions(-) delete mode 100644 src/mbgl/sprite/sprite_image.cpp (limited to 'src/mbgl/sprite') diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp index 3b774b627a..9d499411fe 100644 --- a/src/mbgl/sprite/sprite_atlas.cpp +++ b/src/mbgl/sprite/sprite_atlas.cpp @@ -38,20 +38,20 @@ struct SpriteAtlas::Loader { }; SpriteAtlasElement::SpriteAtlasElement(Rect rect_, - std::shared_ptr spriteImage, + const style::Image& image, Size size_, float pixelRatio) : pos(std::move(rect_)), - sdf(spriteImage->sdf), - relativePixelRatio(spriteImage->pixelRatio / pixelRatio), - width(spriteImage->getWidth()), - height(spriteImage->getHeight()) { + sdf(image.sdf), + relativePixelRatio(image.pixelRatio / pixelRatio), + width(image.getWidth()), + height(image.getHeight()) { const float padding = 1; - const float w = spriteImage->getWidth() * relativePixelRatio; - const float h = spriteImage->getHeight() * relativePixelRatio; + const float w = image.getWidth() * relativePixelRatio; + const float h = image.getHeight() * relativePixelRatio; - size = {{ float(spriteImage->getWidth()), spriteImage->getHeight() }}; + size = {{ float(image.getWidth()), image.getHeight() }}; tl = {{ float(pos.x + padding) / size_.width, float(pos.y + padding) / size_.height }}; br = {{ float(pos.x + padding + w) / size_.width, float(pos.y + padding + h) / size_.height }}; } @@ -116,9 +116,11 @@ void SpriteAtlas::emitSpriteLoadedIfComplete() { // TODO: delete the loader? } -void SpriteAtlas::onParsed(Sprites&& result) { +void SpriteAtlas::onParsed(Images&& result) { markAsLoaded(); - setSprites(result); + for (auto& pair : result) { + addImage(pair.first, std::move(pair.second)); + } observer->onSpriteLoaded(); for (auto requestor : requestors) { requestor->onIconsAvailable(this, buildIconMap()); @@ -138,79 +140,64 @@ void SpriteAtlas::dumpDebugLogs() const { Log::Info(Event::General, "SpriteAtlas::loaded: %d", loaded); } -void SpriteAtlas::setSprites(const Sprites& newSprites) { - for (const auto& pair : newSprites) { - _setSprite(pair.first, pair.second); - } -} - -void SpriteAtlas::setSprite(const std::string& name, std::shared_ptr sprite) { - _setSprite(name, sprite); -} - -void SpriteAtlas::removeSprite(const std::string& name) { +void SpriteAtlas::addImage(const std::string& id, std::unique_ptr image_) { icons.clear(); - auto it = entries.find(name); + + auto it = entries.find(id); if (it == entries.end()) { + entries.emplace(id, Entry { std::move(image_), {}, {} }); return; } 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; + } + + entry.image = std::move(image_); + if (entry.iconRect) { - bin.release(*entry.iconRect); + copy(entry, &Entry::iconRect); } if (entry.patternRect) { - bin.release(*entry.patternRect); + copy(entry, &Entry::patternRect); } - - entries.erase(it); } -void SpriteAtlas::_setSprite(const std::string& name, - const std::shared_ptr& sprite) { +void SpriteAtlas::removeImage(const std::string& id) { icons.clear(); - if (!sprite->image.valid()) { - Log::Warning(Event::Sprite, "invalid sprite image '%s'", name.c_str()); - return; - } - auto it = entries.find(name); + auto it = entries.find(id); if (it == entries.end()) { - entries.emplace(name, Entry { sprite, {}, {} }); return; } Entry& entry = it->second; - // There is already a sprite with that name in our store. - if (entry.spriteImage->image.size != sprite->image.size) { - Log::Warning(Event::Sprite, "Can't change sprite dimensions for '%s'", name.c_str()); - return; - } - - entry.spriteImage = sprite; - if (entry.iconRect) { - copy(entry, &Entry::iconRect); + bin.release(*entry.iconRect); } if (entry.patternRect) { - copy(entry, &Entry::patternRect); + bin.release(*entry.patternRect); } + + entries.erase(it); } -std::shared_ptr SpriteAtlas::getSprite(const std::string& name) { - const auto it = entries.find(name); +const style::Image* SpriteAtlas::getImage(const std::string& id) const { + const auto it = entries.find(id); if (it != entries.end()) { - return it->second.spriteImage; - } else { - if (!entries.empty()) { - Log::Info(Event::Sprite, "Can't find sprite named '%s'", name.c_str()); - } - return nullptr; + return it->second.image.get(); + } + if (!entries.empty()) { + Log::Info(Event::Sprite, "Can't find sprite named '%s'", id.c_str()); } + return nullptr; } void SpriteAtlas::getIcons(IconRequestor& requestor) { @@ -225,21 +212,21 @@ void SpriteAtlas::removeRequestor(IconRequestor& requestor) { requestors.erase(&requestor); } -optional SpriteAtlas::getIcon(const std::string& name) { - return getImage(name, &Entry::iconRect); +optional SpriteAtlas::getIcon(const std::string& id) { + return getImage(id, &Entry::iconRect); } -optional SpriteAtlas::getPattern(const std::string& name) { - return getImage(name, &Entry::patternRect); +optional SpriteAtlas::getPattern(const std::string& id) { + return getImage(id, &Entry::patternRect); } -optional SpriteAtlas::getImage(const std::string& name, +optional SpriteAtlas::getImage(const std::string& id, optional> Entry::*entryRect) { - auto it = entries.find(name); + auto it = entries.find(id); if (it == entries.end()) { if (!entries.empty()) { - Log::Info(Event::Sprite, "Can't find sprite named '%s'", name.c_str()); + Log::Info(Event::Sprite, "Can't find sprite named '%s'", id.c_str()); } return {}; } @@ -247,17 +234,17 @@ optional SpriteAtlas::getImage(const std::string& name, Entry& entry = it->second; if (entry.*entryRect) { - assert(entry.spriteImage.get()); + assert(entry.image.get()); return SpriteAtlasElement { *(entry.*entryRect), - entry.spriteImage, + *entry.image, size, pixelRatio }; } - const uint16_t pixelWidth = std::ceil(entry.spriteImage->image.size.width / pixelRatio); - const uint16_t pixelHeight = std::ceil(entry.spriteImage->image.size.height / pixelRatio); + const uint16_t pixelWidth = std::ceil(entry.image->image.size.width / pixelRatio); + const uint16_t pixelHeight = std::ceil(entry.image->image.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 +266,7 @@ optional SpriteAtlas::getImage(const std::string& name, return SpriteAtlasElement { rect, - entry.spriteImage, + *entry.image, size, pixelRatio }; @@ -292,7 +279,7 @@ void SpriteAtlas::copy(const Entry& entry, optional> Entry::*entr image.fill(0); } - const PremultipliedImage& src = entry.spriteImage->image; + const PremultipliedImage& src = entry.image->image; const Rect& rect = *(entry.*entryRect); const uint32_t padding = 1; @@ -316,7 +303,7 @@ void SpriteAtlas::copy(const Entry& entry, optional> Entry::*entr IconMap SpriteAtlas::buildIconMap() { if (icons.empty()) { - for (auto entry : entries) { + for (const auto& entry : entries) { icons.emplace(std::piecewise_construct, std::forward_as_tuple(entry.first), std::forward_as_tuple(*getIcon(entry.first))); diff --git a/src/mbgl/sprite/sprite_atlas.hpp b/src/mbgl/sprite/sprite_atlas.hpp index 21df25d67d..15bce919b1 100644 --- a/src/mbgl/sprite/sprite_atlas.hpp +++ b/src/mbgl/sprite/sprite_atlas.hpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include @@ -25,7 +25,7 @@ class Context; class SpriteAtlasElement { public: - SpriteAtlasElement(Rect, std::shared_ptr, Size size, float pixelRatio); + SpriteAtlasElement(Rect, const style::Image&, Size size, float pixelRatio); Rect pos; bool sdf; @@ -52,7 +52,7 @@ public: class SpriteAtlas : public util::noncopyable { public: - using Sprites = std::map>; + using Images = std::map>; SpriteAtlas(Size, float pixelRatio); ~SpriteAtlas(); @@ -71,10 +71,9 @@ public: void setObserver(SpriteAtlasObserver*); - void setSprite(const std::string&, std::shared_ptr); - void removeSprite(const std::string&); - - std::shared_ptr getSprite(const std::string& name); + const style::Image* getImage(const std::string&) const; + void addImage(const std::string&, std::unique_ptr); + void removeImage(const std::string&); void getIcons(IconRequestor& requestor); void removeRequestor(IconRequestor& requestor); @@ -93,18 +92,16 @@ public: float getPixelRatio() const { return pixelRatio; } // Only for use in tests. - void setSprites(const Sprites& sprites); const PremultipliedImage& getAtlasImage() const { return image; } private: - void _setSprite(const std::string&, const std::shared_ptr& = nullptr); void emitSpriteLoadedIfComplete(); // Invoked by SpriteAtlasWorker friend class SpriteAtlasWorker; - void onParsed(Sprites&& result); + void onParsed(Images&& result); void onError(std::exception_ptr); const Size size; @@ -118,7 +115,7 @@ private: SpriteAtlasObserver* observer = nullptr; struct Entry { - std::shared_ptr spriteImage; + 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_image.cpp b/src/mbgl/sprite/sprite_image.cpp deleted file mode 100644 index 1579d9d89e..0000000000 --- a/src/mbgl/sprite/sprite_image.cpp +++ /dev/null @@ -1,23 +0,0 @@ -#include - -#include - -#include - -namespace mbgl { - -SpriteImage::SpriteImage(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 mbgl diff --git a/src/mbgl/sprite/sprite_parser.cpp b/src/mbgl/sprite/sprite_parser.cpp index 96a883b0dd..c3ed20d03f 100644 --- a/src/mbgl/sprite/sprite_parser.cpp +++ b/src/mbgl/sprite/sprite_parser.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include @@ -13,7 +13,7 @@ namespace mbgl { -SpriteImagePtr createSpriteImage(const PremultipliedImage& image, +std::unique_ptr createStyleImage(const PremultipliedImage& image, const uint32_t srcX, const uint32_t srcY, const uint32_t width, @@ -37,7 +37,7 @@ SpriteImagePtr createSpriteImage(const PremultipliedImage& image, // Copy from the source image into our individual sprite image PremultipliedImage::copy(image, dstImage, { srcX, srcY }, { 0, 0 }, { width, height }); - return std::make_unique(std::move(dstImage), ratio, sdf); + return std::make_unique(std::move(dstImage), ratio, sdf); } namespace { @@ -84,8 +84,8 @@ bool getBoolean(const JSValue& value, const char* name, const bool def = false) } // namespace -Sprites parseSprite(const std::string& image, const std::string& json) { - const PremultipliedImage raster = decodeImage(image); +Images parseSprite(const std::string& encodedImage, const std::string& json) { + const PremultipliedImage raster = decodeImage(encodedImage); JSDocument doc; doc.Parse<0>(json.c_str()); @@ -96,7 +96,7 @@ Sprites parseSprite(const std::string& image, const std::string& json) { } else if (!doc.IsObject()) { throw std::runtime_error("Sprite JSON root must be an object"); } else { - Sprites sprites; + Images images; for (const auto& property : doc.GetObject()) { const std::string name = { property.name.GetString(), property.name.GetStringLength() }; const JSValue& value = property.value; @@ -109,13 +109,13 @@ Sprites parseSprite(const std::string& image, const std::string& json) { const double pixelRatio = getDouble(value, "pixelRatio", 1); const bool sdf = getBoolean(value, "sdf", false); - auto sprite = createSpriteImage(raster, x, y, width, height, pixelRatio, sdf); - if (sprite) { - sprites.emplace(name, sprite); + auto image = createStyleImage(raster, x, y, width, height, pixelRatio, sdf); + if (image) { + images.emplace(name, std::move(image)); } } } - return sprites; + return images; } } diff --git a/src/mbgl/sprite/sprite_parser.hpp b/src/mbgl/sprite/sprite_parser.hpp index 9e462f324e..5be8435ebb 100644 --- a/src/mbgl/sprite/sprite_parser.hpp +++ b/src/mbgl/sprite/sprite_parser.hpp @@ -11,12 +11,12 @@ namespace mbgl { -class SpriteImage; - -using SpriteImagePtr = std::shared_ptr; +namespace style { +class Image; +} // namespace style // Extracts an individual image from a spritesheet from the given location. -SpriteImagePtr createSpriteImage(const PremultipliedImage&, +std::unique_ptr createStyleImage(const PremultipliedImage&, uint32_t srcX, uint32_t srcY, uint32_t srcWidth, @@ -24,9 +24,9 @@ SpriteImagePtr createSpriteImage(const PremultipliedImage&, double ratio, bool sdf); -using Sprites = std::map; +using Images = std::map>; // Parses an image and an associated JSON file and returns the sprite objects. -Sprites parseSprite(const std::string& image, const std::string& json); +Images parseSprite(const std::string& image, const std::string& json); } // namespace mbgl -- cgit v1.2.1