From 3ec4c24a523bcc9030163844ccb52f16e05d03ab Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Thu, 10 Nov 2016 15:48:32 -0800 Subject: [PATCH 1/5] [core] Return source and layer ownership When a source or layer is removed transfer ownership back to the caller so it can (optionally) take it. --- include/mbgl/map/map.hpp | 4 ++-- src/mbgl/map/map.cpp | 13 ++++++++----- src/mbgl/style/style.cpp | 15 ++++++++++++--- src/mbgl/style/style.hpp | 4 ++-- 4 files changed, 24 insertions(+), 12 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 ff36bd7264f..d27cdc1ff07 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -852,11 +852,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) { @@ -881,18 +882,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/style.cpp b/src/mbgl/style/style.cpp index acbc949b4c0..76d70575a24 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -134,7 +134,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 +143,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 +188,17 @@ 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); 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 8e054008851..757d1d9a4f3 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; From 858492217654d698f3cda8fe4447a9dab81eec87 Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Thu, 10 Nov 2016 16:54:23 -0800 Subject: [PATCH 2/5] [core] Remove whitespace --- src/mbgl/map/map.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index d27cdc1ff07..0ef57bbd1ec 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -855,7 +855,7 @@ void Map::addSource(std::unique_ptr source) { std::unique_ptr Map::removeSource(const std::string& sourceID) { if (impl->style) { impl->styleMutated = true; - return impl->style->removeSource(sourceID); + return impl->style->removeSource(sourceID); } return nullptr; } From 00c5210775d76a64d3d15aeb7587a2a33a940c31 Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Thu, 10 Nov 2016 17:08:44 -0800 Subject: [PATCH 3/5] [core] Make removing a CustomLayer triggers deinitialization --- src/mbgl/style/layers/custom_layer_impl.cpp | 5 +++++ src/mbgl/style/layers/custom_layer_impl.hpp | 1 + src/mbgl/style/style.cpp | 5 +++++ 3 files changed, 11 insertions(+) diff --git a/src/mbgl/style/layers/custom_layer_impl.cpp b/src/mbgl/style/layers/custom_layer_impl.cpp index 124d6b0ce96..982476feb06 100644 --- a/src/mbgl/style/layers/custom_layer_impl.cpp +++ b/src/mbgl/style/layers/custom_layer_impl.cpp @@ -43,6 +43,11 @@ void CustomLayer::Impl::initialize() { initializeFn(context); } +void CustomLayer::Impl::deinitialize() { + assert(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 76d70575a24..89483bbd947 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -197,6 +197,11 @@ std::unique_ptr Style::removeLayer(const std::string& id) { 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; } From 50d3792eb37260df69b487b9b9463bed1fa910a8 Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Thu, 10 Nov 2016 17:49:18 -0800 Subject: [PATCH 4/5] [core] Make destructor default and guard against optional deinitializeFn --- src/mbgl/style/layers/custom_layer_impl.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/mbgl/style/layers/custom_layer_impl.cpp b/src/mbgl/style/layers/custom_layer_impl.cpp index 982476feb06..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); @@ -44,8 +40,9 @@ void CustomLayer::Impl::initialize() { } void CustomLayer::Impl::deinitialize() { - assert(deinitializeFn); - deinitializeFn(context); + if (deinitializeFn) { + deinitializeFn(context); + } } void CustomLayer::Impl::render(const TransformState& state) const { From 554abe9eab82251cf5e0316040cb1fc7d1d3f26b Mon Sep 17 00:00:00 2001 From: Jesse Bounds Date: Fri, 11 Nov 2016 13:26:15 -0800 Subject: [PATCH 5/5] [core] Deinitialize all custom layers when style is destroyed --- src/mbgl/style/style.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index 89483bbd947..c39cf3bc3cf 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); }