From edf40bb6d9f0ee3731f39de597ce9a167571ea5b Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Fri, 11 Nov 2016 14:40:04 -0800 Subject: [PATCH] [core] Return source and layer ownership (#7014) When a source or layer is removed transfer ownership back to the caller so it can (optionally) take it. Preserve the behavior that removing a CustomLayer triggers deinitialization. Deinitialize all custom layers when a style is destroyed in case those layers are not explicitly removed. --- include/mbgl/map/map.hpp | 4 ++-- src/mbgl/map/map.cpp | 13 +++++++---- src/mbgl/style/layers/custom_layer_impl.cpp | 12 ++++++---- src/mbgl/style/layers/custom_layer_impl.hpp | 1 + src/mbgl/style/style.cpp | 26 ++++++++++++++++++--- src/mbgl/style/style.hpp | 4 ++-- 6 files changed, 43 insertions(+), 17 deletions(-) diff --git a/include/mbgl/map/map.hpp b/include/mbgl/map/map.hpp index 6656bccd518..aaec346731e 100644 --- a/include/mbgl/map/map.hpp +++ b/include/mbgl/map/map.hpp @@ -159,12 +159,12 @@ class Map : private util::noncopyable { // Sources style::Source* getSource(const std::string& sourceID); void addSource(std::unique_ptr); - void removeSource(const std::string& sourceID); + std::unique_ptr removeSource(const std::string& sourceID); // Layers style::Layer* getLayer(const std::string& layerID); void addLayer(std::unique_ptr, const optional& beforeLayerID = {}); - void removeLayer(const std::string& layerID); + std::unique_ptr removeLayer(const std::string& layerID); // Add image, bound to the style void addImage(const std::string&, std::unique_ptr); diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index 57845080f44..0c3e395e8a9 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -851,11 +851,12 @@ void Map::addSource(std::unique_ptr source) { } } -void Map::removeSource(const std::string& sourceID) { +std::unique_ptr Map::removeSource(const std::string& sourceID) { if (impl->style) { impl->styleMutated = true; - impl->style->removeSource(sourceID); + return impl->style->removeSource(sourceID); } + return nullptr; } style::Layer* Map::getLayer(const std::string& layerID) { @@ -880,18 +881,20 @@ void Map::addLayer(std::unique_ptr layer, const optional& be impl->backend.deactivate(); } -void Map::removeLayer(const std::string& id) { +std::unique_ptr Map::removeLayer(const std::string& id) { if (!impl->style) { - return; + return nullptr; } impl->styleMutated = true; impl->backend.activate(); - impl->style->removeLayer(id); + auto removedLayer = impl->style->removeLayer(id); impl->onUpdate(Update::Classes); impl->backend.deactivate(); + + return removedLayer; } void Map::addImage(const std::string& name, std::unique_ptr image) { diff --git a/src/mbgl/style/layers/custom_layer_impl.cpp b/src/mbgl/style/layers/custom_layer_impl.cpp index 124d6b0ce96..1126d57552b 100644 --- a/src/mbgl/style/layers/custom_layer_impl.cpp +++ b/src/mbgl/style/layers/custom_layer_impl.cpp @@ -23,11 +23,7 @@ CustomLayer::Impl::Impl(const CustomLayer::Impl& other) // Don't copy anything else. } -CustomLayer::Impl::~Impl() { - if (deinitializeFn) { - deinitializeFn(context); - } -} +CustomLayer::Impl::~Impl() = default; std::unique_ptr CustomLayer::Impl::clone() const { return std::make_unique(*this); @@ -43,6 +39,12 @@ void CustomLayer::Impl::initialize() { initializeFn(context); } +void CustomLayer::Impl::deinitialize() { + if (deinitializeFn) { + deinitializeFn(context); + } +} + void CustomLayer::Impl::render(const TransformState& state) const { assert(renderFn); diff --git a/src/mbgl/style/layers/custom_layer_impl.hpp b/src/mbgl/style/layers/custom_layer_impl.hpp index ffa892ddf81..b5b626ca5e0 100644 --- a/src/mbgl/style/layers/custom_layer_impl.hpp +++ b/src/mbgl/style/layers/custom_layer_impl.hpp @@ -21,6 +21,7 @@ class CustomLayer::Impl : public Layer::Impl { ~Impl() final; void initialize(); + void deinitialize(); void render(const TransformState&) const; private: diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index 57e2580d4d5..0b3d782d06d 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -51,6 +51,12 @@ Style::~Style() { source->baseImpl->setObserver(nullptr); } + for (const auto& layer : layers) { + if (CustomLayer* customLayer = layer->as()) { + customLayer->impl->deinitialize(); + } + } + glyphAtlas->setObserver(nullptr); spriteAtlas->setObserver(nullptr); } @@ -134,7 +140,7 @@ void Style::addSource(std::unique_ptr source) { sources.emplace_back(std::move(source)); } -void Style::removeSource(const std::string& id) { +std::unique_ptr Style::removeSource(const std::string& id) { auto it = std::find_if(sources.begin(), sources.end(), [&](const auto& source) { return source->getID() == id; }); @@ -143,8 +149,11 @@ void Style::removeSource(const std::string& id) { throw std::runtime_error("no such source"); } + auto source = std::move(*it); sources.erase(it); updateBatch.sourceIDs.erase(id); + + return source; } std::vector Style::getLayers() const { @@ -185,11 +194,22 @@ Layer* Style::addLayer(std::unique_ptr layer, optional befor return layers.emplace(before ? findLayer(*before) : layers.end(), std::move(layer))->get(); } -void Style::removeLayer(const std::string& id) { - auto it = findLayer(id); +std::unique_ptr Style::removeLayer(const std::string& id) { + auto it = std::find_if(layers.begin(), layers.end(), [&](const auto& layer) { + return layer->baseImpl->id == id; + }); + if (it == layers.end()) throw std::runtime_error("no such layer"); + + auto layer = std::move(*it); + + if (CustomLayer* customLayer = layer->as()) { + customLayer->impl->deinitialize(); + } + layers.erase(it); + return layer; } std::string Style::getName() const { diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp index 2846ddb388c..1af020d6c2b 100644 --- a/src/mbgl/style/style.hpp +++ b/src/mbgl/style/style.hpp @@ -66,13 +66,13 @@ class Style : public GlyphAtlasObserver, Source* getSource(const std::string& id) const; void addSource(std::unique_ptr); - void removeSource(const std::string& sourceID); + std::unique_ptr removeSource(const std::string& sourceID); std::vector getLayers() const; Layer* getLayer(const std::string& id) const; Layer* addLayer(std::unique_ptr, optional beforeLayerID = {}); - void removeLayer(const std::string& layerID); + std::unique_ptr removeLayer(const std::string& layerID); std::string getName() const; LatLng getDefaultLatLng() const;