From b97f40469b6f3d386ae93bc9a23722ff017162f0 Mon Sep 17 00:00:00 2001
From: Alexander Shalamov <alexander.shalamov@mapbox.com>
Date: Sat, 15 Feb 2020 01:38:45 +0200
Subject: [PATCH] [core] Fix sprite sheet merging in
 Style::Impl::onSpriteLoaded

If we get a new sprite sheet from the server, we need to merge current
sprite sheet with a new one, while overwiting duplicates and keeping
old unique images in a style.
---
 src/mbgl/sprite/sprite_parser.cpp | 18 ++++++++++++------
 src/mbgl/style/image_impl.hpp     |  3 +++
 src/mbgl/style/style_impl.cpp     | 16 +++++++++++++++-
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/src/mbgl/sprite/sprite_parser.cpp b/src/mbgl/sprite/sprite_parser.cpp
index 40e63d2d7..155269cdb 100644
--- a/src/mbgl/sprite/sprite_parser.cpp
+++ b/src/mbgl/sprite/sprite_parser.cpp
@@ -1,5 +1,6 @@
 #include <mbgl/sprite/sprite_parser.hpp>
 #include <mbgl/style/image.hpp>
+#include <mbgl/style/image_impl.hpp>
 
 #include <mbgl/util/exception.hpp>
 #include <mbgl/util/logging.hpp>
@@ -8,6 +9,7 @@
 #include <mbgl/util/rapidjson.hpp>
 #include <mbgl/util/string.hpp>
 
+#include <algorithm>
 #include <cmath>
 #include <limits>
 
@@ -25,10 +27,9 @@ std::unique_ptr<style::Image> createStyleImage(const std::string& id,
                                                style::ImageStretches&& stretchY,
                                                optional<style::ImageContent> content) {
     // Disallow invalid parameter configurations.
-    if (width <= 0 || height <= 0 || width > 1024 || height > 1024 ||
-        ratio <= 0 || ratio > 10 ||
-        srcX >= image.size.width || srcY >= image.size.height ||
-        srcX + width > image.size.width || srcY + height > image.size.height) {
+    if (width <= 0 || height <= 0 || width > 1024 || height > 1024 || ratio <= 0 || ratio > 10 ||
+        srcX >= image.size.width || srcY >= image.size.height || srcX + width > image.size.width ||
+        srcY + height > image.size.height) {
         Log::Error(Event::Sprite,
                    "Can't create image with invalid metrics: %ux%u@%u,%u in %ux%u@%sx sprite",
                    width,
@@ -41,10 +42,10 @@ std::unique_ptr<style::Image> createStyleImage(const std::string& id,
         return nullptr;
     }
 
-    PremultipliedImage dstImage({ width, height });
+    PremultipliedImage dstImage({width, height});
 
     // Copy from the source image into our individual sprite image
-    PremultipliedImage::copy(image, dstImage, { srcX, srcY }, { 0, 0 }, { width, height });
+    PremultipliedImage::copy(image, dstImage, {srcX, srcY}, {0, 0}, {width, height});
 
     try {
         return std::make_unique<style::Image>(
@@ -196,6 +197,11 @@ std::vector<Immutable<style::Image::Impl>> parseSprite(const std::string& encode
             }
         }
     }
+
+    assert([&images] {
+        std::sort(images.begin(), images.end());
+        return std::unique(images.begin(), images.end()) == images.end();
+    }());
     return images;
 }
 
diff --git a/src/mbgl/style/image_impl.hpp b/src/mbgl/style/image_impl.hpp
index 76a68017e..11ceab483 100644
--- a/src/mbgl/style/image_impl.hpp
+++ b/src/mbgl/style/image_impl.hpp
@@ -48,5 +48,8 @@ using ImageMap = std::unordered_map<std::string, Immutable<style::Image::Impl>>;
 using ImageDependencies = std::unordered_map<std::string, ImageType>;
 using ImageRequestPair = std::pair<ImageDependencies, uint64_t>;
 using ImageVersionMap = std::unordered_map<std::string, uint32_t>;
+inline bool operator<(const Immutable<mbgl::style::Image::Impl>& a, const Immutable<mbgl::style::Image::Impl>& b) {
+    return a->id < b->id;
+}
 
 } // namespace mbgl
diff --git a/src/mbgl/style/style_impl.cpp b/src/mbgl/style/style_impl.cpp
index 59cb57aca..83570e40e 100644
--- a/src/mbgl/style/style_impl.cpp
+++ b/src/mbgl/style/style_impl.cpp
@@ -341,9 +341,23 @@ void Style::Impl::onSourceDescriptionChanged(Source& source) {
 
 void Style::Impl::onSpriteLoaded(std::vector<Immutable<style::Image::Impl>> images_) {
     auto newImages = makeMutable<ImageImpls>(*images);
+    assert(std::is_sorted(newImages->begin(), newImages->end()));
+
+    for (auto it = images_.begin(); it != images_.end();) {
+        const auto& image = *it;
+        const auto first = std::lower_bound(newImages->begin(), newImages->end(), image);
+        auto found = first != newImages->end() && (image->id == (*first)->id) ? first : newImages->end();
+        if (found != newImages->end()) {
+            *found = std::move(*it);
+            it = images_.erase(it);
+        } else {
+            ++it;
+        }
+    }
+
     newImages->insert(
         newImages->end(), std::make_move_iterator(images_.begin()), std::make_move_iterator(images_.end()));
-    std::sort(newImages->begin(), newImages->end(), [](const auto& a, const auto& b) { return a->id < b->id; });
+    std::sort(newImages->begin(), newImages->end());
     images = std::move(newImages);
     spriteLoaded = true;
     observer->onUpdate(); // For *-pattern properties.