From 244484ca6c64b6024a59786a194ea8ba19fae809 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Tue, 20 Jun 2017 15:19:12 -0700 Subject: [PATCH] [core] retain GeometryTileLayer in SymbolLayout We're storing GeometryTileFeature objects in SymbolLayout, but they may reference data in GeometryTileLayer, which could go away if we don't retain it. --- src/mbgl/layout/symbol_layout.cpp | 13 +++++++------ src/mbgl/layout/symbol_layout.hpp | 6 ++++-- src/mbgl/renderer/layers/render_symbol_layer.cpp | 4 ++-- src/mbgl/renderer/layers/render_symbol_layer.hpp | 7 +++++-- src/mbgl/tile/geometry_tile_worker.cpp | 5 +++-- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index ed580bcb1..adc7eaaed 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -40,10 +40,10 @@ static bool has(const style::SymbolLayoutProperties::PossiblyEvaluated& layout) SymbolLayout::SymbolLayout(const BucketParameters& parameters, const std::vector& layers, - const GeometryTileLayer& sourceLayer, + std::unique_ptr sourceLayer_, ImageDependencies& imageDependencies, GlyphDependencies& glyphDependencies) - : sourceLayerName(sourceLayer.getName()), + : sourceLayer(std::move(sourceLayer_)), bucketName(layers.at(0)->getID()), overscaling(parameters.tileID.overscaleFactor()), zoom(parameters.tileID.overscaledZ), @@ -95,9 +95,9 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters, } // Determine glyph dependencies - const size_t featureCount = sourceLayer.featureCount(); + const size_t featureCount = sourceLayer->featureCount(); for (size_t i = 0; i < featureCount; ++i) { - auto feature = sourceLayer.getFeature(i); + auto feature = sourceLayer->getFeature(i); if (!leader.filter(feature->getType(), feature->getID(), [&] (const auto& key) { return feature->getValue(key); })) continue; @@ -328,8 +328,9 @@ void SymbolLayout::addFeature(const std::size_t index, ? SymbolPlacementType::Point : layout.get(); const float textRepeatDistance = symbolSpacing / 2; - IndexedSubfeature indexedFeature = {feature.index, sourceLayerName, bucketName, symbolInstances.size()}; - + IndexedSubfeature indexedFeature = { feature.index, sourceLayer->getName(), bucketName, + symbolInstances.size() }; + auto addSymbolInstance = [&] (const GeometryCoordinates& line, Anchor& anchor) { // https://github.com/mapbox/vector-tile-spec/tree/master/2.1#41-layers // +-------------------+ Symbols with anchors located on tile edges diff --git a/src/mbgl/layout/symbol_layout.hpp b/src/mbgl/layout/symbol_layout.hpp index b22c47c56..8cdaadff0 100644 --- a/src/mbgl/layout/symbol_layout.hpp +++ b/src/mbgl/layout/symbol_layout.hpp @@ -29,7 +29,7 @@ class SymbolLayout { public: SymbolLayout(const BucketParameters&, const std::vector&, - const GeometryTileLayer&, + std::unique_ptr, ImageDependencies&, GlyphDependencies&); @@ -74,7 +74,9 @@ private: const float placementAngle, WritingModeType writingModes); - const std::string sourceLayerName; + // Stores the layer so that we can hold on to GeometryTileFeature instances in SymbolFeature, + // which may reference data from this object. + const std::unique_ptr sourceLayer; const std::string bucketName; const float overscaling; const float zoom; diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp index 2edb1ad3c..9d59162a8 100644 --- a/src/mbgl/renderer/layers/render_symbol_layer.cpp +++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp @@ -23,12 +23,12 @@ std::unique_ptr RenderSymbolLayer::createBucket(const BucketParameters&, std::unique_ptr RenderSymbolLayer::createLayout(const BucketParameters& parameters, const std::vector& group, - const GeometryTileLayer& layer, + std::unique_ptr layer, GlyphDependencies& glyphDependencies, ImageDependencies& imageDependencies) const { return std::make_unique(parameters, group, - layer, + std::move(layer), imageDependencies, glyphDependencies); } diff --git a/src/mbgl/renderer/layers/render_symbol_layer.hpp b/src/mbgl/renderer/layers/render_symbol_layer.hpp index 99fd09111..2199103de 100644 --- a/src/mbgl/renderer/layers/render_symbol_layer.hpp +++ b/src/mbgl/renderer/layers/render_symbol_layer.hpp @@ -71,8 +71,11 @@ public: style::SymbolPropertyValues textPropertyValues(const style::SymbolLayoutProperties::PossiblyEvaluated&) const; std::unique_ptr createBucket(const BucketParameters&, const std::vector&) const override; - std::unique_ptr createLayout(const BucketParameters&, const std::vector&, - const GeometryTileLayer&, GlyphDependencies&, ImageDependencies&) const; + std::unique_ptr createLayout(const BucketParameters&, + const std::vector&, + std::unique_ptr, + GlyphDependencies&, + ImageDependencies&) const; // Paint properties style::SymbolPaintProperties::Unevaluated unevaluated; diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index a5a82a792..12bb84d7e 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -309,8 +309,9 @@ void GeometryTileWorker::redoLayout() { featureIndex->setBucketLayerIDs(leader.getID(), layerIDs); if (leader.is()) { - symbolLayoutMap.emplace(leader.getID(), - leader.as()->createLayout(parameters, group, *geometryLayer, glyphDependencies, imageDependencies)); + auto layout = leader.as()->createLayout( + parameters, group, std::move(geometryLayer), glyphDependencies, imageDependencies); + symbolLayoutMap.emplace(leader.getID(), std::move(layout)); } else { const Filter& filter = leader.baseImpl->filter; const std::string& sourceLayerID = leader.baseImpl->sourceLayer;