[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

9 files changed, 107 insertions, 123 deletions

diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp
index f2b2b1837f..049c900b95 100644
--- a/src/mbgl/annotation/annotation_manager.cpp
+++ b/src/mbgl/annotation/annotation_manager.cpp
@@ -208,17 +208,17 @@ void AnnotationManager::removeTile(AnnotationTile& tile) {
     tiles.erase(&tile);
 }
 
-void AnnotationManager::addIcon(const std::string& name, std::shared_ptr<const SpriteImage> sprite) {
-    spriteAtlas.setSprite(name, sprite);
+void AnnotationManager::addImage(const std::string& id, std::unique_ptr<style::Image> image) {
+    spriteAtlas.addImage(id, std::move(image));
 }
 
-void AnnotationManager::removeIcon(const std::string& name) {
-    spriteAtlas.removeSprite(name);
+void AnnotationManager::removeImage(const std::string& id) {
+    spriteAtlas.removeImage(id);
 }
 
-double AnnotationManager::getTopOffsetPixelsForIcon(const std::string& name) {
-    auto sprite = spriteAtlas.getSprite(name);
-    return sprite ? -(sprite->image.size.height / sprite->pixelRatio) / 2 : 0;
+double AnnotationManager::getTopOffsetPixelsForImage(const std::string& id) {
+    const style::Image* image = spriteAtlas.getImage(id);
+    return image ? -(image->image.size.height / image->pixelRatio) / 2 : 0;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp
index d254a360e0..5e2994c44e 100644
--- a/src/mbgl/annotation/annotation_manager.hpp
+++ b/src/mbgl/annotation/annotation_manager.hpp
@@ -20,6 +20,7 @@ class ShapeAnnotationImpl;
 
 namespace style {
 class Style;
+class Image;
 } // namespace style
 
 class AnnotationManager : private util::noncopyable {
@@ -31,9 +32,9 @@ public:
     Update updateAnnotation(const AnnotationID&, const Annotation&, const uint8_t maxZoom);
     void removeAnnotation(const AnnotationID&);
 
-    void addIcon(const std::string& name, std::shared_ptr<const SpriteImage>);
-    void removeIcon(const std::string& name);
-    double getTopOffsetPixelsForIcon(const std::string& name);
+    void addImage(const std::string&, std::unique_ptr<style::Image>);
+    void removeImage(const std::string&);
+    double getTopOffsetPixelsForImage(const std::string&);
     SpriteAtlas& getSpriteAtlas() { return spriteAtlas; }
 
     void updateStyle(style::Style&);
diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index 40be7c374f..dd33d0b170 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -812,16 +812,16 @@ LatLng Map::latLngForPixel(const ScreenCoordinate& pixel) const {
 
 #pragma mark - Annotations
 
-void Map::addAnnotationIcon(const std::string& name, std::shared_ptr<const SpriteImage> sprite) {
-    impl->annotationManager->addIcon(name, sprite);
+void Map::addAnnotationImage(const std::string& id, std::unique_ptr<style::Image> image) {
+    impl->annotationManager->addImage(id, std::move(image));
 }
 
-void Map::removeAnnotationIcon(const std::string& name) {
-    impl->annotationManager->removeIcon(name);
+void Map::removeAnnotationImage(const std::string& id) {
+    impl->annotationManager->removeImage(id);
 }
 
-double Map::getTopOffsetPixelsForAnnotationIcon(const std::string& name) {
-    return impl->annotationManager->getTopOffsetPixelsForIcon(name);
+double Map::getTopOffsetPixelsForAnnotationImage(const std::string& id) {
+    return impl->annotationManager->getTopOffsetPixelsForImage(id);
 }
 
 AnnotationID Map::addAnnotation(const Annotation& annotation) {
@@ -951,29 +951,29 @@ std::unique_ptr<Layer> Map::removeLayer(const std::string& id) {
     return removedLayer;
 }
 
-void Map::addImage(const std::string& name, std::unique_ptr<const SpriteImage> image) {
+void Map::addImage(const std::string& id, std::unique_ptr<style::Image> image) {
     if (!impl->style) {
         return;
     }
 
     impl->styleMutated = true;
-    impl->style->spriteAtlas->setSprite(name, std::move(image));
+    impl->style->spriteAtlas->addImage(id, std::move(image));
     impl->onUpdate(Update::Repaint);
 }
 
-void Map::removeImage(const std::string& name) {
+void Map::removeImage(const std::string& id) {
     if (!impl->style) {
         return;
     }
 
     impl->styleMutated = true;
-    impl->style->spriteAtlas->removeSprite(name);
+    impl->style->spriteAtlas->removeImage(id);
     impl->onUpdate(Update::Repaint);
 }
 
-const SpriteImage* Map::getImage(const std::string& name) {
+const style::Image* Map::getImage(const std::string& id) {
     if (impl->style) {
-        return impl->style->spriteAtlas->getSprite(name).get();
+        return impl->style->spriteAtlas->getImage(id);
     }
     return nullptr;
 }
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<uint16_t> rect_,
-                                       std::shared_ptr<const SpriteImage> 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<const SpriteImage> sprite) {
-    _setSprite(name, sprite);
-}
-
-void SpriteAtlas::removeSprite(const std::string& name) {
+void SpriteAtlas::addImage(const std::string& id, std::unique_ptr<style::Image> 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<const SpriteImage>& 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<const SpriteImage> 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<SpriteAtlasElement> SpriteAtlas::getIcon(const std::string& name) {
-    return getImage(name, &Entry::iconRect);
+optional<SpriteAtlasElement> SpriteAtlas::getIcon(const std::string& id) {
+    return getImage(id, &Entry::iconRect);
 }
 
-optional<SpriteAtlasElement> SpriteAtlas::getPattern(const std::string& name) {
-    return getImage(name, &Entry::patternRect);
+optional<SpriteAtlasElement> SpriteAtlas::getPattern(const std::string& id) {
+    return getImage(id, &Entry::patternRect);
 }
 
-optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::string& name,
+optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::string& id,
                                                    optional<Rect<uint16_t>> 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<SpriteAtlasElement> 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<SpriteAtlasElement> 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<Rect<uint16_t>> Entry::*entr
         image.fill(0);
     }
 
-    const PremultipliedImage& src = entry.spriteImage->image;
+    const PremultipliedImage& src = entry.image->image;
     const Rect<uint16_t>& rect = *(entry.*entryRect);
 
     const uint32_t padding = 1;
@@ -316,7 +303,7 @@ void SpriteAtlas::copy(const Entry& entry, optional<Rect<uint16_t>> 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 <mbgl/gl/texture.hpp>
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/util/optional.hpp>
-#include <mbgl/sprite/sprite_image.hpp>
+#include <mbgl/style/image.hpp>
 
 #include <string>
 #include <map>
@@ -25,7 +25,7 @@ class Context;
 
 class SpriteAtlasElement {
 public:
-    SpriteAtlasElement(Rect<uint16_t>, std::shared_ptr<const SpriteImage>, Size size, float pixelRatio);
+    SpriteAtlasElement(Rect<uint16_t>, const style::Image&, Size size, float pixelRatio);
 
     Rect<uint16_t> pos;
     bool sdf;
@@ -52,7 +52,7 @@ public:
 
 class SpriteAtlas : public util::noncopyable {
 public:
-    using Sprites = std::map<std::string, std::shared_ptr<const SpriteImage>>;
+    using Images = std::map<std::string, std::unique_ptr<style::Image>>;
 
     SpriteAtlas(Size, float pixelRatio);
     ~SpriteAtlas();
@@ -71,10 +71,9 @@ public:
 
     void setObserver(SpriteAtlasObserver*);
 
-    void setSprite(const std::string&, std::shared_ptr<const SpriteImage>);
-    void removeSprite(const std::string&);
-    
-    std::shared_ptr<const SpriteImage> getSprite(const std::string& name);
+    const style::Image* getImage(const std::string&) const;
+    void addImage(const std::string&, std::unique_ptr<style::Image>);
+    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<const SpriteImage>& = 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<const SpriteImage> spriteImage;
+        std::unique_ptr<style::Image> 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_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 <mbgl/sprite/sprite_parser.hpp>
-#include <mbgl/sprite/sprite_image.hpp>
+#include <mbgl/style/image.hpp>
 
 #include <mbgl/util/logging.hpp>
 
@@ -13,7 +13,7 @@
 
 namespace mbgl {
 
-SpriteImagePtr createSpriteImage(const PremultipliedImage& image,
+std::unique_ptr<style::Image> 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<const SpriteImage>(std::move(dstImage), ratio, sdf);
+    return std::make_unique<style::Image>(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<const SpriteImage>;
+namespace style {
+class Image;
+} // namespace style
 
 // Extracts an individual image from a spritesheet from the given location.
-SpriteImagePtr createSpriteImage(const PremultipliedImage&,
+std::unique_ptr<style::Image> 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<std::string, SpriteImagePtr>;
+using Images = std::map<std::string, std::unique_ptr<style::Image>>;
 
 // 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
diff --git a/src/mbgl/sprite/sprite_image.cpp b/src/mbgl/style/image.cpp
index 1579d9d89e..4c0c6a859b 100644
--- a/src/mbgl/sprite/sprite_image.cpp
+++ b/src/mbgl/style/image.cpp
@@ -1,14 +1,12 @@
-#include <mbgl/sprite/sprite_image.hpp>
-
+#include <mbgl/style/image.hpp>
 #include <mbgl/util/exception.hpp>
 
-#include <cmath>
-
 namespace mbgl {
+namespace style {
 
-SpriteImage::SpriteImage(PremultipliedImage&& image_,
-                         const float pixelRatio_,
-                         bool sdf_)
+Image::Image(PremultipliedImage&& image_,
+             const float pixelRatio_,
+             bool sdf_)
     : image(std::move(image_)),
       pixelRatio(pixelRatio_),
       sdf(sdf_) {
@@ -20,4 +18,5 @@ SpriteImage::SpriteImage(PremultipliedImage&& image_,
     }
 }
 
+} // namespace style
 } // namespace mbgl
diff --git a/src/mbgl/text/shaping.hpp b/src/mbgl/text/shaping.hpp
index 5e932e6df7..b7eee5a5db 100644
--- a/src/mbgl/text/shaping.hpp
+++ b/src/mbgl/text/shaping.hpp
@@ -2,7 +2,7 @@
 
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/sprite/sprite_atlas.hpp>
-#include <mbgl/sprite/sprite_image.hpp>
+#include <mbgl/style/image.hpp>
 #include <mbgl/util/optional.hpp>
 
 namespace mbgl {