From 17264c5ee0a4262c1e44750a05474286732a27df Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Tue, 21 Aug 2018 14:54:08 -0700 Subject: [core] only wait for images if bucket uses pattern property --- cmake/core-files.txt | 1 + src/mbgl/layout/pattern_layout.hpp | 80 ++++++++++++++++++---- src/mbgl/layout/symbol_layout.cpp | 7 +- src/mbgl/renderer/bucket.hpp | 5 +- src/mbgl/renderer/buckets/circle_bucket.cpp | 5 +- src/mbgl/renderer/buckets/circle_bucket.hpp | 6 +- src/mbgl/renderer/buckets/fill_bucket.cpp | 10 ++- src/mbgl/renderer/buckets/fill_bucket.hpp | 6 +- .../renderer/buckets/fill_extrusion_bucket.cpp | 10 ++- .../renderer/buckets/fill_extrusion_bucket.hpp | 6 +- src/mbgl/renderer/buckets/heatmap_bucket.cpp | 5 +- src/mbgl/renderer/buckets/heatmap_bucket.hpp | 3 +- src/mbgl/renderer/buckets/line_bucket.cpp | 10 ++- src/mbgl/renderer/buckets/line_bucket.hpp | 3 +- src/mbgl/renderer/paint_property_binder.hpp | 31 ++++----- .../renderer/possibly_evaluated_property_value.hpp | 2 +- src/mbgl/style/layers/fill_extrusion_layer.cpp | 2 +- src/mbgl/style/layers/fill_layer.cpp | 2 +- src/mbgl/style/layers/line_layer.cpp | 2 +- src/mbgl/tile/geometry_tile_worker.cpp | 52 +++++++------- src/mbgl/tile/geometry_tile_worker.hpp | 3 + test/gl/bucket.test.cpp | 10 +-- 22 files changed, 171 insertions(+), 90 deletions(-) diff --git a/cmake/core-files.txt b/cmake/core-files.txt index 0bfded8db8..a1e43d2357 100644 --- a/cmake/core-files.txt +++ b/cmake/core-files.txt @@ -97,6 +97,7 @@ src/mbgl/layout/clip_lines.cpp src/mbgl/layout/clip_lines.hpp src/mbgl/layout/merge_lines.cpp src/mbgl/layout/merge_lines.hpp +src/mbgl/layout/pattern_layout.hpp src/mbgl/layout/symbol_feature.hpp src/mbgl/layout/symbol_instance.cpp src/mbgl/layout/symbol_instance.hpp diff --git a/src/mbgl/layout/pattern_layout.hpp b/src/mbgl/layout/pattern_layout.hpp index 61a70a2991..ed89874fa0 100644 --- a/src/mbgl/layout/pattern_layout.hpp +++ b/src/mbgl/layout/pattern_layout.hpp @@ -1,9 +1,26 @@ #pragma once #include #include +#include namespace mbgl { +class PatternDependency { +public: + std::string min; + std::string mid; + std::string max; +}; + +using PatternLayerMap = std::unordered_map; + +class PatternFeature { +public: + const uint32_t i; + std::unique_ptr feature; + PatternLayerMap patterns; +}; + template class PatternLayout { public: @@ -14,7 +31,8 @@ public: : bucketLeaderID(layers.at(0)->getID()), sourceLayer(std::move(sourceLayer_)), zoom(parameters.tileID.overscaledZ), - overscaling(parameters.tileID.overscaleFactor()) { + overscaling(parameters.tileID.overscaleFactor()), + hasPattern(false) { using PatternLayer = typename B::RenderLayerType; const auto renderLayer = layers.at(0)->as(); @@ -26,32 +44,65 @@ public: for (const auto& layer : layers) { const typename B::PossiblyEvaluatedPaintProperties evaluatedProps = layer->as()->paintProperties(); layerPaintProperties.emplace(layer->getID(), std::move(evaluatedProps)); - const auto patterns = evaluatedProps.template get().possibleOutputs(); - - for (auto& pattern : patterns) { - const auto patternString = pattern.value_or(""); - if (!patternString.empty()) { - patternDependencies.emplace(patternString, ImageType::Pattern); - } + const auto patternProperty = evaluatedProps.template get(); + const auto constantPattern = patternProperty.constantOr(Faded >{ "", "", 0.0f, 0.0f, 0.0f}); + // determine if layer group has any layers that use *-pattern property and add + // constant pattern dependencies. + if (!patternProperty.isConstant()) { + hasPattern = true; + } else if (!constantPattern.to.empty()){ + hasPattern = true; + patternDependencies.emplace(constantPattern.to, ImageType::Pattern); + patternDependencies.emplace(constantPattern.from, ImageType::Pattern); } } + const size_t featureCount = sourceLayer->featureCount(); for (size_t i = 0; i < featureCount; ++i) { auto feature = sourceLayer->getFeature(i); if (!leader.filter(style::expression::EvaluationContext { this->zoom, feature.get() })) continue; - features.push_back({i, std::move(feature)}); + + std::unordered_map patternDependencyMap; + if (hasPattern) { + for (const auto& layer : layers) { + const auto it = layerPaintProperties.find(layer->getID()); + if (it != layerPaintProperties.end()) { + const auto paint = it->second; + const auto patternProperty = paint.template get(); + if (!patternProperty.isConstant()) { + // For layers with non-data-constant pattern properties, evaluate their expression and add + // the patterns to the dependency vector + const auto min = patternProperty.evaluate(*feature, zoom - 1, PatternLayer::PatternProperty::defaultValue()); + const auto mid = patternProperty.evaluate(*feature, zoom, PatternLayer::PatternProperty::defaultValue()); + const auto max = patternProperty.evaluate(*feature, zoom + 1, PatternLayer::PatternProperty::defaultValue()); + + patternDependencies.emplace(min.to, ImageType::Pattern); + patternDependencies.emplace(mid.to, ImageType::Pattern); + patternDependencies.emplace(max.to, ImageType::Pattern); + patternDependencyMap.emplace(layer->getID(), PatternDependency {min.to, mid.to, max.to}); + + } + } + } + } + features.push_back({static_cast(i), std::move(feature), patternDependencyMap}); } }; + bool pattern() { + return hasPattern; + } + std::unique_ptr createBucket(const ImagePositions& patternPositions, std::unique_ptr& featureIndex) { auto bucket = std::make_unique(layout, layerPaintProperties, zoom, overscaling); - for (auto & pair : features) { - std::unique_ptr feature = std::move(pair.second); - const auto i = pair.first; + for (auto & patternFeature : features) { + const auto i = patternFeature.i; + std::unique_ptr feature = std::move(patternFeature.feature); + PatternLayerMap patterns = patternFeature.patterns; GeometryCollection geometries = feature->getGeometries(); - bucket->addFeature(*feature, geometries, patternPositions); + bucket->addFeature(*feature, geometries, patternPositions, patterns); featureIndex->insert(geometries, i, sourceLayerID, groupID); } return bucket; @@ -62,13 +113,14 @@ public: const std::string bucketLeaderID; private: const std::unique_ptr sourceLayer; - std::vector>> features; + std::vector features; typename B::PossiblyEvaluatedLayoutProperties layout; const float zoom; const uint32_t overscaling; std::string sourceLayerID; std::string groupID; + bool hasPattern; }; } // namespace mbgl diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index 5ee2e3d188..fd4dd2bedd 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -455,15 +455,14 @@ std::unique_ptr SymbolLayout::place(const bool showCollisionBoxes) symbolInstance.iconOffset, WritingModeType::None, symbolInstance.line, std::vector()); symbolInstance.placedIconIndex = bucket->icon.placedSymbols.size() - 1; PlacedSymbol& iconSymbol = bucket->icon.placedSymbols.back(); - iconSymbol.vertexStartIndex = addSymbol( - bucket->icon, sizeData, *symbolInstance.iconQuad, + iconSymbol.vertexStartIndex = addSymbol(bucket->icon, sizeData, *symbolInstance.iconQuad, symbolInstance.anchor, iconSymbol); } } for (auto& pair : bucket->paintPropertyBinders) { - pair.second.first.populateVertexVectors(feature, bucket->icon.vertices.vertexSize(), {}); - pair.second.second.populateVertexVectors(feature, bucket->text.vertices.vertexSize(), {}); + pair.second.first.populateVertexVectors(feature, bucket->icon.vertices.vertexSize(), {}, {}); + pair.second.second.populateVertexVectors(feature, bucket->text.vertices.vertexSize(), {}, {}); } } diff --git a/src/mbgl/renderer/bucket.hpp b/src/mbgl/renderer/bucket.hpp index 62be423d66..7ccab8672d 100644 --- a/src/mbgl/renderer/bucket.hpp +++ b/src/mbgl/renderer/bucket.hpp @@ -14,6 +14,8 @@ class Context; } // namespace gl class RenderLayer; +class PatternDependency; +using PatternLayerMap = std::unordered_map; class Bucket : private util::noncopyable { public: @@ -43,7 +45,8 @@ public: // pass-by-const-ref the geometries as a second parameter. virtual void addFeature(const GeometryTileFeature&, const GeometryCollection&, - const mbgl::ImagePositions&) {}; + const ImagePositions&, + const PatternLayerMap&) {}; virtual void populateFeatureBuffers(const ImagePositions&) {}; virtual void addPatternDependencies(const std::vector&, ImageDependencies&) {}; diff --git a/src/mbgl/renderer/buckets/circle_bucket.cpp b/src/mbgl/renderer/buckets/circle_bucket.cpp index e2f5c51a7b..7ab2ef3078 100644 --- a/src/mbgl/renderer/buckets/circle_bucket.cpp +++ b/src/mbgl/renderer/buckets/circle_bucket.cpp @@ -39,8 +39,9 @@ bool CircleBucket::hasData() const { } void CircleBucket::addFeature(const GeometryTileFeature& feature, - const GeometryCollection& geometry, - const ImagePositions&) { + const GeometryCollection& geometry, + const ImagePositions&, + const PatternLayerMap&) { constexpr const uint16_t vertexLength = 4; for (auto& circle : geometry) { diff --git a/src/mbgl/renderer/buckets/circle_bucket.hpp b/src/mbgl/renderer/buckets/circle_bucket.hpp index 011244c1f0..db61a0c112 100644 --- a/src/mbgl/renderer/buckets/circle_bucket.hpp +++ b/src/mbgl/renderer/buckets/circle_bucket.hpp @@ -18,8 +18,10 @@ public: CircleBucket(const BucketParameters&, const std::vector&); void addFeature(const GeometryTileFeature&, - const GeometryCollection&, - const mbgl::ImagePositions&) override; + const GeometryCollection&, + const ImagePositions&, + const PatternLayerMap&) override; + bool hasData() const override; void upload(gl::Context&) override; diff --git a/src/mbgl/renderer/buckets/fill_bucket.cpp b/src/mbgl/renderer/buckets/fill_bucket.cpp index c8fe5a2bc1..3b1b7ec00f 100644 --- a/src/mbgl/renderer/buckets/fill_bucket.cpp +++ b/src/mbgl/renderer/buckets/fill_bucket.cpp @@ -45,7 +45,8 @@ FillBucket::FillBucket(const FillBucket::PossiblyEvaluatedLayoutProperties, void FillBucket::addFeature(const GeometryTileFeature& feature, const GeometryCollection& geometry, - const ImagePositions& patternPositions) { + const ImagePositions& patternPositions, + const PatternLayerMap& patternDependencies) { for (auto& polygon : classifyRings(geometry)) { // Optimize polygons with many interior rings for earcut tesselation. limitHoles(polygon, 500); @@ -110,7 +111,12 @@ void FillBucket::addFeature(const GeometryTileFeature& feature, } for (auto& pair : paintPropertyBinders) { - pair.second.populateVertexVectors(feature, vertices.vertexSize(), patternPositions); + const auto it = patternDependencies.find(pair.first); + if (it != patternDependencies.end()){ + pair.second.populateVertexVectors(feature, vertices.vertexSize(), patternPositions, it->second); + } else { + pair.second.populateVertexVectors(feature, vertices.vertexSize(), patternPositions, {}); + } } } diff --git a/src/mbgl/renderer/buckets/fill_bucket.hpp b/src/mbgl/renderer/buckets/fill_bucket.hpp index 8f2954977a..47e273d4d2 100644 --- a/src/mbgl/renderer/buckets/fill_bucket.hpp +++ b/src/mbgl/renderer/buckets/fill_bucket.hpp @@ -29,8 +29,10 @@ public: const uint32_t overscaling); void addFeature(const GeometryTileFeature&, - const GeometryCollection&, - const mbgl::ImagePositions&) override; + const GeometryCollection&, + const mbgl::ImagePositions&, + const PatternLayerMap&) override; + bool hasData() const override; void upload(gl::Context&) override; diff --git a/src/mbgl/renderer/buckets/fill_extrusion_bucket.cpp b/src/mbgl/renderer/buckets/fill_extrusion_bucket.cpp index 98569d3f74..fe742828b8 100644 --- a/src/mbgl/renderer/buckets/fill_extrusion_bucket.cpp +++ b/src/mbgl/renderer/buckets/fill_extrusion_bucket.cpp @@ -52,7 +52,8 @@ FillExtrusionBucket::FillExtrusionBucket(const FillExtrusionBucket::PossiblyEval void FillExtrusionBucket::addFeature(const GeometryTileFeature& feature, const GeometryCollection& geometry, - const ImagePositions& patternPositions) { + const ImagePositions& patternPositions, + const PatternLayerMap& patternDependencies) { for (auto& polygon : classifyRings(geometry)) { // Optimize polygons with many interior rings for earcut tesselation. limitHoles(polygon, 500); @@ -149,7 +150,12 @@ void FillExtrusionBucket::addFeature(const GeometryTileFeature& feature, } for (auto& pair : paintPropertyBinders) { - pair.second.populateVertexVectors(feature, vertices.vertexSize(), patternPositions); + const auto it = patternDependencies.find(pair.first); + if (it != patternDependencies.end()){ + pair.second.populateVertexVectors(feature, vertices.vertexSize(), patternPositions, it->second); + } else { + pair.second.populateVertexVectors(feature, vertices.vertexSize(), patternPositions, {}); + } } } diff --git a/src/mbgl/renderer/buckets/fill_extrusion_bucket.hpp b/src/mbgl/renderer/buckets/fill_extrusion_bucket.hpp index 0da69cd749..362a9f4c99 100644 --- a/src/mbgl/renderer/buckets/fill_extrusion_bucket.hpp +++ b/src/mbgl/renderer/buckets/fill_extrusion_bucket.hpp @@ -27,8 +27,10 @@ public: const uint32_t); void addFeature(const GeometryTileFeature&, - const GeometryCollection&, - const mbgl::ImagePositions&) override; + const GeometryCollection&, + const mbgl::ImagePositions&, + const PatternLayerMap&) override; + bool hasData() const override; void upload(gl::Context&) override; diff --git a/src/mbgl/renderer/buckets/heatmap_bucket.cpp b/src/mbgl/renderer/buckets/heatmap_bucket.cpp index 1dc231bc59..7eb92a336c 100644 --- a/src/mbgl/renderer/buckets/heatmap_bucket.cpp +++ b/src/mbgl/renderer/buckets/heatmap_bucket.cpp @@ -39,8 +39,9 @@ bool HeatmapBucket::hasData() const { } void HeatmapBucket::addFeature(const GeometryTileFeature& feature, - const GeometryCollection& geometry, - const ImagePositions&) { + const GeometryCollection& geometry, + const ImagePositions&, + const PatternLayerMap&) { constexpr const uint16_t vertexLength = 4; for (auto& points : geometry) { diff --git a/src/mbgl/renderer/buckets/heatmap_bucket.hpp b/src/mbgl/renderer/buckets/heatmap_bucket.hpp index 669fdc47e3..b2d866d2a2 100644 --- a/src/mbgl/renderer/buckets/heatmap_bucket.hpp +++ b/src/mbgl/renderer/buckets/heatmap_bucket.hpp @@ -19,7 +19,8 @@ public: void addFeature(const GeometryTileFeature&, const GeometryCollection&, - const mbgl::ImagePositions&) override; + const ImagePositions&, + const PatternLayerMap&) override; bool hasData() const override; void upload(gl::Context&) override; diff --git a/src/mbgl/renderer/buckets/line_bucket.cpp b/src/mbgl/renderer/buckets/line_bucket.cpp index eedd565776..2efb60c9ac 100644 --- a/src/mbgl/renderer/buckets/line_bucket.cpp +++ b/src/mbgl/renderer/buckets/line_bucket.cpp @@ -32,13 +32,19 @@ LineBucket::LineBucket(const style::LineLayoutProperties::PossiblyEvaluated layo void LineBucket::addFeature(const GeometryTileFeature& feature, const GeometryCollection& geometryCollection, - const mbgl::ImagePositions& patternPositions) { + const ImagePositions& patternPositions, + const PatternLayerMap& patternDependencies) { for (auto& line : geometryCollection) { addGeometry(line, feature); } for (auto& pair : paintPropertyBinders) { - pair.second.populateVertexVectors(feature, vertices.vertexSize(), patternPositions); + const auto it = patternDependencies.find(pair.first); + if (it != patternDependencies.end()){ + pair.second.populateVertexVectors(feature, vertices.vertexSize(), patternPositions, it->second); + } else { + pair.second.populateVertexVectors(feature, vertices.vertexSize(), patternPositions, {}); + } } } diff --git a/src/mbgl/renderer/buckets/line_bucket.hpp b/src/mbgl/renderer/buckets/line_bucket.hpp index 86c4cdbe50..6bd37076de 100644 --- a/src/mbgl/renderer/buckets/line_bucket.hpp +++ b/src/mbgl/renderer/buckets/line_bucket.hpp @@ -30,7 +30,8 @@ public: void addFeature(const GeometryTileFeature&, const GeometryCollection&, - const mbgl::ImagePositions& patternPositions) override; + const mbgl::ImagePositions& patternPositions, + const PatternLayerMap&) override; bool hasData() const override; diff --git a/src/mbgl/renderer/paint_property_binder.hpp b/src/mbgl/renderer/paint_property_binder.hpp index e2802671b5..83427e3544 100644 --- a/src/mbgl/renderer/paint_property_binder.hpp +++ b/src/mbgl/renderer/paint_property_binder.hpp @@ -11,6 +11,7 @@ #include #include #include +#include #include @@ -83,7 +84,7 @@ public: virtual ~PaintPropertyBinder() = default; - virtual void populateVertexVector(const GeometryTileFeature& feature, std::size_t length, const ImagePositions&) = 0; + virtual void populateVertexVector(const GeometryTileFeature& feature, std::size_t length, const ImagePositions&, const PatternDependency& = {}) = 0; virtual void upload(gl::Context& context) = 0; virtual void setPatternParameters(const optional&, const optional&, CrossfadeParameters&) = 0; virtual std::tuple>...> attributeBinding(const PossiblyEvaluatedType& currentValue) const = 0; @@ -102,7 +103,7 @@ public: : constant(std::move(constant_)) { } - void populateVertexVector(const GeometryTileFeature&, std::size_t, const ImagePositions&) override {} + void populateVertexVector(const GeometryTileFeature&, std::size_t, const ImagePositions&, const PatternDependency&) override {} void upload(gl::Context&) override {} void setPatternParameters(const optional&, const optional&, CrossfadeParameters&) override {}; @@ -129,7 +130,7 @@ public: : constant(std::move(constant_)), constantPatternPositions({}) { } - void populateVertexVector(const GeometryTileFeature&, std::size_t, const ImagePositions&) override {} + void populateVertexVector(const GeometryTileFeature&, std::size_t, const ImagePositions&, const PatternDependency&) override {} void upload(gl::Context&) override {} void setPatternParameters(const optional& posA, const optional& posB, CrossfadeParameters&) override { @@ -171,7 +172,7 @@ public: defaultValue(std::move(defaultValue_)) { } void setPatternParameters(const optional&, const optional&, CrossfadeParameters&) override {}; - void populateVertexVector(const GeometryTileFeature& feature, std::size_t length, const ImagePositions&) override { + void populateVertexVector(const GeometryTileFeature& feature, std::size_t length, const ImagePositions&, const PatternDependency&) override { auto evaluated = expression.evaluate(feature, defaultValue); this->statistics.add(evaluated); auto value = attributeValue(evaluated); @@ -226,7 +227,7 @@ public: zoomRange({zoom, zoom + 1}) { } void setPatternParameters(const optional&, const optional&, CrossfadeParameters&) override {}; - void populateVertexVector(const GeometryTileFeature& feature, std::size_t length, const ImagePositions&) override { + void populateVertexVector(const GeometryTileFeature& feature, std::size_t length, const ImagePositions&, const PatternDependency&) override { Range range = expression.evaluate(zoomRange, feature, defaultValue); this->statistics.add(range.min); this->statistics.add(range.max); @@ -278,7 +279,6 @@ private: template class CompositeCrossFadedPaintPropertyBinder : public PaintPropertyBinder, PossiblyEvaluatedPropertyValue>, A1, A2> { public: - // to fix -- we will want to use both attributes using AttributeType = ZoomInterpolatedAttributeType; using AttributeType2 = ZoomInterpolatedAttributeType; @@ -301,19 +301,12 @@ public: crossfade = crossfade_; }; - void populateVertexVector(const GeometryTileFeature& feature, std::size_t length, const ImagePositions& patternPositions) override { - std::array range; - if (expression.isZoomConstant()) { - const T evaluatedValue = expression.evaluate(feature, defaultValue); - range = {{evaluatedValue, evaluatedValue, evaluatedValue}}; - } else { - range = {{expression.evaluate(zoomRange.min, feature, defaultValue), expression.evaluate(zoomRange.min, feature, defaultValue), expression.evaluate(zoomRange.min, feature, defaultValue)}}; - } + void populateVertexVector(const GeometryTileFeature&, std::size_t length, const ImagePositions& patternPositions, const PatternDependency& patternDependencies) override { if (!patternPositions.empty()) { - const auto min = patternPositions.find(range[0]); - const auto mid = patternPositions.find(range[1]); - const auto max = patternPositions.find(range[2]); + const auto min = patternPositions.find(patternDependencies.min); + const auto mid = patternPositions.find(patternDependencies.mid); + const auto max = patternPositions.find(patternDependencies.max); const auto end = patternPositions.end(); if (min == end || mid == end || max == end) return; @@ -460,9 +453,9 @@ public: PaintPropertyBinders(PaintPropertyBinders&&) = default; PaintPropertyBinders(const PaintPropertyBinders&) = delete; - void populateVertexVectors(const GeometryTileFeature& feature, std::size_t length, const ImagePositions& patternPositions) { + void populateVertexVectors(const GeometryTileFeature& feature, std::size_t length, const ImagePositions& patternPositions, const PatternDependency& patternDependencies = {}) { util::ignore({ - (binders.template get()->populateVertexVector(feature, length, patternPositions), 0)... + (binders.template get()->populateVertexVector(feature, length, patternPositions, patternDependencies), 0)... }); } diff --git a/src/mbgl/renderer/possibly_evaluated_property_value.hpp b/src/mbgl/renderer/possibly_evaluated_property_value.hpp index 85c125fa91..b49299ac3a 100644 --- a/src/mbgl/renderer/possibly_evaluated_property_value.hpp +++ b/src/mbgl/renderer/possibly_evaluated_property_value.hpp @@ -107,7 +107,7 @@ public: return this->match( [&] (const Faded& constant_) { return constant_; }, [&] (const style::PropertyExpression& expression) { - if (expression.isZoomDependent()) { + if (!expression.isZoomConstant()) { const T min = expression.evaluate(floor(zoom), feature, defaultValue); const T max = expression.evaluate(floor(zoom) + 1, feature, defaultValue); return Faded {min, max, 0.0f, 0.0f, 0.0f}; diff --git a/src/mbgl/style/layers/fill_extrusion_layer.cpp b/src/mbgl/style/layers/fill_extrusion_layer.cpp index 74cdb9abe6..9301f8dd00 100644 --- a/src/mbgl/style/layers/fill_extrusion_layer.cpp +++ b/src/mbgl/style/layers/fill_extrusion_layer.cpp @@ -443,7 +443,7 @@ optional FillExtrusionLayer::setPaintProperty(const std::string& name, co if (property == Property::FillExtrusionPattern) { Error error; - optional> typedValue = convert>(value, error, false, false); + optional> typedValue = convert>(value, error, true, false); if (!typedValue) { return error; } diff --git a/src/mbgl/style/layers/fill_layer.cpp b/src/mbgl/style/layers/fill_layer.cpp index bdfc000736..69b3a16004 100644 --- a/src/mbgl/style/layers/fill_layer.cpp +++ b/src/mbgl/style/layers/fill_layer.cpp @@ -462,7 +462,7 @@ optional FillLayer::setPaintProperty(const std::string& name, const Conve if (property == Property::FillPattern) { Error error; - optional> typedValue = convert>(value, error, false, false); + optional> typedValue = convert>(value, error, true, false); if (!typedValue) { return error; } diff --git a/src/mbgl/style/layers/line_layer.cpp b/src/mbgl/style/layers/line_layer.cpp index 1ddc690cc7..f5354e2bdb 100644 --- a/src/mbgl/style/layers/line_layer.cpp +++ b/src/mbgl/style/layers/line_layer.cpp @@ -698,7 +698,7 @@ optional LineLayer::setPaintProperty(const std::string& name, const Conve if (property == Property::LinePattern) { Error error; - optional> typedValue = convert>(value, error, false, false); + optional> typedValue = convert>(value, error, true, false); if (!typedValue) { return error; } diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index 463150e59b..cc08b33b0a 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -325,6 +325,24 @@ static std::vector> toRenderLayers(const std::vecto return renderLayers; } +// Helper function that takes a PatternLayout and either adds it to patternLayouts to await the +// availability of the imageDependencies or, if the layergroup does not use a *-pattern property, +// creates the Bucket and adds it to GeometryTileWorker::buckets. +template +void GeometryTileWorker::checkPatternLayout(std::unique_ptr> layout) { + if (layout->pattern()) { + patternLayouts.push_back(std::move(layout)); + patternNeedsLayout = true; + } else { + std::shared_ptr bucket = layout->createBucket({}, featureIndex); + if (bucket->hasData()) { + for (const auto& pair : layout->layerPaintProperties) { + buckets.emplace(pair.first, bucket); + } + } + } +} + void GeometryTileWorker::parse() { if (!data || !layers) { return; @@ -338,15 +356,7 @@ void GeometryTileWorker::parse() { } } - std::vector patternOrder; - for (auto it = layers->rbegin(); it != layers->rend(); it++) { - if ((*it)->type == LayerType::Line || (*it)->type == LayerType::Fill || (*it)->type == LayerType::FillExtrusion) { - patternOrder.push_back((*it)->id); - } - } - std::unordered_map> symbolLayoutMap; - std::unordered_map, std::unique_ptr, std::unique_ptr>> patternLayoutMap; buckets.clear(); featureIndex = std::make_unique(*data ? (*data)->clone() : nullptr); @@ -381,27 +391,27 @@ void GeometryTileWorker::parse() { } featureIndex->setBucketLayerIDs(leader.getID(), layerIDs); - if (leader.is()) { auto layout = leader.as()->createLayout( parameters, group, std::move(geometryLayer), glyphDependencies, imageDependencies); symbolLayoutMap.emplace(leader.getID(), std::move(layout)); symbolLayoutsNeedPreparation = true; } else if (leader.is()) { + // Layers that support pattern properties have an extra step at layout time to figure out what images + // are needed to render the layer. They use the intermediate PatternLayout data structure to accomplish this, + // and either immediately create a bucket if no pattern properties are used, or the PatternLayout is stored until + // the images are available to add the features to the buckets. std::unique_ptr> layout = leader.as()->createLayout( parameters, group, std::move(geometryLayer), imageDependencies); - patternLayoutMap.emplace(leader.getID(), std::move(layout)); - patternNeedsLayout = true; + checkPatternLayout(std::move(layout)); } else if (leader.is()) { std::unique_ptr> layout = leader.as()->createLayout( parameters, group, std::move(geometryLayer), imageDependencies); - patternLayoutMap.emplace(leader.getID(), std::move(layout)); - patternNeedsLayout = true; + checkPatternLayout(std::move(layout)); } else if (leader.is()) { std::unique_ptr> layout = leader.as()->createLayout( parameters, group, std::move(geometryLayer), imageDependencies); - patternLayoutMap.emplace(leader.getID(), std::move(layout)); - patternNeedsLayout = true; + checkPatternLayout(std::move(layout)); } else { const Filter& filter = leader.baseImpl->filter; const std::string& sourceLayerID = leader.baseImpl->sourceLayer; @@ -414,7 +424,7 @@ void GeometryTileWorker::parse() { continue; GeometryCollection geometries = feature->getGeometries(); - bucket->addFeature(*feature, geometries, {}); + bucket->addFeature(*feature, geometries, {}, {}); featureIndex->insert(geometries, i, sourceLayerID, leader.getID()); } @@ -436,14 +446,6 @@ void GeometryTileWorker::parse() { } } - patternLayouts.clear(); - for (const auto& patternLayerID : patternOrder) { - auto it = patternLayoutMap.find(patternLayerID); - if (it != patternLayoutMap.end()) { - patternLayouts.push_back(std::move(it->second)); - } - } - requestNewGlyphs(glyphDependencies); requestNewImages(imageDependencies); @@ -526,6 +528,7 @@ void GeometryTileWorker::performSymbolLayout() { ); } patternNeedsLayout = false; + patternLayouts.clear(); for (auto& symbolLayout : symbolLayouts) { if (obsolete) { @@ -559,5 +562,4 @@ void GeometryTileWorker::performSymbolLayout() { std::move(iconAtlas) }, correlationID); } - } // namespace mbgl diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp index 3f0d435a47..616d1a3625 100644 --- a/src/mbgl/tile/geometry_tile_worker.hpp +++ b/src/mbgl/tile/geometry_tile_worker.hpp @@ -63,6 +63,9 @@ private: bool hasPendingSymbolDependencies() const; bool hasPendingParseResult() const; + template + void checkPatternLayout(std::unique_ptr> layout); + ActorRef self; ActorRef parent; diff --git a/test/gl/bucket.test.cpp b/test/gl/bucket.test.cpp index c5be975ee5..0f34eb3b8c 100644 --- a/test/gl/bucket.test.cpp +++ b/test/gl/bucket.test.cpp @@ -52,7 +52,7 @@ TEST(Buckets, CircleBucket) { ASSERT_FALSE(bucket.needsUpload()); GeometryCollection point { { { 0, 0 } } }; - bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Point, point, properties }, point, {}); + bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Point, point, properties }, point, {}, {}); ASSERT_TRUE(bucket.hasData()); ASSERT_TRUE(bucket.needsUpload()); @@ -72,7 +72,7 @@ TEST(Buckets, FillBucket) { ASSERT_FALSE(bucket.needsUpload()); GeometryCollection polygon { { { 0, 0 }, { 0, 1 }, { 1, 1 } } }; - bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Polygon, polygon, properties }, polygon, {}); + bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Polygon, polygon, properties }, polygon, {}, {}); ASSERT_TRUE(bucket.hasData()); ASSERT_TRUE(bucket.needsUpload()); @@ -92,11 +92,11 @@ TEST(Buckets, LineBucket) { // Ignore invalid feature type. GeometryCollection point { { { 0, 0 } } }; - bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Point, point, properties }, point, {}); + bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Point, point, properties }, point, {}, {}); ASSERT_FALSE(bucket.hasData()); GeometryCollection line { { { 0, 0 }, { 1, 1 } } }; - bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::LineString, line, properties }, line, {}); + bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::LineString, line, properties }, line, {}, {}); ASSERT_TRUE(bucket.hasData()); ASSERT_TRUE(bucket.needsUpload()); @@ -125,7 +125,7 @@ TEST(Buckets, SymbolBucket) { // SymbolBucket::addFeature() is a no-op. GeometryCollection point { { { 0, 0 } } }; - bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Point, point, properties }, point, {}); + bucket.addFeature(StubGeometryTileFeature { {}, FeatureType::Point, point, properties }, point, {}, {}); ASSERT_FALSE(bucket.hasData()); ASSERT_FALSE(bucket.needsUpload()); -- cgit v1.2.1