Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[core] shader program must always match bucket in render symbol layer
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@pozdnyakov
pozdnyakov committed Jan 10, 2019
1 parent 50f5bbc commit e01182c
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 53 deletions.
5 changes: 1 addition & 4 deletions src/mbgl/layout/symbol_layout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/mbgl/layout/symbol_layout.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ class SymbolLayout : public Layout {
bool hasSymbolInstances() const override;
bool hasDependencies() const override;

std::map<std::string,
std::pair<style::IconPaintProperties::PossiblyEvaluated, style::TextPaintProperties::PossiblyEvaluated>> layerPaintProperties;
std::map<std::string, style::SymbolPaintProperties::PossiblyEvaluated> layerPaintProperties;

const std::string bucketLeaderID;
std::vector<SymbolInstance> symbolInstances;
Expand Down
11 changes: 5 additions & 6 deletions src/mbgl/renderer/buckets/symbol_bucket.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ namespace mbgl {
using namespace style;

SymbolBucket::SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated layout_,
const std::map<std::string, std::pair<
style::IconPaintProperties::PossiblyEvaluated,
style::TextPaintProperties::PossiblyEvaluated>>& layerPaintProperties,
std::map<std::string, style::SymbolPaintProperties::PossiblyEvaluated> paintProperties_,
const style::PropertyValue<float>& textSize,
const style::PropertyValue<float>& iconSize,
float zoom,
Expand All @@ -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)));
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/mbgl/renderer/buckets/symbol_bucket.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class PlacedSymbol {
class SymbolBucket final : public Bucket {
public:
SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated,
const std::map<std::string, std::pair<style::IconPaintProperties::PossiblyEvaluated, style::TextPaintProperties::PossiblyEvaluated>>&,
std::map<std::string, style::SymbolPaintProperties::PossiblyEvaluated>,
const style::PropertyValue<float>& textSize,
const style::PropertyValue<float>& iconSize,
float zoom,
Expand Down Expand Up @@ -78,6 +78,8 @@ class SymbolBucket final : public Bucket {

std::vector<SymbolInstance> symbolInstances;

std::map<std::string, style::SymbolPaintProperties::PossiblyEvaluated> paintProperties;

std::map<std::string, std::pair<
SymbolIconProgram::PaintPropertyBinders,
SymbolSDFTextProgram::PaintPropertyBinders>> paintPropertyBinders;
Expand Down
78 changes: 44 additions & 34 deletions src/mbgl/renderer/layers/render_symbol_layer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -152,8 +153,8 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) {
GeometryTile& geometryTile = static_cast<GeometryTile&>(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<SymbolPlacement>() != SymbolPlacementType::Point &&
layout.get<IconRotationAlignment>() == AlignmentType::Map;
Expand Down Expand Up @@ -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<SymbolPlacement>() != SymbolPlacementType::Point &&
layout.get<TextRotationAlignment>() == AlignmentType::Map;
Expand Down Expand Up @@ -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<style::IconOpacity>(),
evaluated.get<style::IconColor>(),
evaluated.get<style::IconHaloColor>(),
evaluated.get<style::IconHaloWidth>(),
evaluated.get<style::IconHaloBlur>(),
evaluated.get<style::IconTranslate>(),
evaluated.get<style::IconTranslateAnchor>()
evaluated_.get<style::IconOpacity>(),
evaluated_.get<style::IconColor>(),
evaluated_.get<style::IconHaloColor>(),
evaluated_.get<style::IconHaloWidth>(),
evaluated_.get<style::IconHaloBlur>(),
evaluated_.get<style::IconTranslate>(),
evaluated_.get<style::IconTranslateAnchor>()
};
}

style::TextPaintProperties::PossiblyEvaluated RenderSymbolLayer::textPaintProperties() const {
// static
style::TextPaintProperties::PossiblyEvaluated RenderSymbolLayer::textPaintProperties(const style::SymbolPaintProperties::PossiblyEvaluated& evaluated_) {
return style::TextPaintProperties::PossiblyEvaluated {
evaluated.get<style::TextOpacity>(),
evaluated.get<style::TextColor>(),
evaluated.get<style::TextHaloColor>(),
evaluated.get<style::TextHaloWidth>(),
evaluated.get<style::TextHaloBlur>(),
evaluated.get<style::TextTranslate>(),
evaluated.get<style::TextTranslateAnchor>()
evaluated_.get<style::TextOpacity>(),
evaluated_.get<style::TextColor>(),
evaluated_.get<style::TextHaloColor>(),
evaluated_.get<style::TextHaloWidth>(),
evaluated_.get<style::TextHaloBlur>(),
evaluated_.get<style::TextTranslate>(),
evaluated_.get<style::TextTranslateAnchor>()
};
}


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<style::IconPitchAlignment>(),
layout_.get<style::IconRotationAlignment>(),
layout_.get<style::IconKeepUpright>(),
evaluated.get<style::IconTranslate>(),
evaluated.get<style::IconTranslateAnchor>(),
evaluated.get<style::IconHaloColor>().constantOr(Color::black()).a > 0 &&
evaluated.get<style::IconHaloWidth>().constantOr(1),
evaluated.get<style::IconColor>().constantOr(Color::black()).a > 0
evaluated_.get<style::IconTranslate>(),
evaluated_.get<style::IconTranslateAnchor>(),
evaluated_.get<style::IconHaloColor>().constantOr(Color::black()).a > 0 &&
evaluated_.get<style::IconHaloWidth>().constantOr(1),
evaluated_.get<style::IconColor>().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<style::TextPitchAlignment>(),
layout_.get<style::TextRotationAlignment>(),
layout_.get<style::TextKeepUpright>(),
evaluated.get<style::TextTranslate>(),
evaluated.get<style::TextTranslateAnchor>(),
evaluated.get<style::TextHaloColor>().constantOr(Color::black()).a > 0 &&
evaluated.get<style::TextHaloWidth>().constantOr(1),
evaluated.get<style::TextColor>().constantOr(Color::black()).a > 0
evaluated_.get<style::TextTranslate>(),
evaluated_.get<style::TextTranslateAnchor>(),
evaluated_.get<style::TextHaloColor>().constantOr(Color::black()).a > 0 &&
evaluated_.get<style::TextHaloWidth>().constantOr(1),
evaluated_.get<style::TextColor>().constantOr(Color::black()).a > 0
};
}

Expand All @@ -398,4 +404,8 @@ void RenderSymbolLayer::sortRenderTiles(const TransformState& state) {
});
}

void RenderSymbolLayer::updateBucketPaintProperties(Bucket* bucket) const {
static_cast<SymbolBucket*>(bucket)->paintProperties[getID()] = evaluated;
}

} // namespace mbgl
12 changes: 7 additions & 5 deletions src/mbgl/renderer/layers/render_symbol_layer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,8 @@ class RenderSymbolLayer: public RenderLayer, public RenderLayerSymbolInterface {
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<Bucket> createBucket(const BucketParameters&, const std::vector<const RenderLayer*>&) const override;
std::unique_ptr<Layout> createLayout(const BucketParameters&,
Expand All @@ -97,8 +94,13 @@ class RenderSymbolLayer: public RenderLayer, public RenderLayerSymbolInterface {
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) {
Expand Down
9 changes: 8 additions & 1 deletion src/mbgl/renderer/render_layer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions src/mbgl/renderer/render_layer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ class RenderLayer {
// 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;

Expand Down

0 comments on commit e01182c

Please sign in to comment.