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

[core] Decrease size of SymbolBucket #15178

Merged
merged 3 commits into from
Jul 22, 2019
Merged

[core] Decrease size of SymbolBucket #15178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
89efc2a
[core] sizeof(SymbolBucket) 2312 -> 2296
pozdnyakov Jul 19, 2019
a43b807
[core] Symbol bucket uses shared layout
pozdnyakov Jul 21, 2019
1fc1b88
[core] SymbolBacket creates collision buffers optionally
pozdnyakov Jul 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 85 additions & 81 deletions src/mbgl/layout/symbol_layout.cpp
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions src/mbgl/layout/symbol_layout.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ class SymbolLayout final : public Layout {
const MapMode mode;
const float pixelRatio;

style::SymbolLayoutProperties::PossiblyEvaluated layout;

const uint32_t tileSize;
const float tilePixelRatio;

Expand All @@ -101,7 +99,7 @@ class SymbolLayout final : public Layout {

style::TextSize::UnevaluatedType textSize;
style::IconSize::UnevaluatedType iconSize;

Immutable<style::SymbolLayoutProperties::PossiblyEvaluated> layout;
std::vector<SymbolFeature> features;

BiDi bidi; // Consider moving this up to geometry tile worker to reduce reinstantiation costs; use of BiDi/ubiditransform object must be constrained to one thread
Expand Down
39 changes: 23 additions & 16 deletions src/mbgl/renderer/buckets/symbol_bucket.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace {
std::atomic<uint32_t> maxBucketInstanceId;
} // namespace

SymbolBucket::SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated layout_,
SymbolBucket::SymbolBucket(Immutable<style::SymbolLayoutProperties::PossiblyEvaluated> layout_,
const std::map<std::string, Immutable<style::LayerProperties>>& paintProperties_,
const style::PropertyValue<float>& textSize,
const style::PropertyValue<float>& iconSize,
Expand All @@ -25,10 +25,16 @@ SymbolBucket::SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated layo
const std::vector<SymbolInstance>&& symbolInstances_,
float tilePixelRatio_)
: layout(std::move(layout_)),
bucketLeaderID(std::move(bucketName_)),
sdfIcons(sdfIcons_),
iconsNeedLinear(iconsNeedLinear_ || iconSize.isDataDriven() || !iconSize.isZoomConstant()),
sortFeaturesByY(sortFeaturesByY_),
bucketLeaderID(std::move(bucketName_)),
staticUploaded(false),
placementChangesUploaded(false),
dynamicUploaded(false),
sortUploaded(false),
justReloaded(false),
hasVariablePlacement(false),
symbolInstances(std::move(symbolInstances_)),
textSizeBinder(SymbolSizeBinder::create(zoom, textSize, TextSize::defaultValue())),
iconSizeBinder(SymbolSizeBinder::create(zoom, iconSize, IconSize::defaultValue())),
Expand Down Expand Up @@ -105,28 +111,28 @@ void SymbolBucket::upload(gfx::UploadPass& uploadPass) {

if (hasCollisionBoxData()) {
if (!staticUploaded) {
collisionBox.indexBuffer = uploadPass.createIndexBuffer(std::move(collisionBox.lines));
collisionBox.vertexBuffer = uploadPass.createVertexBuffer(std::move(collisionBox.vertices));
collisionBox->indexBuffer = uploadPass.createIndexBuffer(std::move(collisionBox->lines));
collisionBox->vertexBuffer = uploadPass.createVertexBuffer(std::move(collisionBox->vertices));
}
if (!placementChangesUploaded) {
if (!collisionBox.dynamicVertexBuffer) {
collisionBox.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(collisionBox.dynamicVertices), gfx::BufferUsageType::StreamDraw);
if (!collisionBox->dynamicVertexBuffer) {
collisionBox->dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(collisionBox->dynamicVertices), gfx::BufferUsageType::StreamDraw);
} else {
uploadPass.updateVertexBuffer(*collisionBox.dynamicVertexBuffer, std::move(collisionBox.dynamicVertices));
uploadPass.updateVertexBuffer(*collisionBox->dynamicVertexBuffer, std::move(collisionBox->dynamicVertices));
}
}
}

if (hasCollisionCircleData()) {
if (!staticUploaded) {
collisionCircle.indexBuffer = uploadPass.createIndexBuffer(std::move(collisionCircle.triangles));
collisionCircle.vertexBuffer = uploadPass.createVertexBuffer(std::move(collisionCircle.vertices));
collisionCircle->indexBuffer = uploadPass.createIndexBuffer(std::move(collisionCircle->triangles));
collisionCircle->vertexBuffer = uploadPass.createVertexBuffer(std::move(collisionCircle->vertices));
}
if (!placementChangesUploaded) {
if (!collisionCircle.dynamicVertexBuffer) {
collisionCircle.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(collisionCircle.dynamicVertices), gfx::BufferUsageType::StreamDraw);
if (!collisionCircle->dynamicVertexBuffer) {
collisionCircle->dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(collisionCircle->dynamicVertices), gfx::BufferUsageType::StreamDraw);
} else {
uploadPass.updateVertexBuffer(*collisionCircle.dynamicVertexBuffer, std::move(collisionCircle.dynamicVertices));
uploadPass.updateVertexBuffer(*collisionCircle->dynamicVertexBuffer, std::move(collisionCircle->dynamicVertices));
}
}
}
Expand All @@ -151,11 +157,11 @@ bool SymbolBucket::hasIconData() const {
}

bool SymbolBucket::hasCollisionBoxData() const {
return !collisionBox.segments.empty();
return collisionBox && !collisionBox->segments.empty();
}

bool SymbolBucket::hasCollisionCircleData() const {
return !collisionCircle.segments.empty();
return collisionCircle && !collisionCircle->segments.empty();
}

void addPlacedSymbol(gfx::IndexVector<gfx::Triangles>& triangles, const PlacedSymbol& placedSymbol) {
Expand All @@ -170,8 +176,9 @@ void SymbolBucket::sortFeatures(const float angle) {
if (!sortFeaturesByY) {
return;
}
assert(angle != std::numeric_limits<float>::max());

if (sortedAngle && *sortedAngle == angle) {
if (sortedAngle == angle) {
return;
}

Expand Down Expand Up @@ -239,7 +246,7 @@ std::vector<std::reference_wrapper<SymbolInstance>> SymbolBucket::getSortedSymbo

bool SymbolBucket::hasFormatSectionOverrides() const {
if (!hasFormatSectionOverrides_) {
hasFormatSectionOverrides_= SymbolLayerPaintPropertyOverrides::hasOverrides(layout.get<TextField>());
hasFormatSectionOverrides_= SymbolLayerPaintPropertyOverrides::hasOverrides(layout->get<TextField>());
}
return *hasFormatSectionOverrides_;
}
Expand Down
46 changes: 29 additions & 17 deletions src/mbgl/renderer/buckets/symbol_bucket.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class PlacedSymbol {

class SymbolBucket final : public Bucket {
public:
SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated,
SymbolBucket(Immutable<style::SymbolLayoutProperties::PossiblyEvaluated>,
const std::map<std::string, Immutable<style::LayerProperties>>&,
const style::PropertyValue<float>& textSize,
const style::PropertyValue<float>& iconSize,
Expand Down Expand Up @@ -72,19 +72,20 @@ class SymbolBucket final : public Bucket {
// The result contains references to the `symbolInstances` items, sorted by viewport Y.
std::vector<std::reference_wrapper<SymbolInstance>> getSortedSymbols(const float angle);

const style::SymbolLayoutProperties::PossiblyEvaluated layout;
const bool sdfIcons;
const bool iconsNeedLinear;
const bool sortFeaturesByY;

Immutable<style::SymbolLayoutProperties::PossiblyEvaluated> layout;
const std::string bucketLeaderID;

optional<float> sortedAngle;

bool staticUploaded = false;
bool placementChangesUploaded = false;
bool dynamicUploaded = false;
bool sortUploaded = false;
float sortedAngle = std::numeric_limits<float>::max();

// Flags
const bool sdfIcons : 1;
const bool iconsNeedLinear : 1;
const bool sortFeaturesByY : 1;
bool staticUploaded : 1;
bool placementChangesUploaded : 1;
bool dynamicUploaded : 1;
bool sortUploaded : 1;
bool justReloaded : 1;
bool hasVariablePlacement : 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too bad we have 9 bits :-|


std::vector<SymbolInstance> symbolInstances;

Expand Down Expand Up @@ -128,17 +129,28 @@ class SymbolBucket final : public Bucket {
struct CollisionBoxBuffer : public CollisionBuffer {
gfx::IndexVector<gfx::Lines> lines;
optional<gfx::IndexBuffer> indexBuffer;
} collisionBox;
};
std::unique_ptr<CollisionBoxBuffer> collisionBox;

CollisionBoxBuffer& getOrCreateCollisionBox() {
if (!collisionBox) collisionBox = std::make_unique<CollisionBoxBuffer>();
return *collisionBox;
}

struct CollisionCircleBuffer : public CollisionBuffer {
gfx::IndexVector<gfx::Triangles> triangles;
optional<gfx::IndexBuffer> indexBuffer;
} collisionCircle;
};
std::unique_ptr<CollisionCircleBuffer> collisionCircle;

CollisionCircleBuffer& getOrCreateCollisionCircleBuffer() {
if (!collisionCircle) collisionCircle = std::make_unique<CollisionCircleBuffer>();
return *collisionCircle;
}

const float tilePixelRatio;
uint32_t bucketInstanceId;
bool justReloaded = false;
bool hasVariablePlacement = false;

mutable optional<bool> hasFormatSectionOverrides_;

std::shared_ptr<std::vector<size_t>> featureSortOrder;
Expand Down
20 changes: 10 additions & 10 deletions src/mbgl/renderer/layers/render_symbol_layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ void drawIcon(const DrawFn& draw,
const PaintParameters& parameters) {
auto& bucket = static_cast<SymbolBucket&>(*renderData.bucket);
const auto& evaluated = getEvaluated<SymbolLayerProperties>(renderData.layerProperties);
const auto& layout = bucket.layout;
const auto& layout = *bucket.layout;
auto values = iconPropertyValues(evaluated, layout);
const auto& paintPropertyValues = RenderSymbolLayer::iconPaintProperties(evaluated);

Expand Down Expand Up @@ -184,7 +184,7 @@ void drawText(const DrawFn& draw,
const PaintParameters& parameters) {
auto& bucket = static_cast<SymbolBucket&>(*renderData.bucket);
const auto& evaluated = getEvaluated<SymbolLayerProperties>(renderData.layerProperties);
const auto& layout = bucket.layout;
const auto& layout = *bucket.layout;

const gfx::TextureBinding textureBinding{ tile.getGlyphAtlasTexture().getResource(),
gfx::TextureFilterType::Linear };
Expand Down Expand Up @@ -405,10 +405,10 @@ void RenderSymbolLayer::render(PaintParameters& parameters) {
uniforms::extrude_scale::Value( extrudeScale ),
uniforms::camera_to_center_distance::Value( parameters.state.getCameraToCenterDistance() )
},
*bucket.collisionBox.vertexBuffer,
*bucket.collisionBox.dynamicVertexBuffer,
*bucket.collisionBox.indexBuffer,
bucket.collisionBox.segments,
*bucket.collisionBox->vertexBuffer,
*bucket.collisionBox->dynamicVertexBuffer,
*bucket.collisionBox->indexBuffer,
bucket.collisionBox->segments,
paintAttributeData,
properties,
CollisionBoxProgram::TextureBindings{},
Expand Down Expand Up @@ -443,10 +443,10 @@ void RenderSymbolLayer::render(PaintParameters& parameters) {
uniforms::overscale_factor::Value( float(tile.getOverscaledTileID().overscaleFactor()) ),
uniforms::camera_to_center_distance::Value( parameters.state.getCameraToCenterDistance() )
},
*bucket.collisionCircle.vertexBuffer,
*bucket.collisionCircle.dynamicVertexBuffer,
*bucket.collisionCircle.indexBuffer,
bucket.collisionCircle.segments,
*bucket.collisionCircle->vertexBuffer,
*bucket.collisionCircle->dynamicVertexBuffer,
*bucket.collisionCircle->indexBuffer,
bucket.collisionCircle->segments,
paintAttributeData,
properties,
CollisionCircleProgram::TextureBindings{},
Expand Down
34 changes: 17 additions & 17 deletions src/mbgl/text/placement.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ void Placement::placeBucket(
SymbolBucket& bucket,
const BucketPlacementParameters& params,
std::set<uint32_t>& seenCrossTileIDs) {
const auto& layout = bucket.layout;
const auto& layout = *bucket.layout;
const auto& renderTile = params.tile;
const auto& state = collisionIndex.getTransformState();
const float pixelsToTileUnits = renderTile.id.pixelsToTileUnits(1, state.getZoom());
Expand Down Expand Up @@ -409,7 +409,7 @@ Point<float> calculateVariableRenderShift(style::SymbolAnchorType anchor, float

bool Placement::updateBucketDynamicVertices(SymbolBucket& bucket, const TransformState& state, const RenderTile& tile) {
using namespace style;
const auto& layout = bucket.layout;
const auto& layout = *bucket.layout;
const bool alongLine = layout.get<SymbolPlacement>() != SymbolPlacementType::Point;
bool result = false;

Expand Down Expand Up @@ -507,24 +507,24 @@ bool Placement::updateBucketDynamicVertices(SymbolBucket& bucket, const Transfor
void Placement::updateBucketOpacities(SymbolBucket& bucket, const TransformState& state, std::set<uint32_t>& seenCrossTileIDs) {
if (bucket.hasTextData()) bucket.text.opacityVertices.clear();
if (bucket.hasIconData()) bucket.icon.opacityVertices.clear();
if (bucket.hasCollisionBoxData()) bucket.collisionBox.dynamicVertices.clear();
if (bucket.hasCollisionCircleData()) bucket.collisionCircle.dynamicVertices.clear();
if (bucket.hasCollisionBoxData()) bucket.collisionBox->dynamicVertices.clear();
if (bucket.hasCollisionCircleData()) bucket.collisionCircle->dynamicVertices.clear();

JointOpacityState duplicateOpacityState(false, false, true);

const bool textAllowOverlap = bucket.layout.get<style::TextAllowOverlap>();
const bool iconAllowOverlap = bucket.layout.get<style::IconAllowOverlap>();
const bool variablePlacement = !bucket.layout.get<style::TextVariableAnchor>().empty();
const bool rotateWithMap = bucket.layout.get<style::TextRotationAlignment>() == style::AlignmentType::Map;
const bool pitchWithMap = bucket.layout.get<style::TextPitchAlignment>() == style::AlignmentType::Map;
const bool textAllowOverlap = bucket.layout->get<style::TextAllowOverlap>();
const bool iconAllowOverlap = bucket.layout->get<style::IconAllowOverlap>();
const bool variablePlacement = !bucket.layout->get<style::TextVariableAnchor>().empty();
const bool rotateWithMap = bucket.layout->get<style::TextRotationAlignment>() == style::AlignmentType::Map;
const bool pitchWithMap = bucket.layout->get<style::TextPitchAlignment>() == style::AlignmentType::Map;

// If allow-overlap is true, we can show symbols before placement runs on them
// But we have to wait for placement if we potentially depend on a paired icon/text
// with allow-overlap: false.
// See https://github.com/mapbox/mapbox-gl-native/issues/12483
JointOpacityState defaultOpacityState(
textAllowOverlap && (iconAllowOverlap || !bucket.hasIconData() || bucket.layout.get<style::IconOptional>()),
iconAllowOverlap && (textAllowOverlap || !bucket.hasTextData() || bucket.layout.get<style::TextOptional>()),
textAllowOverlap && (iconAllowOverlap || !bucket.hasIconData() || bucket.layout->get<style::IconOptional>()),
iconAllowOverlap && (textAllowOverlap || !bucket.hasTextData() || bucket.layout->get<style::TextOptional>()),
true);

for (SymbolInstance& symbolInstance : bucket.symbolInstances) {
Expand Down Expand Up @@ -598,7 +598,7 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, const TransformState
}
auto dynamicVertex = CollisionBoxProgram::dynamicVertex(placed, false, {});
for (size_t i = 0; i < feature.boxes.size() * 4; i++) {
bucket.collisionBox.dynamicVertices.emplace_back(dynamicVertex);
bucket.collisionBox->dynamicVertices.emplace_back(dynamicVertex);
}
};

Expand Down Expand Up @@ -633,7 +633,7 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, const TransformState
}
auto dynamicVertex = CollisionBoxProgram::dynamicVertex(placed, !used, shift);
for (size_t i = 0; i < feature.boxes.size() * 4; i++) {
bucket.collisionBox.dynamicVertices.emplace_back(dynamicVertex);
bucket.collisionBox->dynamicVertices.emplace_back(dynamicVertex);
}
};

Expand All @@ -643,10 +643,10 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, const TransformState
}
for (const CollisionBox& box : feature.boxes) {
auto dynamicVertex = CollisionBoxProgram::dynamicVertex(placed, !box.used, {});
bucket.collisionCircle.dynamicVertices.emplace_back(dynamicVertex);
bucket.collisionCircle.dynamicVertices.emplace_back(dynamicVertex);
bucket.collisionCircle.dynamicVertices.emplace_back(dynamicVertex);
bucket.collisionCircle.dynamicVertices.emplace_back(dynamicVertex);
bucket.collisionCircle->dynamicVertices.emplace_back(dynamicVertex);
bucket.collisionCircle->dynamicVertices.emplace_back(dynamicVertex);
bucket.collisionCircle->dynamicVertices.emplace_back(dynamicVertex);
bucket.collisionCircle->dynamicVertices.emplace_back(dynamicVertex);
}
};

Expand Down
4 changes: 2 additions & 2 deletions test/gl/bucket.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,15 @@ TEST(Buckets, SymbolBucket) {
gl::HeadlessBackend backend({ 512, 256 });
gfx::BackendScope scope { backend };

style::SymbolLayoutProperties::PossiblyEvaluated layout;
auto layout = makeMutable<style::SymbolLayoutProperties::PossiblyEvaluated>();
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
std::string bucketLeaderID = "test";
std::vector<SymbolInstance> symbolInstances;

gl::Context context{ backend };
SymbolBucket bucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(symbolInstances), 1.0f };
SymbolBucket bucket { std::move(layout), {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(symbolInstances), 1.0f };
ASSERT_FALSE(bucket.hasIconData());
ASSERT_FALSE(bucket.hasTextData());
ASSERT_FALSE(bucket.hasCollisionBoxData());
Expand Down
12 changes: 8 additions & 4 deletions test/text/cross_tile_symbol_index.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
uint32_t maxBucketInstanceId = 0;
CrossTileSymbolLayerIndex index;

style::SymbolLayoutProperties::PossiblyEvaluated layout;
Immutable<style::SymbolLayoutProperties::PossiblyEvaluated> layout =
makeMutable<style::SymbolLayoutProperties::PossiblyEvaluated>();
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
Expand Down Expand Up @@ -95,7 +96,8 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) {
uint32_t maxBucketInstanceId = 0;
CrossTileSymbolLayerIndex index;

style::SymbolLayoutProperties::PossiblyEvaluated layout;
Immutable<style::SymbolLayoutProperties::PossiblyEvaluated> layout =
makeMutable<style::SymbolLayoutProperties::PossiblyEvaluated>();
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
Expand Down Expand Up @@ -135,7 +137,8 @@ TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) {
uint32_t maxBucketInstanceId = 0;
CrossTileSymbolLayerIndex index;

style::SymbolLayoutProperties::PossiblyEvaluated layout;
Immutable<style::SymbolLayoutProperties::PossiblyEvaluated> layout =
makeMutable<style::SymbolLayoutProperties::PossiblyEvaluated>();
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
Expand Down Expand Up @@ -173,7 +176,8 @@ TEST(CrossTileSymbolLayerIndex, bucketReplacement) {
uint32_t maxBucketInstanceId = 0;
CrossTileSymbolLayerIndex index;

style::SymbolLayoutProperties::PossiblyEvaluated layout;
Immutable<style::SymbolLayoutProperties::PossiblyEvaluated> layout =
makeMutable<style::SymbolLayoutProperties::PossiblyEvaluated>();
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
Expand Down