From 2bdab82dedf545b4272c534b067c68614d0355b7 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 3 Jan 2019 17:17:57 +0200 Subject: [core] shader program must always match bucket in render symbol layer Before this change, `RenderSymbolLayer` with updated style was trying to render symbols using the previous bucket (with paint property binders that matched a previous program). Now, symbol bucket caches the latest corresponding paint properties (caching is happening on complete tiles only). As a result, `RenderSymbolLayer` always picks the shader program and its parameters in sync with the obtained bucket. --- src/mbgl/layout/symbol_layout.cpp | 5 +- src/mbgl/layout/symbol_layout.hpp | 3 +- src/mbgl/renderer/buckets/symbol_bucket.cpp | 11 ++-- src/mbgl/renderer/buckets/symbol_bucket.hpp | 4 +- src/mbgl/renderer/layers/render_symbol_layer.cpp | 78 +++++++++++++----------- src/mbgl/renderer/layers/render_symbol_layer.hpp | 12 ++-- src/mbgl/renderer/render_layer.cpp | 9 ++- src/mbgl/renderer/render_layer.hpp | 4 ++ 8 files changed, 73 insertions(+), 53 deletions(-) diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index 332fb3f46a..4acd0a1ccd 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -90,10 +90,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters, } for (const auto& layer : layers) { - layerPaintProperties.emplace(layer->getID(), std::make_pair( - toRenderSymbolLayer(layer)->iconPaintProperties(), - toRenderSymbolLayer(layer)->textPaintProperties() - )); + layerPaintProperties.emplace(layer->getID(), toRenderSymbolLayer(layer)->evaluated); } // Determine glyph dependencies diff --git a/src/mbgl/layout/symbol_layout.hpp b/src/mbgl/layout/symbol_layout.hpp index 7045eebc22..ab6dc049a2 100644 --- a/src/mbgl/layout/symbol_layout.hpp +++ b/src/mbgl/layout/symbol_layout.hpp @@ -44,8 +44,7 @@ public: bool hasSymbolInstances() const override; bool hasDependencies() const override; - std::map> layerPaintProperties; + std::map layerPaintProperties; const std::string bucketLeaderID; std::vector symbolInstances; diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp index 93870b287b..4947b6b521 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.cpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp @@ -9,9 +9,7 @@ namespace mbgl { using namespace style; SymbolBucket::SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated layout_, - const std::map>& layerPaintProperties, + std::map paintProperties_, const style::PropertyValue& textSize, const style::PropertyValue& iconSize, float zoom, @@ -26,17 +24,18 @@ SymbolBucket::SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated layo sortFeaturesByY(sortFeaturesByY_), bucketLeaderID(std::move(bucketName_)), symbolInstances(std::move(symbolInstances_)), + paintProperties(std::move(paintProperties_)), textSizeBinder(SymbolSizeBinder::create(zoom, textSize, TextSize::defaultValue())), iconSizeBinder(SymbolSizeBinder::create(zoom, iconSize, IconSize::defaultValue())) { - for (const auto& pair : layerPaintProperties) { + for (const auto& pair : paintProperties) { paintPropertyBinders.emplace( std::piecewise_construct, std::forward_as_tuple(pair.first), std::forward_as_tuple( std::piecewise_construct, - std::forward_as_tuple(pair.second.first, zoom), - std::forward_as_tuple(pair.second.second, zoom))); + std::forward_as_tuple(RenderSymbolLayer::iconPaintProperties(pair.second), zoom), + std::forward_as_tuple(RenderSymbolLayer::textPaintProperties(pair.second), zoom))); } } diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp index ccb2cce513..76fab2fa0f 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.hpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp @@ -40,7 +40,7 @@ public: class SymbolBucket final : public Bucket { public: SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated, - const std::map>&, + std::map, const style::PropertyValue& textSize, const style::PropertyValue& iconSize, float zoom, @@ -78,6 +78,8 @@ public: std::vector symbolInstances; + std::map paintProperties; + std::map> paintPropertyBinders; diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp index 11ffd74b10..716b06c24d 100644 --- a/src/mbgl/renderer/layers/render_symbol_layer.cpp +++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp @@ -101,7 +101,8 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) { continue; } SymbolBucket& bucket = *bucket_; - + assert(bucket.paintProperties.find(getID()) != bucket.paintProperties.end()); + const auto& evaluated_ = bucket.paintProperties.at(getID()); const auto& layout = bucket.layout; auto draw = [&] (auto& program, @@ -152,8 +153,8 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) { GeometryTile& geometryTile = static_cast(tile.tile); if (bucket.hasIconData()) { - auto values = iconPropertyValues(layout); - auto paintPropertyValues = iconPaintProperties(); + auto values = iconPropertyValues(evaluated_, layout); + const auto& paintPropertyValues = iconPaintProperties(evaluated_); const bool alongLine = layout.get() != SymbolPlacementType::Point && layout.get() == AlignmentType::Map; @@ -213,8 +214,8 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) { if (bucket.hasTextData()) { parameters.context.bindTexture(*geometryTile.glyphAtlasTexture, 0, gl::TextureFilter::Linear); - auto values = textPropertyValues(layout); - auto paintPropertyValues = textPaintProperties(); + auto values = textPropertyValues(evaluated_, layout); + const auto& paintPropertyValues = textPaintProperties(evaluated_); const bool alongLine = layout.get() != SymbolPlacementType::Point && layout.get() == AlignmentType::Map; @@ -328,54 +329,59 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) { } } -style::IconPaintProperties::PossiblyEvaluated RenderSymbolLayer::iconPaintProperties() const { +// static +style::IconPaintProperties::PossiblyEvaluated RenderSymbolLayer::iconPaintProperties(const style::SymbolPaintProperties::PossiblyEvaluated& evaluated_) { return style::IconPaintProperties::PossiblyEvaluated { - evaluated.get(), - evaluated.get(), - evaluated.get(), - evaluated.get(), - evaluated.get(), - evaluated.get(), - evaluated.get() + evaluated_.get(), + evaluated_.get(), + evaluated_.get(), + evaluated_.get(), + evaluated_.get(), + evaluated_.get(), + evaluated_.get() }; } -style::TextPaintProperties::PossiblyEvaluated RenderSymbolLayer::textPaintProperties() const { +// static +style::TextPaintProperties::PossiblyEvaluated RenderSymbolLayer::textPaintProperties(const style::SymbolPaintProperties::PossiblyEvaluated& evaluated_) { return style::TextPaintProperties::PossiblyEvaluated { - evaluated.get(), - evaluated.get(), - evaluated.get(), - evaluated.get(), - evaluated.get(), - evaluated.get(), - evaluated.get() + evaluated_.get(), + evaluated_.get(), + evaluated_.get(), + evaluated_.get(), + evaluated_.get(), + evaluated_.get(), + evaluated_.get() }; } - -style::SymbolPropertyValues RenderSymbolLayer::iconPropertyValues(const style::SymbolLayoutProperties::PossiblyEvaluated& layout_) const { +// static +style::SymbolPropertyValues RenderSymbolLayer::iconPropertyValues(const style::SymbolPaintProperties::PossiblyEvaluated& evaluated_, + const style::SymbolLayoutProperties::PossiblyEvaluated& layout_) { return style::SymbolPropertyValues { layout_.get(), layout_.get(), layout_.get(), - evaluated.get(), - evaluated.get(), - evaluated.get().constantOr(Color::black()).a > 0 && - evaluated.get().constantOr(1), - evaluated.get().constantOr(Color::black()).a > 0 + evaluated_.get(), + evaluated_.get(), + evaluated_.get().constantOr(Color::black()).a > 0 && + evaluated_.get().constantOr(1), + evaluated_.get().constantOr(Color::black()).a > 0 }; } -style::SymbolPropertyValues RenderSymbolLayer::textPropertyValues(const style::SymbolLayoutProperties::PossiblyEvaluated& layout_) const { +// static +style::SymbolPropertyValues RenderSymbolLayer::textPropertyValues(const style::SymbolPaintProperties::PossiblyEvaluated& evaluated_, + const style::SymbolLayoutProperties::PossiblyEvaluated& layout_) { return style::SymbolPropertyValues { layout_.get(), layout_.get(), layout_.get(), - evaluated.get(), - evaluated.get(), - evaluated.get().constantOr(Color::black()).a > 0 && - evaluated.get().constantOr(1), - evaluated.get().constantOr(Color::black()).a > 0 + evaluated_.get(), + evaluated_.get(), + evaluated_.get().constantOr(Color::black()).a > 0 && + evaluated_.get().constantOr(1), + evaluated_.get().constantOr(Color::black()).a > 0 }; } @@ -398,4 +404,8 @@ void RenderSymbolLayer::sortRenderTiles(const TransformState& state) { }); } +void RenderSymbolLayer::updateBucketPaintProperties(Bucket* bucket) const { + static_cast(bucket)->paintProperties[getID()] = evaluated; +} + } // namespace mbgl diff --git a/src/mbgl/renderer/layers/render_symbol_layer.hpp b/src/mbgl/renderer/layers/render_symbol_layer.hpp index 832fb94001..6b43f91a22 100644 --- a/src/mbgl/renderer/layers/render_symbol_layer.hpp +++ b/src/mbgl/renderer/layers/render_symbol_layer.hpp @@ -68,11 +68,8 @@ public: bool hasCrossfade() const override; void render(PaintParameters&, RenderSource*) override; - style::IconPaintProperties::PossiblyEvaluated iconPaintProperties() const; - style::TextPaintProperties::PossiblyEvaluated textPaintProperties() const; - - style::SymbolPropertyValues iconPropertyValues(const style::SymbolLayoutProperties::PossiblyEvaluated&) const; - style::SymbolPropertyValues textPropertyValues(const style::SymbolLayoutProperties::PossiblyEvaluated&) const; + static style::IconPaintProperties::PossiblyEvaluated iconPaintProperties(const style::SymbolPaintProperties::PossiblyEvaluated&); + static style::TextPaintProperties::PossiblyEvaluated textPaintProperties(const style::SymbolPaintProperties::PossiblyEvaluated&); std::unique_ptr createBucket(const BucketParameters&, const std::vector&) const override; std::unique_ptr createLayout(const BucketParameters&, @@ -97,8 +94,13 @@ public: const style::SymbolLayer::Impl& impl() const; protected: + static style::SymbolPropertyValues iconPropertyValues(const style::SymbolPaintProperties::PossiblyEvaluated&, + const style::SymbolLayoutProperties::PossiblyEvaluated&); + static style::SymbolPropertyValues textPropertyValues(const style::SymbolPaintProperties::PossiblyEvaluated&, + const style::SymbolLayoutProperties::PossiblyEvaluated&); RenderTiles filterRenderTiles(RenderTiles) const final; void sortRenderTiles(const TransformState&) final; + void updateBucketPaintProperties(Bucket*) const final; }; inline const RenderSymbolLayer* toRenderSymbolLayer(const RenderLayer* layer) { diff --git a/src/mbgl/renderer/render_layer.cpp b/src/mbgl/renderer/render_layer.cpp index 033df02282..c2c57deb22 100644 --- a/src/mbgl/renderer/render_layer.cpp +++ b/src/mbgl/renderer/render_layer.cpp @@ -71,10 +71,13 @@ RenderLayer::RenderTiles RenderLayer::filterRenderTiles(RenderTiles tiles, Filte continue; } - if (tile.tile.getBucket(*baseImpl)) { + if (Bucket* bucket = tile.tile.getBucket(*baseImpl)) { tile.used = true; tile.needsClipping |= needsClipping_; filtered.emplace_back(tile); + if (tile.tile.isComplete()) { + updateBucketPaintProperties(bucket); + } } } return filtered; @@ -84,6 +87,10 @@ void RenderLayer::markContextDestroyed() { // no-op } +void RenderLayer::updateBucketPaintProperties(Bucket*) const { + // no-op +} + void RenderLayer::checkRenderability(const PaintParameters& parameters, const uint32_t activeBindingCount) { // Only warn once for every layer. diff --git a/src/mbgl/renderer/render_layer.hpp b/src/mbgl/renderer/render_layer.hpp index 485e5551c6..758412f932 100644 --- a/src/mbgl/renderer/render_layer.hpp +++ b/src/mbgl/renderer/render_layer.hpp @@ -96,6 +96,10 @@ protected: // Code specific to RenderTiles sorting / filtering virtual RenderTiles filterRenderTiles(RenderTiles) const; virtual void sortRenderTiles(const TransformState&); + // For some layers, we want Buckets to cache their corresponding paint properties, so that outdated buckets (and + // the cached paint properties) can be still in use while the tile is loading new buckets (which will + // correpond to the current paint properties of the layer). + virtual void updateBucketPaintProperties(Bucket*) const; using FilterFunctionPtr = bool (*)(RenderTile&); RenderTiles filterRenderTiles(RenderTiles, FilterFunctionPtr) const; -- cgit v1.2.1