[core] Introduce style::CollectionWithPersistentOrder

src/mbgl/style/collection.hpp

@@ -22,13 +22,13 @@ namespace style {
       * A `std::string getID() const` method
 */
 template <class T>
-class Collection {
+class CollectionBase {
 public:
     using Impl = typename T::Impl;
     using WrapperVector = std::vector<std::unique_ptr<T>>;
     using ImmutableVector = Immutable<std::vector<Immutable<Impl>>>;
 
-    Collection();
+    CollectionBase();
 
     std::size_t size() const;
     T* get(const std::string&) const;
@@ -41,46 +41,86 @@ class Collection {
 
     void clear();
 
-    T* add(std::unique_ptr<T>, const optional<std::string>& = {});
-    std::unique_ptr<T> remove(const std::string&);
-
+protected:
+    std::size_t index(const std::string&) const;
+    T* add(std::size_t wrapperIndex, std::size_t implIndex, std::unique_ptr<T> wrapper);
+    std::unique_ptr<T> remove(std::size_t wrapperIndex, std::size_t implIndex);
     // Must be called whenever an element of the collection is internally mutated.
     // Typically, each element permits registration of an observer, and the observer
     // should call this method.
-    void update(const T&);
-
-private:
-    std::size_t index(const std::string&) const;
+    void update(std::size_t implIndex, const T&);
 
     WrapperVector wrappers;
     ImmutableVector impls;
 };
 
+template <class T, bool persistentImplsOrder = false>
+class Collection;
+
 template <class T>
-Collection<T>::Collection()
-    : impls(makeMutable<std::vector<Immutable<Impl>>>()) {
-}
+class Collection<T, false /*persistentImplsOrder*/> : public CollectionBase<T> {
+    using Base = CollectionBase<T>;
+
+public:
+    T* add(std::unique_ptr<T> wrapper, const optional<std::string>& before = nullopt) {
+        std::size_t index = before ? Base::index(*before) : Base::size();
+        return Base::add(index, index, std::move(wrapper));
+    }
+    std::unique_ptr<T> remove(const std::string& id) {
+        std::size_t index = Base::index(id);
+        return Base::remove(index, index);
+    }
+    void update(const T& wrapper) { Base::update(Base::index(wrapper.getID()), wrapper); }
+};
+
+template <class T>
+class Collection<T, true /*persistentImplsOrder*/> : public CollectionBase<T> {
+    using Base = CollectionBase<T>;
+
+public:
+    T* add(std::unique_ptr<T> wrapper) {
+        std::size_t i = implsIndex(wrapper->getID());
+        return Base::add(Base::size(), i, std::move(wrapper));
+    }
+
+    std::unique_ptr<T> remove(const std::string& id) { return Base::remove(Base::index(id), implsIndex(id)); }
 
+    void update(const T& wrapper) { Base::update(implsIndex(wrapper.getID()), wrapper); }
+
+private:
+    std::size_t implsIndex(const std::string& id) const {
+        const auto& impls_ = *Base::impls;
+        auto it = std::lower_bound(
+            impls_.begin(), impls_.end(), id, [](const auto& a, const std::string& b) { return a->id < b; });
+        return it - impls_.begin();
+    }
+};
 template <class T>
-std::size_t Collection<T>::size() const {
+using CollectionWithPersistentOrder = Collection<T, true>;
+
+template <class T>
+CollectionBase<T>::CollectionBase() : impls(makeMutable<std::vector<Immutable<Impl>>>()) {}
+
+template <class T>
+std::size_t CollectionBase<T>::size() const {
     return wrappers.size();
 }
 
 template <class T>
-std::size_t Collection<T>::index(const std::string& id) const {
+std::size_t CollectionBase<T>::index(const std::string& id) const {
     return std::find_if(wrappers.begin(), wrappers.end(), [&](const auto& e) {
         return e->getID() == id;
     }) - wrappers.begin();
 }
 
 template <class T>
-T* Collection<T>::get(const std::string& id) const {
+T* CollectionBase<T>::get(const std::string& id) const {
     std::size_t i = index(id);
     return i < size() ? wrappers[i].get() : nullptr;
 }
 
 template <class T>
-std::vector<T*> Collection<T>::getWrappers() const {
+std::vector<T*> CollectionBase<T>::getWrappers() const {
     std::vector<T*> result;
     result.reserve(wrappers.size());
 
@@ -92,7 +132,7 @@ std::vector<T*> Collection<T>::getWrappers() const {
 }
 
 template <class T>
-void Collection<T>::clear() {
+void CollectionBase<T>::clear() {
     mutate(impls, [&] (auto& impls_) {
         impls_.clear();
     });
@@ -101,40 +141,37 @@ void Collection<T>::clear() {
 }
 
 template <class T>
-T* Collection<T>::add(std::unique_ptr<T> wrapper, const optional<std::string>& before) {
-    std::size_t i = before ? index(*before) : size();
-
-    mutate(impls, [&] (auto& impls_) {
-        impls_.emplace(impls_.begin() + i, wrapper->baseImpl);
-    });
+T* CollectionBase<T>::add(std::size_t wrapperIndex, std::size_t implIndex, std::unique_ptr<T> wrapper) {
+    assert(wrapperIndex <= size());
+    assert(implIndex <= size());
+    mutate(impls, [&](auto& impls_) { impls_.emplace(impls_.begin() + implIndex, wrapper->baseImpl); });
 
-    return wrappers.emplace(wrappers.begin() + i, std::move(wrapper))->get();
+    return wrappers.emplace(wrappers.begin() + wrapperIndex, std::move(wrapper))->get();
 }
 
 template <class T>
-std::unique_ptr<T> Collection<T>::remove(const std::string& id) {
-    std::size_t i = index(id);
-
-    if (i >= size()) {
+std::unique_ptr<T> CollectionBase<T>::remove(std::size_t wrapperIndex, std::size_t implIndex) {
+    if (wrapperIndex >= size()) {
         return nullptr;
     }
+    assert(implIndex < size());
 
-    auto source = std::move(wrappers[i]);
+    auto source = std::move(wrappers[wrapperIndex]);
 
-    mutate(impls, [&] (auto& impls_) {
-        impls_.erase(impls_.begin() + i);
-    });
+    mutate(impls, [&](auto& impls_) { impls_.erase(impls_.begin() + implIndex); });
 
-    wrappers.erase(wrappers.begin() + i);
+    wrappers.erase(wrappers.begin() + wrapperIndex);
 
     return source;
 }
 
 template <class T>
-void Collection<T>::update(const T& wrapper) {
-    mutate(impls, [&] (auto& impls_) {
-        impls_.at(this->index(wrapper.getID())) = wrapper.baseImpl;
-    });
+void CollectionBase<T>::update(std::size_t implIndex, const T& wrapper) {
+    if (implIndex >= size()) {
+        assert(false);
+        return;
+    }
+    mutate(impls, [&](auto& impls_) { impls_.at(implIndex) = wrapper.baseImpl; });
 }
 
 } // namespace style

src/mbgl/style/style_impl.cpp

@@ -2,6 +2,7 @@
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/storage/resource.hpp>
 #include <mbgl/storage/response.hpp>
+#include <mbgl/style/image_impl.hpp>
 #include <mbgl/style/layer_impl.hpp>
 #include <mbgl/style/layers/background_layer.hpp>
 #include <mbgl/style/layers/circle_layer.hpp>

src/mbgl/style/style_impl.hpp

@@ -106,8 +106,8 @@ class Style::Impl : public SpriteLoaderObserver,
     std::unique_ptr<SpriteLoader> spriteLoader;
 
     std::string glyphURL;
-    Collection<style::Image> images;
-    Collection<Source> sources;
+    CollectionWithPersistentOrder<style::Image> images;
+    CollectionWithPersistentOrder<Source> sources;
     Collection<Layer> layers;
     TransitionOptions transitionOptions;
     std::unique_ptr<Light> light;

test/style/style.test.cpp

@@ -103,3 +103,25 @@ TEST(Style, RemoveSourceInUse) {
 
     EXPECT_EQ(log->count(logMessage), 1u);
 }
+
+TEST(Style, SourceImplsOrder) {
+    util::RunLoop loop;
+    StubFileSource fileSource;
+    Style::Impl style{fileSource, 1.0};
+
+    style.addSource(std::make_unique<VectorSource>("c", "mapbox://mapbox.mapbox-terrain-v2"));
+    style.addSource(std::make_unique<VectorSource>("b", "mapbox://mapbox.mapbox-terrain-v2"));
+    style.addSource(std::make_unique<VectorSource>("a", "mapbox://mapbox.mapbox-terrain-v2"));
+
+    auto sources = style.getSources();
+    ASSERT_EQ(3u, sources.size());
+    EXPECT_EQ("c", sources[0]->getID());
+    EXPECT_EQ("b", sources[1]->getID());
+    EXPECT_EQ("a", sources[2]->getID());
+
+    const auto& sourceImpls = *style.getSourceImpls();
+    ASSERT_EQ(3u, sourceImpls.size());
+    EXPECT_EQ("a", sourceImpls[0]->id);
+    EXPECT_EQ("b", sourceImpls[1]->id);
+    EXPECT_EQ("c", sourceImpls[2]->id);
+}