From c2f83049bbcb2b868d1aa002c9000a96c69e1b6c Mon Sep 17 00:00:00 2001
From: Adam Hunter <adam@mapbox.com>
Date: Mon, 16 Nov 2015 16:26:02 -0800
Subject: [PATCH] [core] Use a separate atlas and store for annotation sprites

Fixes #1488
---
 src/mbgl/annotation/annotation_manager.cpp | 17 ++++++++++++++++-
 src/mbgl/annotation/annotation_manager.hpp | 11 ++++++++++-
 src/mbgl/layer/symbol_layer.cpp            |  3 ++-
 src/mbgl/layer/symbol_layer.hpp            |  4 ++++
 src/mbgl/map/map_context.cpp               | 19 ++++---------------
 src/mbgl/map/map_data.hpp                  |  1 +
 src/mbgl/map/tile_worker.cpp               |  4 +++-
 src/mbgl/renderer/painter.cpp              |  3 ++-
 src/mbgl/renderer/painter.hpp              |  3 ++-
 src/mbgl/renderer/painter_symbol.cpp       |  5 +++--
 src/mbgl/sprite/sprite_atlas.cpp           | 11 +++++++++--
 src/mbgl/sprite/sprite_atlas.hpp           |  4 +++-
 src/mbgl/style/style.cpp                   |  7 +++++++
 test/sprite/sprite_atlas.cpp               |  4 +++-
 14 files changed, 69 insertions(+), 27 deletions(-)

diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp
index 5b1138a14af..83cf34ac66a 100644
--- a/src/mbgl/annotation/annotation_manager.cpp
+++ b/src/mbgl/annotation/annotation_manager.cpp
@@ -10,7 +10,11 @@ namespace mbgl {
 const std::string AnnotationManager::SourceID = "com.mapbox.annotations";
 const std::string AnnotationManager::PointLayerID = "com.mapbox.annotations.points";
 
-AnnotationManager::AnnotationManager() = default;
+AnnotationManager::AnnotationManager(float pixelRatio)
+    : spriteStore(pixelRatio),
+      spriteAtlas(512, 512, pixelRatio, spriteStore) {
+}
+
 AnnotationManager::~AnnotationManager() = default;
 
 AnnotationIDs
@@ -122,6 +126,7 @@ void AnnotationManager::updateStyle(Style& style) {
         layer->sourceLayer = PointLayerID;
         layer->layout.icon.image = std::string("{sprite}");
         layer->layout.icon.allowOverlap = true;
+        layer->spriteAtlas = &spriteAtlas;
 
         style.addLayer(std::move(layer));
     }
@@ -152,4 +157,14 @@ void AnnotationManager::removeTileMonitor(AnnotationTileMonitor& monitor) {
     monitors.erase(&monitor);
 }
 
+void AnnotationManager::setSprite(const std::string& name, std::shared_ptr<const SpriteImage> sprite) {
+    spriteStore.setSprite(name, sprite);
+    spriteAtlas.updateDirty();
+}
+
+double AnnotationManager::getTopOffsetPixelsForAnnotationSymbol(const std::string& name) {
+    auto sprite = spriteStore.getSprite(name);
+    return sprite ? -sprite->height / 2 : 0;
+}
+
 }
diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp
index f1b41c9ccce..9fbbcc57c83 100644
--- a/src/mbgl/annotation/annotation_manager.hpp
+++ b/src/mbgl/annotation/annotation_manager.hpp
@@ -4,6 +4,8 @@
 #include <mbgl/annotation/annotation.hpp>
 #include <mbgl/annotation/point_annotation_impl.hpp>
 #include <mbgl/annotation/shape_annotation_impl.hpp>
+#include <mbgl/sprite/sprite_store.hpp>
+#include <mbgl/sprite/sprite_atlas.hpp>
 #include <mbgl/util/geo.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
@@ -21,7 +23,7 @@ class Style;
 
 class AnnotationManager : private util::noncopyable {
 public:
-    AnnotationManager();
+    AnnotationManager(float pixelRatio);
     ~AnnotationManager();
 
     AnnotationIDs addPointAnnotations(const std::vector<PointAnnotation>&, const uint8_t maxZoom);
@@ -31,6 +33,10 @@ class AnnotationManager : private util::noncopyable {
     AnnotationIDs getPointAnnotationsInBounds(const LatLngBounds&) const;
     LatLngBounds getBoundsForAnnotations(const AnnotationIDs&) const;
 
+    void setSprite(const std::string& name, std::shared_ptr<const SpriteImage>);
+    double getTopOffsetPixelsForAnnotationSymbol(const std::string& name);
+    SpriteAtlas& getSpriteAtlas() { return spriteAtlas; }
+
     void updateStyle(Style&);
 
     void addTileMonitor(AnnotationTileMonitor&);
@@ -48,6 +54,9 @@ class AnnotationManager : private util::noncopyable {
     ShapeAnnotationImpl::Map shapeAnnotations;
     std::vector<std::string> obsoleteShapeAnnotationLayers;
     std::set<AnnotationTileMonitor*> monitors;
+
+    SpriteStore spriteStore;
+    SpriteAtlas spriteAtlas;
 };
 
 }
diff --git a/src/mbgl/layer/symbol_layer.cpp b/src/mbgl/layer/symbol_layer.cpp
index 8e43d3454e2..f9549996147 100644
--- a/src/mbgl/layer/symbol_layer.cpp
+++ b/src/mbgl/layer/symbol_layer.cpp
@@ -10,6 +10,7 @@ std::unique_ptr<StyleLayer> SymbolLayer::clone() const {
     result->copy(*this);
     result->layout = layout;
     result->paint = paint;
+    result->spriteAtlas = spriteAtlas;
     return std::move(result);
 }
 
@@ -178,7 +179,7 @@ std::unique_ptr<Bucket> SymbolLayer::createBucket(StyleBucketParameters& paramet
     // needed by this tile.
     if (!parameters.partialParse) {
         bucket->addFeatures(parameters.tileUID,
-                            parameters.spriteAtlas,
+                            *spriteAtlas,
                             parameters.glyphAtlas,
                             parameters.glyphStore,
                             parameters.collisionTile);
diff --git a/src/mbgl/layer/symbol_layer.hpp b/src/mbgl/layer/symbol_layer.hpp
index 9e4555350a2..1a9e4587bb5 100644
--- a/src/mbgl/layer/symbol_layer.hpp
+++ b/src/mbgl/layer/symbol_layer.hpp
@@ -7,6 +7,8 @@
 
 namespace mbgl {
 
+class SpriteAtlas;
+
 class SymbolLayoutProperties {
 public:
     LayoutProperty<PlacementType> placement = PlacementType::Point;
@@ -94,6 +96,8 @@ class SymbolLayer : public StyleLayer {
 
     SymbolLayoutProperties layout;
     SymbolPaintProperties paint;
+
+    SpriteAtlas* spriteAtlas;
 };
 
 }
diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index 11356a25cc7..560c3ebdd8a 100644
--- a/src/mbgl/map/map_context.cpp
+++ b/src/mbgl/map/map_context.cpp
@@ -244,7 +244,7 @@ bool MapContext::renderSync(const TransformState& state, const FrameData& frame)
     glObjectStore.performCleanup();
 
     if (!painter) painter = std::make_unique<Painter>(data, transformState);
-    painter->render(*style, frame);
+    painter->render(*style, frame, data.getAnnotationManager()->getSpriteAtlas());
 
     if (data.mode == MapMode::Still) {
         callback(nullptr, std::move(view.readStillImage()));
@@ -270,12 +270,7 @@ bool MapContext::isLoaded() const {
 
 double MapContext::getTopOffsetPixelsForAnnotationSymbol(const std::string& symbol) {
     assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
-    auto sprite = style->spriteStore->getSprite(symbol);
-    if (sprite) {
-        return -sprite->height / 2;
-    } else {
-        return 0;
-    }
+    return data.getAnnotationManager()->getTopOffsetPixelsForAnnotationSymbol(symbol);
 }
 
 void MapContext::setSourceTileCacheSize(size_t size) {
@@ -300,14 +295,8 @@ void MapContext::onLowMemory() {
 }
 
 void MapContext::setSprite(const std::string& name, std::shared_ptr<const SpriteImage> sprite) {
-    if (!style) {
-        Log::Info(Event::Sprite, "Ignoring sprite without stylesheet");
-        return;
-    }
-
-    style->spriteStore->setSprite(name, sprite);
-
-    style->spriteAtlas->updateDirty();
+    assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
+    data.getAnnotationManager()->setSprite(name, sprite);
 }
 
 void MapContext::onTileDataChanged() {
diff --git a/src/mbgl/map/map_data.hpp b/src/mbgl/map/map_data.hpp
index c1898fc37f7..2afc1cef728 100644
--- a/src/mbgl/map/map_data.hpp
+++ b/src/mbgl/map/map_data.hpp
@@ -24,6 +24,7 @@ class MapData {
         : mode(mode_)
         , contextMode(contextMode_)
         , pixelRatio(pixelRatio_)
+        , annotationManager(pixelRatio)
         , animationTime(Duration::zero())
         , defaultFadeDuration(mode_ == MapMode::Continuous ? std::chrono::milliseconds(300) : Duration::zero())
         , defaultTransitionDuration(Duration::zero())
diff --git a/src/mbgl/map/tile_worker.cpp b/src/mbgl/map/tile_worker.cpp
index 9dc0698db7c..9686d579ae8 100644
--- a/src/mbgl/map/tile_worker.cpp
+++ b/src/mbgl/map/tile_worker.cpp
@@ -64,7 +64,9 @@ TileParseResult TileWorker::parsePendingLayers() {
         if (layer.type == StyleLayerType::Symbol) {
             auto symbolBucket = dynamic_cast<SymbolBucket*>(bucket.get());
             if (!symbolBucket->needsDependencies(*style.glyphStore, *style.spriteStore)) {
-                symbolBucket->addFeatures(reinterpret_cast<uintptr_t>(this), *style.spriteAtlas,
+                const SymbolLayer* symbolLayer = dynamic_cast<const SymbolLayer*>(&layer);
+                symbolBucket->addFeatures(reinterpret_cast<uintptr_t>(this),
+                                          *symbolLayer->spriteAtlas,
                                           *style.glyphAtlas, *style.glyphStore, *collisionTile);
                 insertBucket(layer.bucketName(), std::move(bucket));
                 pending.erase(it++);
diff --git a/src/mbgl/renderer/painter.cpp b/src/mbgl/renderer/painter.cpp
index a3902fece06..9c64fc9f905 100644
--- a/src/mbgl/renderer/painter.cpp
+++ b/src/mbgl/renderer/painter.cpp
@@ -128,7 +128,7 @@ void Painter::prepareTile(const Tile& tile) {
     config.stencilFunc = { GL_EQUAL, ref, mask };
 }
 
-void Painter::render(const Style& style, const FrameData& frame_) {
+void Painter::render(const Style& style, const FrameData& frame_, SpriteAtlas& annotationSpriteAtlas) {
     frame = frame_;
 
     glyphAtlas = style.glyphAtlas.get();
@@ -158,6 +158,7 @@ void Painter::render(const Style& style, const FrameData& frame_) {
         spriteAtlas->upload();
         lineAtlas->upload();
         glyphAtlas->upload();
+        annotationSpriteAtlas.upload();
 
         for (const auto& item : order) {
             if (item.bucket && item.bucket->needsUpload()) {
diff --git a/src/mbgl/renderer/painter.hpp b/src/mbgl/renderer/painter.hpp
index 5de1031d23c..0995ee4e440 100644
--- a/src/mbgl/renderer/painter.hpp
+++ b/src/mbgl/renderer/painter.hpp
@@ -93,7 +93,8 @@ class Painter : private util::noncopyable {
     void changeMatrix();
 
     void render(const Style& style,
-                const FrameData& frame);
+                const FrameData& frame,
+                SpriteAtlas& annotationSpriteAtlas);
 
     // Renders debug information for a tile.
     void renderTileDebug(const Tile& tile);
diff --git a/src/mbgl/renderer/painter_symbol.cpp b/src/mbgl/renderer/painter_symbol.cpp
index f46a9c84543..4a00d941db4 100644
--- a/src/mbgl/renderer/painter_symbol.cpp
+++ b/src/mbgl/renderer/painter_symbol.cpp
@@ -192,7 +192,8 @@ void Painter::renderSymbol(SymbolBucket& bucket, const SymbolLayer& layer, const
         const float fontSize = properties.icon.size;
         const float fontScale = fontSize / 1.0f;
 
-        spriteAtlas->bind(state.isChanging() || layout.placement == PlacementType::Line
+        SpriteAtlas* activeSpriteAtlas = layer.spriteAtlas;
+        activeSpriteAtlas->bind(state.isChanging() || layout.placement == PlacementType::Line
                 || angleOffset != 0 || fontScale != 1 || sdf || state.getPitch() != 0);
 
         if (sdf) {
@@ -202,7 +203,7 @@ void Painter::renderSymbol(SymbolBucket& bucket, const SymbolLayer& layer, const
                       layout.icon,
                       properties.icon,
                       1.0f,
-                      {{ float(spriteAtlas->getWidth()) / 4.0f, float(spriteAtlas->getHeight()) / 4.0f }},
+                      {{ float(activeSpriteAtlas->getWidth()) / 4.0f, float(activeSpriteAtlas->getHeight()) / 4.0f }},
                       *sdfIconShader,
                       &SymbolBucket::drawIcons);
         } else {
diff --git a/src/mbgl/sprite/sprite_atlas.cpp b/src/mbgl/sprite/sprite_atlas.cpp
index 45b0511c68c..ae71f18f037 100644
--- a/src/mbgl/sprite/sprite_atlas.cpp
+++ b/src/mbgl/sprite/sprite_atlas.cpp
@@ -24,9 +24,7 @@ SpriteAtlas::SpriteAtlas(dimension width_, dimension height_, float pixelRatio_,
       pixelRatio(pixelRatio_),
       store(store_),
       bin(width_, height_),
-      data(std::make_unique<uint32_t[]>(pixelWidth * pixelHeight)),
       dirty(true) {
-    std::fill(data.get(), data.get() + pixelWidth * pixelHeight, 0);
 }
 
 Rect<SpriteAtlas::dimension> SpriteAtlas::allocateImage(const size_t pixel_width, const size_t pixel_height) {
@@ -103,6 +101,11 @@ SpriteAtlasPosition SpriteAtlas::getPosition(const std::string& name, bool repea
 }
 
 void SpriteAtlas::copy(const Holder& holder, const bool wrap) {
+    if (!data) {
+        data = std::make_unique<uint32_t[]>(pixelWidth * pixelHeight);
+        std::fill(data.get(), data.get() + pixelWidth * pixelHeight, 0);
+    }
+
     const uint32_t *srcData = reinterpret_cast<const uint32_t *>(holder.texture->data.data());
     if (!srcData) return;
     const vec2<uint32_t> srcSize { holder.texture->pixelWidth, holder.texture->pixelHeight };
@@ -189,6 +192,10 @@ void SpriteAtlas::updateDirty() {
 }
 
 void SpriteAtlas::bind(bool linear) {
+    if (!data) {
+        return; // Empty atlas
+    }
+
     if (!texture) {
         MBGL_CHECK_ERROR(glGenTextures(1, &texture));
         MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, texture));
diff --git a/src/mbgl/sprite/sprite_atlas.hpp b/src/mbgl/sprite/sprite_atlas.hpp
index 0d86279e2d4..0e3e8cf225d 100644
--- a/src/mbgl/sprite/sprite_atlas.hpp
+++ b/src/mbgl/sprite/sprite_atlas.hpp
@@ -66,6 +66,8 @@ class SpriteAtlas : public util::noncopyable {
     inline dimension getTextureWidth() const { return pixelWidth; }
     inline dimension getTextureHeight() const { return pixelHeight; }
     inline float getPixelRatio() const { return pixelRatio; }
+
+    // Only for use in tests.
     inline const uint32_t* getData() const { return data.get(); }
 
 private:
@@ -90,7 +92,7 @@ class SpriteAtlas : public util::noncopyable {
     BinPack<dimension> bin;
     std::map<Key, Holder> images;
     std::set<std::string> uninitialized;
-    const std::unique_ptr<uint32_t[]> data;
+    std::unique_ptr<uint32_t[]> data;
     std::atomic<bool> dirty;
     bool fullUploadRequired = true;
     GLuint texture = 0;
diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp
index 3523a72079d..b56eeecf674 100644
--- a/src/mbgl/style/style.cpp
+++ b/src/mbgl/style/style.cpp
@@ -2,6 +2,7 @@
 #include <mbgl/map/map_data.hpp>
 #include <mbgl/map/source.hpp>
 #include <mbgl/map/transform_state.hpp>
+#include <mbgl/layer/symbol_layer.hpp>
 #include <mbgl/sprite/sprite_store.hpp>
 #include <mbgl/sprite/sprite_atlas.hpp>
 #include <mbgl/style/style_layer.hpp>
@@ -88,6 +89,12 @@ StyleLayer* Style::getLayer(const std::string& id) const {
 }
 
 void Style::addLayer(util::ptr<StyleLayer> layer) {
+    if (SymbolLayer* symbolLayer = dynamic_cast<SymbolLayer*>(layer.get())) {
+        if (!symbolLayer->spriteAtlas) {
+            symbolLayer->spriteAtlas = spriteAtlas.get();
+        }
+    }
+
     layers.emplace_back(std::move(layer));
 }
 
diff --git a/test/sprite/sprite_atlas.cpp b/test/sprite/sprite_atlas.cpp
index e54a88dabe2..ddbd54406d7 100644
--- a/test/sprite/sprite_atlas.cpp
+++ b/test/sprite/sprite_atlas.cpp
@@ -27,7 +27,7 @@ TEST(Sprite, SpriteAtlas) {
     EXPECT_EQ(112, atlas.getTextureHeight());
 
     // Image hasn't been created yet.
-    EXPECT_TRUE(atlas.getData());
+    EXPECT_FALSE(atlas.getData());
 
     auto metro = atlas.getImage("metro", false);
     EXPECT_EQ(0, metro.pos.x);
@@ -42,6 +42,8 @@ TEST(Sprite, SpriteAtlas) {
     EXPECT_EQ(18, metro.texture->pixelHeight);
     EXPECT_EQ(1.0f, metro.texture->pixelRatio);
 
+    EXPECT_TRUE(atlas.getData());
+
     auto pos = atlas.getPosition("metro", false);
     EXPECT_DOUBLE_EQ(20, pos.size[0]);
     EXPECT_DOUBLE_EQ(20, pos.size[1]);