[core] Immutable<Impl> for Image

include/mbgl/style/image.hpp

@@ -1,8 +1,7 @@
 #pragma once
 
 #include <mbgl/util/image.hpp>
-
-#include <memory>
+#include <mbgl/util/immutable.hpp>
 
 namespace mbgl {
 namespace style {
@@ -11,7 +10,7 @@ class Image {
 public:
     Image(PremultipliedImage&&, float pixelRatio, bool sdf = false);
 
-    PremultipliedImage& getImage() const;
+    const PremultipliedImage& getImage() const;
 
     // Pixel ratio of the sprite image.
     float getPixelRatio() const;
@@ -23,9 +22,7 @@ class Image {
     float getHeight() const;
 
     class Impl;
-
-private:
-    const std::shared_ptr<Impl> impl;
+    Immutable<Impl> impl;
 };
 
 } // namespace style

src/mbgl/annotation/annotation_manager.cpp

@@ -6,6 +6,7 @@
 #include <mbgl/annotation/fill_annotation_impl.hpp>
 #include <mbgl/sprite/sprite_image_collection.hpp>
 #include <mbgl/style/style.hpp>
+#include <mbgl/style/image_impl.hpp>
 #include <mbgl/style/layers/symbol_layer.hpp>
 #include <mbgl/style/layers/symbol_layer_impl.hpp>
 #include <mbgl/storage/file_source.hpp>
@@ -192,7 +193,7 @@ void AnnotationManager::removeTile(AnnotationTile& tile) {
 
 void AnnotationManager::addImage(const std::string& id, std::unique_ptr<style::Image> image) {
     addSpriteImage(spriteImages, id, std::move(image), [&](style::Image& added) {
-        spriteAtlas.addImage(id, std::make_unique<style::Image>(added));
+        spriteAtlas.addImage(id, added.impl);
     });
 }
 
@@ -203,8 +204,8 @@ void AnnotationManager::removeImage(const std::string& id) {
 }
 
 double AnnotationManager::getTopOffsetPixelsForImage(const std::string& id) {
-    const style::Image* image = spriteAtlas.getImage(id);
-    return image ? -(image->getImage().size.height / image->getPixelRatio()) / 2 : 0;
+    const style::Image::Impl* impl = spriteAtlas.getImage(id);
+    return impl ? -(impl->image.size.height / impl->pixelRatio) / 2 : 0;
 }
 
 } // namespace mbgl

src/mbgl/sprite/sprite_atlas.cpp

@@ -1,4 +1,5 @@
 #include <mbgl/sprite/sprite_atlas.hpp>
+#include <mbgl/style/image_impl.hpp>
 #include <mbgl/gl/context.hpp>
 #include <mbgl/util/logging.hpp>
 #include <mbgl/util/platform.hpp>
@@ -14,11 +15,11 @@
 namespace mbgl {
 
 SpriteAtlasElement::SpriteAtlasElement(Rect<uint16_t> rect_,
-                                       const style::Image& image,
+                                       const style::Image::Impl& image,
                                        Size size_, float pixelRatio)
     : pos(std::move(rect_)),
-      sdf(image.isSdf()),
-      relativePixelRatio(image.getPixelRatio() / pixelRatio),
+      sdf(image.sdf),
+      relativePixelRatio(image.pixelRatio / pixelRatio),
       width(image.getWidth()),
       height(image.getHeight()) {
 
@@ -45,7 +46,7 @@ void SpriteAtlas::onSpriteLoaded(Images&& result) {
     markAsLoaded();
 
     for (auto& pair : result) {
-        addImage(pair.first, std::move(pair.second));
+        addImage(pair.first, pair.second->impl);
     }
 
     for (auto requestor : requestors) {
@@ -54,7 +55,7 @@ void SpriteAtlas::onSpriteLoaded(Images&& result) {
     requestors.clear();
 }
 
-void SpriteAtlas::addImage(const std::string& id, std::unique_ptr<style::Image> image_) {
+void SpriteAtlas::addImage(const std::string& id, Immutable<style::Image::Impl> image_) {
     icons.clear();
 
     auto it = entries.find(id);
@@ -66,7 +67,7 @@ void SpriteAtlas::addImage(const std::string& id, std::unique_ptr<style::Image>
     Entry& entry = it->second;
 
     // There is already a sprite with that name in our store.
-    assert(entry.image->getImage().size == image_->getImage().size);
+    assert(entry.image->image.size == image_->image.size);
 
     entry.image = std::move(image_);
 
@@ -98,7 +99,7 @@ void SpriteAtlas::removeImage(const std::string& id) {
     entries.erase(it);
 }
 
-const style::Image* SpriteAtlas::getImage(const std::string& id) const {
+const style::Image::Impl* SpriteAtlas::getImage(const std::string& id) const {
     const auto it = entries.find(id);
     if (it != entries.end()) {
         return it->second.image.get();
@@ -152,8 +153,8 @@ optional<SpriteAtlasElement> SpriteAtlas::getImage(const std::string& id,
         };
     }
 
-    const uint16_t pixelWidth = std::ceil(entry.image->getImage().size.width / pixelRatio);
-    const uint16_t pixelHeight = std::ceil(entry.image->getImage().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
@@ -188,7 +189,7 @@ void SpriteAtlas::copy(const Entry& entry, optional<Rect<uint16_t>> Entry::*entr
         image.fill(0);
     }
 
-    const PremultipliedImage& src = entry.image->getImage();
+    const PremultipliedImage& src = entry.image->image;
     const Rect<uint16_t>& rect = *(entry.*entryRect);
 
     const uint32_t padding = 1;

src/mbgl/sprite/sprite_atlas.hpp

@@ -20,7 +20,7 @@ class Context;
 
 class SpriteAtlasElement {
 public:
-    SpriteAtlasElement(Rect<uint16_t>, const style::Image&, Size size, float pixelRatio);
+    SpriteAtlasElement(Rect<uint16_t>, const style::Image::Impl&, Size size, float pixelRatio);
 
     Rect<uint16_t> pos;
     bool sdf;
@@ -60,8 +60,8 @@ class SpriteAtlas : public util::noncopyable {
 
     void dumpDebugLogs() const;
 
-    const style::Image* getImage(const std::string&) const;
-    void addImage(const std::string&, std::unique_ptr<style::Image>);
+    const style::Image::Impl* getImage(const std::string&) const;
+    void addImage(const std::string&, Immutable<style::Image::Impl>);
     void removeImage(const std::string&);
 
     void getIcons(IconRequestor& requestor);
@@ -91,7 +91,7 @@ class SpriteAtlas : public util::noncopyable {
     bool loaded = false;
 
     struct Entry {
-        std::unique_ptr<const style::Image> image;
+        Immutable<style::Image::Impl> 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

src/mbgl/style/image.cpp

@@ -8,11 +8,10 @@ namespace style {
 Image::Image(PremultipliedImage &&image,
              const float pixelRatio,
              bool sdf)
-        : impl(std::make_shared<Impl>(std::move(image), pixelRatio, sdf)) {
+    : impl(makeMutable<Impl>(std::move(image), pixelRatio, sdf)) {
 }
 
-PremultipliedImage& Image::getImage() const {
-    assert(impl);
+const PremultipliedImage& Image::getImage() const {
     return impl->image;
 }
 

src/mbgl/style/style.cpp

@@ -544,7 +544,7 @@ bool Style::isLoaded() const {
 
 void Style::addImage(const std::string& id, std::unique_ptr<style::Image> image) {
     addSpriteImage(spriteImages, id, std::move(image), [&](style::Image& added) {
-        spriteAtlas->addImage(id, std::make_unique<style::Image>(added));
+        spriteAtlas->addImage(id, added.impl);
         observer->onUpdate(Update::Repaint);
     });
 }
@@ -557,7 +557,8 @@ void Style::removeImage(const std::string& id) {
 }
 
 const style::Image* Style::getImage(const std::string& id) const {
-    return spriteAtlas->getImage(id);
+    auto it = spriteImages.find(id);
+    return it == spriteImages.end() ? nullptr : it->second.get();
 }
 
 RenderData Style::getRenderData(MapDebugOptions debugOptions, float angle) const {

test/sprite/sprite_atlas.test.cpp

@@ -5,6 +5,7 @@
 
 #include <mbgl/sprite/sprite_atlas.hpp>
 #include <mbgl/sprite/sprite_parser.hpp>
+#include <mbgl/style/image_impl.hpp>
 #include <mbgl/util/io.hpp>
 #include <mbgl/util/image.hpp>
 #include <mbgl/util/run_loop.hpp>
@@ -22,7 +23,7 @@ TEST(SpriteAtlas, Basic) {
     auto images = parseSprite(util::read_file("test/fixtures/annotations/emerald.png"),
                               util::read_file("test/fixtures/annotations/emerald.json"));
     for (auto& pair : images) {
-        atlas.addImage(pair.first, std::move(pair.second));
+        atlas.addImage(pair.first, pair.second->impl);
     }
 
     EXPECT_EQ(1.0f, atlas.getPixelRatio());
@@ -79,7 +80,7 @@ TEST(SpriteAtlas, Size) {
     auto images = parseSprite(util::read_file("test/fixtures/annotations/emerald.png"),
                               util::read_file("test/fixtures/annotations/emerald.json"));
     for (auto& pair : images) {
-        atlas.addImage(pair.first, std::move(pair.second));
+        atlas.addImage(pair.first, pair.second->impl);
     }
 
     EXPECT_DOUBLE_EQ(1.4f, atlas.getPixelRatio());
@@ -112,7 +113,7 @@ TEST(SpriteAtlas, Updates) {
     EXPECT_EQ(32u, atlas.getSize().width);
     EXPECT_EQ(32u, atlas.getSize().height);
 
-    atlas.addImage("one", std::make_unique<style::Image>(PremultipliedImage({ 16, 12 }), 1));
+    atlas.addImage("one", makeMutable<style::Image::Impl>(PremultipliedImage({ 16, 12 }), 1));
     auto one = *atlas.getIcon("one");
     float imagePixelRatio = one.relativePixelRatio * atlas.getPixelRatio();
     EXPECT_EQ(0, one.pos.x);
@@ -136,7 +137,7 @@ TEST(SpriteAtlas, Updates) {
     for (size_t i = 0; i < image2.bytes(); i++) {
         image2.data.get()[i] = 255;
     }
-    atlas.addImage("one", std::make_unique<style::Image>(std::move(image2), 1));
+    atlas.addImage("one", makeMutable<style::Image::Impl>(std::move(image2), 1));
 
     test::checkImage("test/fixtures/sprite_atlas/updates_after", atlas.getAtlasImage());
 }
@@ -145,9 +146,9 @@ TEST(SpriteAtlas, AddRemove) {
     FixtureLog log;
     SpriteAtlas atlas({ 32, 32 }, 1);
 
-    atlas.addImage("one", std::make_unique<style::Image>(PremultipliedImage({ 16, 16 }), 2));
-    atlas.addImage("two", std::make_unique<style::Image>(PremultipliedImage({ 16, 16 }), 2));
-    atlas.addImage("three", std::make_unique<style::Image>(PremultipliedImage({ 16, 16 }), 2));
+    atlas.addImage("one", makeMutable<style::Image::Impl>(PremultipliedImage({ 16, 16 }), 2));
+    atlas.addImage("two", makeMutable<style::Image::Impl>(PremultipliedImage({ 16, 16 }), 2));
+    atlas.addImage("three", makeMutable<style::Image::Impl>(PremultipliedImage({ 16, 16 }), 2));
 
     atlas.removeImage("one");
     atlas.removeImage("two");
@@ -175,12 +176,12 @@ TEST(SpriteAtlas, RemoveReleasesBinPackRect) {
 
     SpriteAtlas atlas({ 36, 36 }, 1);
 
-    atlas.addImage("big", std::make_unique<style::Image>(PremultipliedImage({ 32, 32 }), 1));
+    atlas.addImage("big", makeMutable<style::Image::Impl>(PremultipliedImage({ 32, 32 }), 1));
     EXPECT_TRUE(atlas.getIcon("big"));
 
     atlas.removeImage("big");
 
-    atlas.addImage("big", std::make_unique<style::Image>(PremultipliedImage({ 32, 32 }), 1));
+    atlas.addImage("big", makeMutable<style::Image::Impl>(PremultipliedImage({ 32, 32 }), 1));
     EXPECT_TRUE(atlas.getIcon("big"));
     EXPECT_TRUE(log.empty());
 }

test/text/quads.test.cpp

@@ -1,5 +1,5 @@
 #include <mbgl/geometry/anchor.hpp>
-#include <mbgl/sprite/sprite_atlas.hpp>
+#include <mbgl/style/image_impl.hpp>
 #include <mbgl/test/util.hpp>
 #include <mbgl/text/quads.hpp>
 #include <mbgl/text/shaping.hpp>
@@ -14,7 +14,7 @@ TEST(getIconQuads, normal) {
     Anchor anchor(2.0, 3.0, 0.0, 0.5f, 0);
     SpriteAtlasElement image = {
         Rect<uint16_t>( 0, 0, 15, 11 ),
-        style::Image(PremultipliedImage({1,1}), 1.0),
+        style::Image::Impl(PremultipliedImage({1,1}), 1.0),
         { 0, 0 },
         1.0f
     };
@@ -47,7 +47,7 @@ TEST(getIconQuads, style) {
     Anchor anchor(0.0, 0.0, 0.0, 0.5f, 0);
     SpriteAtlasElement image = {
         Rect<uint16_t>( 0, 0, 20, 20 ),
-        style::Image(PremultipliedImage({1,1}), 1.0),
+        style::Image::Impl(PremultipliedImage({1,1}), 1.0),
         { 0, 0 },
         1.0f
     };