summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChris Loer <chris.loer@gmail.com>2018-04-27 17:18:29 -0700
committerChris Loer <chris.loer@mapbox.com>2018-05-03 14:17:25 -0700
commit27b21363e62c105db0b040b4c5a5ef31170ebd30 (patch)
tree65e3b174fcf583378450252e2a322dbc79ee1a79
parent60ac08afae3d3ca0e94ce4f1ce7dfb98c25e2345 (diff)
downloadqtlocation-mapboxgl-27b21363e62c105db0b040b4c5a5ef31170ebd30.tar.gz
[core] Only run placement for first layer per SymbolBucket
Native version of mapbox/mapbox-gl-js#6548. Port of mapbox/mapbox-gl-js#6550. Prevents symbols that share the same layout properties from colliding against each other. Bump GL JS pin to get regression test. Rename "bucketName" -> "bucketLeaderID" to make it clearer what it represents.
m---------mapbox-gl-js0
-rw-r--r--src/mbgl/geometry/feature_index.cpp10
-rw-r--r--src/mbgl/geometry/feature_index.hpp10
-rw-r--r--src/mbgl/layout/symbol_layout.cpp6
-rw-r--r--src/mbgl/layout/symbol_layout.hpp2
-rw-r--r--src/mbgl/renderer/buckets/symbol_bucket.cpp2
-rw-r--r--src/mbgl/renderer/buckets/symbol_bucket.hpp3
-rw-r--r--src/mbgl/text/cross_tile_symbol_index.cpp4
-rw-r--r--src/mbgl/text/placement.cpp9
-rw-r--r--test/gl/bucket.test.cpp3
-rw-r--r--test/text/cross_tile_symbol_index.test.cpp24
11 files changed, 48 insertions, 25 deletions
diff --git a/mapbox-gl-js b/mapbox-gl-js
-Subproject b0ba8bded514e791ba8bd7eda4a513a762c5e5a
+Subproject 5f3bdb9b00915d702ae9b967c5ebc15ab33ba65
diff --git a/src/mbgl/geometry/feature_index.cpp b/src/mbgl/geometry/feature_index.cpp
index 520fd313a2..fdd9558d0b 100644
--- a/src/mbgl/geometry/feature_index.cpp
+++ b/src/mbgl/geometry/feature_index.cpp
@@ -25,14 +25,14 @@ FeatureIndex::FeatureIndex(std::unique_ptr<const GeometryTileData> tileData_)
void FeatureIndex::insert(const GeometryCollection& geometries,
std::size_t index,
const std::string& sourceLayerName,
- const std::string& bucketName) {
+ const std::string& bucketLeaderID) {
for (const auto& ring : geometries) {
auto envelope = mapbox::geometry::envelope(ring);
if (envelope.min.x < util::EXTENT &&
envelope.min.y < util::EXTENT &&
envelope.max.x >= 0 &&
envelope.max.y >= 0) {
- grid.insert(IndexedSubfeature(index, sourceLayerName, bucketName, sortIndex++),
+ grid.insert(IndexedSubfeature(index, sourceLayerName, bucketLeaderID, sortIndex++),
{convertPoint<float>(envelope.min), convertPoint<float>(envelope.max)});
}
}
@@ -141,7 +141,7 @@ void FeatureIndex::addFeature(
std::unique_ptr<GeometryTileLayer> sourceLayer;
std::unique_ptr<GeometryTileFeature> geometryTileFeature;
- for (const std::string& layerID : bucketLayerIDs.at(indexedFeature.bucketName)) {
+ for (const std::string& layerID : bucketLayerIDs.at(indexedFeature.bucketLeaderID)) {
const RenderLayer* renderLayer = getRenderLayer(layerID);
if (!renderLayer) {
continue;
@@ -190,8 +190,8 @@ optional<GeometryCoordinates> FeatureIndex::translateQueryGeometry(
return translated;
}
-void FeatureIndex::setBucketLayerIDs(const std::string& bucketName, const std::vector<std::string>& layerIDs) {
- bucketLayerIDs[bucketName] = layerIDs;
+void FeatureIndex::setBucketLayerIDs(const std::string& bucketLeaderID, const std::vector<std::string>& layerIDs) {
+ bucketLayerIDs[bucketLeaderID] = layerIDs;
}
} // namespace mbgl
diff --git a/src/mbgl/geometry/feature_index.hpp b/src/mbgl/geometry/feature_index.hpp
index cc91791d36..739c1f282f 100644
--- a/src/mbgl/geometry/feature_index.hpp
+++ b/src/mbgl/geometry/feature_index.hpp
@@ -25,7 +25,7 @@ public:
IndexedSubfeature(std::size_t index_, std::string sourceLayerName_, std::string bucketName_, size_t sortIndex_)
: index(index_)
, sourceLayerName(std::move(sourceLayerName_))
- , bucketName(std::move(bucketName_))
+ , bucketLeaderID(std::move(bucketName_))
, sortIndex(sortIndex_)
, bucketInstanceId(0)
{}
@@ -34,13 +34,13 @@ public:
IndexedSubfeature(const IndexedSubfeature& other, uint32_t bucketInstanceId_)
: index(other.index)
, sourceLayerName(other.sourceLayerName)
- , bucketName(other.bucketName)
+ , bucketLeaderID(other.bucketLeaderID)
, sortIndex(other.sortIndex)
, bucketInstanceId(bucketInstanceId_)
{}
size_t index;
std::string sourceLayerName;
- std::string bucketName;
+ std::string bucketLeaderID;
size_t sortIndex;
// Only used for symbol features
@@ -53,7 +53,7 @@ public:
const GeometryTileData* getData() { return tileData.get(); }
- void insert(const GeometryCollection&, std::size_t index, const std::string& sourceLayerName, const std::string& bucketName);
+ void insert(const GeometryCollection&, std::size_t index, const std::string& sourceLayerName, const std::string& bucketLeaderID);
void query(
std::unordered_map<std::string, std::vector<Feature>>& result,
@@ -74,7 +74,7 @@ public:
const float bearing,
const float pixelsToTileUnits);
- void setBucketLayerIDs(const std::string& bucketName, const std::vector<std::string>& layerIDs);
+ void setBucketLayerIDs(const std::string& bucketLeaderID, const std::vector<std::string>& layerIDs);
std::unordered_map<std::string, std::vector<Feature>> lookupSymbolFeatures(
const std::vector<IndexedSubfeature>& symbolFeatures,
diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp
index d3126bba45..b2f6fd450f 100644
--- a/src/mbgl/layout/symbol_layout.cpp
+++ b/src/mbgl/layout/symbol_layout.cpp
@@ -42,7 +42,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters,
std::unique_ptr<GeometryTileLayer> sourceLayer_,
ImageDependencies& imageDependencies,
GlyphDependencies& glyphDependencies)
- : bucketName(layers.at(0)->getID()),
+ : bucketLeaderID(layers.at(0)->getID()),
sourceLayer(std::move(sourceLayer_)),
overscaling(parameters.tileID.overscaleFactor()),
zoom(parameters.tileID.overscaledZ),
@@ -294,7 +294,7 @@ void SymbolLayout::addFeature(const std::size_t index,
: layout.get<SymbolPlacement>();
const float textRepeatDistance = symbolSpacing / 2;
- IndexedSubfeature indexedFeature(feature.index, sourceLayer->getName(), bucketName, symbolInstances.size());
+ IndexedSubfeature indexedFeature(feature.index, sourceLayer->getName(), bucketLeaderID, symbolInstances.size());
auto addSymbolInstance = [&] (const GeometryCoordinates& line, Anchor& anchor) {
// https://github.com/mapbox/vector-tile-spec/tree/master/2.1#41-layers
@@ -420,7 +420,7 @@ std::unique_ptr<SymbolBucket> SymbolLayout::place(const bool showCollisionBoxes)
const bool mayOverlap = layout.get<TextAllowOverlap>() || layout.get<IconAllowOverlap>() ||
layout.get<TextIgnorePlacement>() || layout.get<IconIgnorePlacement>();
- auto bucket = std::make_unique<SymbolBucket>(layout, layerPaintProperties, textSize, iconSize, zoom, sdfIcons, iconsNeedLinear, mayOverlap, std::move(symbolInstances));
+ auto bucket = std::make_unique<SymbolBucket>(layout, layerPaintProperties, textSize, iconSize, zoom, sdfIcons, iconsNeedLinear, mayOverlap, bucketLeaderID, std::move(symbolInstances));
for (SymbolInstance &symbolInstance : bucket->symbolInstances) {
diff --git a/src/mbgl/layout/symbol_layout.hpp b/src/mbgl/layout/symbol_layout.hpp
index c93d8f4106..43b577859f 100644
--- a/src/mbgl/layout/symbol_layout.hpp
+++ b/src/mbgl/layout/symbol_layout.hpp
@@ -43,7 +43,7 @@ public:
std::map<std::string,
std::pair<style::IconPaintProperties::PossiblyEvaluated, style::TextPaintProperties::PossiblyEvaluated>> layerPaintProperties;
- const std::string bucketName;
+ const std::string bucketLeaderID;
std::vector<SymbolInstance> symbolInstances;
private:
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp
index 3d40a1714d..4fe03eb453 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.cpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp
@@ -18,11 +18,13 @@ SymbolBucket::SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated layo
bool sdfIcons_,
bool iconsNeedLinear_,
bool sortFeaturesByY_,
+ const std::string bucketName_,
const std::vector<SymbolInstance>&& symbolInstances_)
: layout(std::move(layout_)),
sdfIcons(sdfIcons_),
iconsNeedLinear(iconsNeedLinear_ || iconSize.isDataDriven() || !iconSize.isZoomConstant()),
sortFeaturesByY(sortFeaturesByY_),
+ bucketLeaderID(std::move(bucketName_)),
symbolInstances(std::move(symbolInstances_)),
textSizeBinder(SymbolSizeBinder::create(zoom, textSize, TextSize::defaultValue())),
iconSizeBinder(SymbolSizeBinder::create(zoom, iconSize, IconSize::defaultValue())) {
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp
index e52d18372d..e4aaf5ba30 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.hpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp
@@ -47,6 +47,7 @@ public:
bool sdfIcons,
bool iconsNeedLinear,
bool sortFeaturesByY,
+ const std::string bucketLeaderID,
const std::vector<SymbolInstance>&&);
void upload(gl::Context&) override;
@@ -64,6 +65,8 @@ public:
const bool iconsNeedLinear;
const bool sortFeaturesByY;
+ const std::string bucketLeaderID;
+
optional<float> sortedAngle;
bool staticUploaded = false;
diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp
index 01a4a02b4e..b0c3511ce3 100644
--- a/src/mbgl/text/cross_tile_symbol_index.cpp
+++ b/src/mbgl/text/cross_tile_symbol_index.cpp
@@ -153,6 +153,10 @@ bool CrossTileSymbolIndex::addLayer(RenderSymbolLayer& symbolLayer) {
auto bucket = renderTile.tile.getBucket(*symbolLayer.baseImpl);
assert(dynamic_cast<SymbolBucket*>(bucket));
SymbolBucket& symbolBucket = *reinterpret_cast<SymbolBucket*>(bucket);
+ if (symbolBucket.bucketLeaderID != symbolLayer.getID()) {
+ // Only add this layer if it's the "group leader" for the bucket
+ continue;
+ }
if (!symbolBucket.bucketInstanceId) {
symbolBucket.bucketInstanceId = ++maxBucketInstanceId;
diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp
index 43e8ff4f93..9883a1f456 100644
--- a/src/mbgl/text/placement.cpp
+++ b/src/mbgl/text/placement.cpp
@@ -56,6 +56,11 @@ void Placement::placeLayer(RenderSymbolLayer& symbolLayer, const mat4& projMatri
auto bucket = geometryTile.getBucket(*symbolLayer.baseImpl);
assert(dynamic_cast<SymbolBucket*>(bucket));
SymbolBucket& symbolBucket = *reinterpret_cast<SymbolBucket*>(bucket);
+
+ if (symbolBucket.bucketLeaderID != symbolLayer.getID()) {
+ // Only place this layer if it's the "group leader" for the bucket
+ continue;
+ }
auto& layout = symbolBucket.layout;
@@ -230,6 +235,10 @@ void Placement::updateLayerOpacities(RenderSymbolLayer& symbolLayer) {
auto bucket = renderTile.tile.getBucket(*symbolLayer.baseImpl);
assert(dynamic_cast<SymbolBucket*>(bucket));
SymbolBucket& symbolBucket = *reinterpret_cast<SymbolBucket*>(bucket);
+ if (symbolBucket.bucketLeaderID != symbolLayer.getID()) {
+ // Only update opacities this layer if it's the "group leader" for the bucket
+ continue;
+ }
updateBucketOpacities(symbolBucket, seenCrossTileIDs);
}
}
diff --git a/test/gl/bucket.test.cpp b/test/gl/bucket.test.cpp
index 8393fd130d..9b28b2e01a 100644
--- a/test/gl/bucket.test.cpp
+++ b/test/gl/bucket.test.cpp
@@ -110,10 +110,11 @@ TEST(Buckets, SymbolBucket) {
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
+ std::string bucketLeaderID = "test";
std::vector<SymbolInstance> symbolInstances;
gl::Context context;
- SymbolBucket bucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(symbolInstances) };
+ SymbolBucket bucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(symbolInstances) };
ASSERT_FALSE(bucket.hasIconData());
ASSERT_FALSE(bucket.hasTextData());
ASSERT_FALSE(bucket.hasCollisionBoxData());
diff --git a/test/text/cross_tile_symbol_index.test.cpp b/test/text/cross_tile_symbol_index.test.cpp
index 794c2b59a1..5d255a6105 100644
--- a/test/text/cross_tile_symbol_index.test.cpp
+++ b/test/text/cross_tile_symbol_index.test.cpp
@@ -25,12 +25,13 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
+ std::string bucketLeaderID = "test";
OverscaledTileID mainID(6, 0, 6, 8, 8);
std::vector<SymbolInstance> mainInstances;
mainInstances.push_back(makeSymbolInstance(1000, 1000, u"Detroit"));
mainInstances.push_back(makeSymbolInstance(2000, 2000, u"Toronto"));
- SymbolBucket mainBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(mainInstances) };
+ SymbolBucket mainBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(mainInstances) };
mainBucket.bucketInstanceId = ++maxBucketInstanceId;
index.addBucket(mainID, mainBucket, maxCrossTileID);
@@ -45,7 +46,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
childInstances.push_back(makeSymbolInstance(2000, 2000, u"Windsor"));
childInstances.push_back(makeSymbolInstance(3000, 3000, u"Toronto"));
childInstances.push_back(makeSymbolInstance(4001, 4001, u"Toronto"));
- SymbolBucket childBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(childInstances) };
+ SymbolBucket childBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(childInstances) };
childBucket.bucketInstanceId = ++maxBucketInstanceId;
index.addBucket(childID, childBucket, maxCrossTileID);
@@ -61,7 +62,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
OverscaledTileID parentID(5, 0, 5, 4, 4);
std::vector<SymbolInstance> parentInstances;
parentInstances.push_back(makeSymbolInstance(500, 500, u"Detroit"));
- SymbolBucket parentBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(parentInstances) };
+ SymbolBucket parentBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(parentInstances) };
parentBucket.bucketInstanceId = ++maxBucketInstanceId;
index.addBucket(parentID, parentBucket, maxCrossTileID);
@@ -77,7 +78,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) {
std::vector<SymbolInstance> grandchildInstances;
grandchildInstances.push_back(makeSymbolInstance(4000, 4000, u"Detroit"));
grandchildInstances.push_back(makeSymbolInstance(4000, 4000, u"Windsor"));
- SymbolBucket grandchildBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(grandchildInstances) };
+ SymbolBucket grandchildBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(grandchildInstances) };
grandchildBucket.bucketInstanceId = ++maxBucketInstanceId;
index.addBucket(grandchildID, grandchildBucket, maxCrossTileID);
@@ -98,17 +99,18 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) {
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
+ std::string bucketLeaderID = "test";
OverscaledTileID mainID(6, 0, 6, 8, 8);
std::vector<SymbolInstance> mainInstances;
mainInstances.push_back(makeSymbolInstance(1000, 1000, u"Detroit"));
- SymbolBucket mainBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(mainInstances) };
+ SymbolBucket mainBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(mainInstances) };
mainBucket.bucketInstanceId = ++maxBucketInstanceId;
OverscaledTileID childID(7, 0, 7, 16, 16);
std::vector<SymbolInstance> childInstances;
childInstances.push_back(makeSymbolInstance(2000, 2000, u"Detroit"));
- SymbolBucket childBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(childInstances) };
+ SymbolBucket childBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(childInstances) };
childBucket.bucketInstanceId = ++maxBucketInstanceId;
// assigns a new id
@@ -137,12 +139,13 @@ TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) {
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
+ std::string bucketLeaderID = "test";
OverscaledTileID mainID(6, 0, 6, 8, 8);
std::vector<SymbolInstance> mainInstances;
mainInstances.push_back(makeSymbolInstance(1000, 1000, u"")); // A
mainInstances.push_back(makeSymbolInstance(1000, 1000, u"")); // B
- SymbolBucket mainBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(mainInstances) };
+ SymbolBucket mainBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(mainInstances) };
mainBucket.bucketInstanceId = ++maxBucketInstanceId;
OverscaledTileID childID(7, 0, 7, 16, 16);
@@ -150,7 +153,7 @@ TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) {
childInstances.push_back(makeSymbolInstance(2000, 2000, u"")); // A'
childInstances.push_back(makeSymbolInstance(2000, 2000, u"")); // B'
childInstances.push_back(makeSymbolInstance(2000, 2000, u"")); // C'
- SymbolBucket childBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(childInstances) };
+ SymbolBucket childBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(childInstances) };
childBucket.bucketInstanceId = ++maxBucketInstanceId;
// assigns new ids
@@ -174,19 +177,20 @@ TEST(CrossTileSymbolLayerIndex, bucketReplacement) {
bool sdfIcons = false;
bool iconsNeedLinear = false;
bool sortFeaturesByY = false;
+ std::string bucketLeaderID = "test";
OverscaledTileID tileID(6, 0, 6, 8, 8);
std::vector<SymbolInstance> firstInstances;
firstInstances.push_back(makeSymbolInstance(1000, 1000, u"")); // A
firstInstances.push_back(makeSymbolInstance(1000, 1000, u"")); // B
- SymbolBucket firstBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(firstInstances) };
+ SymbolBucket firstBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(firstInstances) };
firstBucket.bucketInstanceId = ++maxBucketInstanceId;
std::vector<SymbolInstance> secondInstances;
secondInstances.push_back(makeSymbolInstance(1000, 1000, u"")); // A'
secondInstances.push_back(makeSymbolInstance(1000, 1000, u"")); // B'
secondInstances.push_back(makeSymbolInstance(1000, 1000, u"")); // C'
- SymbolBucket secondBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, std::move(secondInstances) };
+ SymbolBucket secondBucket { layout, {}, 16.0f, 1.0f, 0, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(secondInstances) };
secondBucket.bucketInstanceId = ++maxBucketInstanceId;
// assigns new ids