This repository has been archived by the owner on Aug 8, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[core] Enable circle-sort-key property #15875
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
35 changes: 35 additions & 0 deletions
35
metrics/android-render-test-runner/render-tests/circle-sort-key/literal/metrics.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
{ | ||
"network": [ | ||
[ | ||
"probeNetwork - default - end", | ||
0, | ||
0 | ||
], | ||
[ | ||
"probeNetwork - default - start", | ||
0, | ||
0 | ||
] | ||
], | ||
"gfx": [ | ||
[ | ||
"probeGFX - default - end", | ||
4, | ||
9, | ||
13, | ||
1, | ||
[ | ||
65552, | ||
65552 | ||
], | ||
[ | ||
94, | ||
94 | ||
], | ||
[ | ||
448, | ||
448 | ||
] | ||
] | ||
] | ||
} |
Binary file added
BIN
+950 Bytes
...ics/expectations/platform-all/render-tests/circle-sort-key/literal/expected.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
35 changes: 35 additions & 0 deletions
35
metrics/ios-render-test-runner/render-tests/circle-sort-key/literal/metrics.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
{ | ||
"network": [ | ||
[ | ||
"probeNetwork - default - end", | ||
0, | ||
0 | ||
], | ||
[ | ||
"probeNetwork - default - start", | ||
0, | ||
0 | ||
] | ||
], | ||
"gfx": [ | ||
[ | ||
"probeGFX - default - end", | ||
4, | ||
9, | ||
13, | ||
1, | ||
[ | ||
65552, | ||
65552 | ||
], | ||
[ | ||
94, | ||
94 | ||
], | ||
[ | ||
448, | ||
448 | ||
] | ||
] | ||
] | ||
} |
35 changes: 35 additions & 0 deletions
35
metrics/linux-clang8-release/render-tests/circle-sort-key/literal/metrics.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
{ | ||
"network": [ | ||
[ | ||
"probeNetwork - default - end", | ||
0, | ||
0 | ||
], | ||
[ | ||
"probeNetwork - default - start", | ||
0, | ||
0 | ||
] | ||
], | ||
"gfx": [ | ||
[ | ||
"probeGFX - default - end", | ||
4, | ||
9, | ||
13, | ||
1, | ||
[ | ||
65552, | ||
65552 | ||
], | ||
[ | ||
94, | ||
94 | ||
], | ||
[ | ||
448, | ||
448 | ||
] | ||
] | ||
] | ||
} |
35 changes: 35 additions & 0 deletions
35
metrics/linux-gcc8-release/render-tests/circle-sort-key/literal/metrics.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
{ | ||
"network": [ | ||
[ | ||
"probeNetwork - default - end", | ||
0, | ||
0 | ||
], | ||
[ | ||
"probeNetwork - default - start", | ||
0, | ||
0 | ||
] | ||
], | ||
"gfx": [ | ||
[ | ||
"probeGFX - default - end", | ||
4, | ||
9, | ||
13, | ||
1, | ||
[ | ||
65552, | ||
65552 | ||
], | ||
[ | ||
94, | ||
94 | ||
], | ||
[ | ||
448, | ||
448 | ||
] | ||
] | ||
] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
#pragma once | ||
#include <mbgl/geometry/feature_index.hpp> | ||
#include <mbgl/layout/layout.hpp> | ||
#include <mbgl/renderer/bucket_parameters.hpp> | ||
#include <mbgl/renderer/buckets/circle_bucket.hpp> | ||
#include <mbgl/renderer/render_layer.hpp> | ||
#include <mbgl/style/layers/circle_layer_impl.hpp> | ||
|
||
namespace mbgl { | ||
|
||
class CircleLayout final : public Layout { | ||
public: | ||
CircleLayout(const BucketParameters& parameters, | ||
const std::vector<Immutable<style::LayerProperties>>& group, | ||
std::unique_ptr<GeometryTileLayer> sourceLayer_) | ||
: sourceLayer(std::move(sourceLayer_)), zoom(parameters.tileID.overscaledZ), mode(parameters.mode) { | ||
assert(!group.empty()); | ||
auto leaderLayerProperties = staticImmutableCast<style::CircleLayerProperties>(group.front()); | ||
const auto& unevaluatedLayout = leaderLayerProperties->layerImpl().layout; | ||
const bool sortFeaturesByKey = !unevaluatedLayout.get<style::CircleSortKey>().isUndefined(); | ||
const auto& layout = unevaluatedLayout.evaluate(PropertyEvaluationParameters(zoom)); | ||
sourceLayerID = leaderLayerProperties->layerImpl().sourceLayer; | ||
bucketLeaderID = leaderLayerProperties->layerImpl().id; | ||
|
||
for (const auto& layerProperties : group) { | ||
const std::string& layerId = layerProperties->baseImpl->id; | ||
layerPropertiesMap.emplace(layerId, layerProperties); | ||
} | ||
|
||
const size_t featureCount = sourceLayer->featureCount(); | ||
for (size_t i = 0; i < featureCount; ++i) { | ||
auto feature = sourceLayer->getFeature(i); | ||
if (!leaderLayerProperties->layerImpl().filter(style::expression::EvaluationContext(zoom, feature.get()) | ||
.withCanonicalTileID(¶meters.tileID.canonical))) { | ||
continue; | ||
} | ||
|
||
if (!sortFeaturesByKey) { | ||
features.push_back({i, std::move(feature), style::CircleSortKey::defaultValue()}); | ||
continue; | ||
} | ||
|
||
const auto& sortKeyProperty = layout.template get<style::CircleSortKey>(); | ||
float sortKey = sortKeyProperty.evaluate(*feature, zoom, style::CircleSortKey::defaultValue()); | ||
CircleFeature circleFeature{i, std::move(feature), sortKey}; | ||
const auto sortPosition = std::lower_bound(features.cbegin(), features.cend(), circleFeature); | ||
features.insert(sortPosition, std::move(circleFeature)); | ||
} | ||
} | ||
|
||
bool hasDependencies() const override { return false; } | ||
|
||
void createBucket(const ImagePositions&, | ||
std::unique_ptr<FeatureIndex>& featureIndex, | ||
std::unordered_map<std::string, LayerRenderData>& renderData, | ||
const bool, | ||
const bool, | ||
const CanonicalTileID& canonical) override { | ||
auto bucket = std::make_shared<CircleBucket>(layerPropertiesMap, mode, zoom); | ||
|
||
for (auto& circleFeature : features) { | ||
const auto i = circleFeature.i; | ||
const std::unique_ptr<GeometryTileFeature>& feature = circleFeature.feature; | ||
const GeometryCollection& geometries = feature->getGeometries(); | ||
|
||
addCircle(*bucket, *feature, geometries, i, circleFeature.sortKey, canonical); | ||
|
||
bucket->addFeature(*feature, geometries, {}, PatternLayerMap(), i, canonical); | ||
featureIndex->insert(geometries, i, sourceLayerID, bucketLeaderID); | ||
} | ||
|
||
if (!bucket->hasData()) return; | ||
|
||
for (const auto& pair : layerPropertiesMap) { | ||
renderData.emplace(pair.first, LayerRenderData{bucket, pair.second}); | ||
} | ||
} | ||
|
||
private: | ||
struct CircleFeature { | ||
friend bool operator<(const CircleFeature& lhs, const CircleFeature& rhs) { return lhs.sortKey < rhs.sortKey; } | ||
|
||
size_t i; | ||
std::unique_ptr<GeometryTileFeature> feature; | ||
float sortKey; | ||
}; | ||
|
||
void addCircle(CircleBucket& bucket, | ||
const GeometryTileFeature& feature, | ||
const GeometryCollection& geometry, | ||
std::size_t featureIndex, | ||
float sortKey, | ||
const CanonicalTileID& canonical) { | ||
constexpr const uint16_t vertexLength = 4; | ||
|
||
auto& segments = bucket.segments; | ||
auto& vertices = bucket.vertices; | ||
auto& triangles = bucket.triangles; | ||
|
||
for (auto& circle : geometry) { | ||
for (auto& point : circle) { | ||
auto x = point.x; | ||
auto y = point.y; | ||
|
||
// Do not include points that are outside the tile boundaries. | ||
// Include all points in Still mode. You need to include points from | ||
// neighbouring tiles so that they are not clipped at tile boundaries. | ||
if ((mode == MapMode::Continuous) && (x < 0 || x >= util::EXTENT || y < 0 || y >= util::EXTENT)) | ||
continue; | ||
|
||
if (segments.empty() || | ||
segments.back().vertexLength + vertexLength > std::numeric_limits<uint16_t>::max()) { | ||
// Move to a new segments because the old one can't hold the geometry. | ||
segments.emplace_back(vertices.elements(), triangles.elements(), 0ul, 0ul, sortKey); | ||
} | ||
|
||
// this geometry will be of the Point type, and we'll derive | ||
// two triangles from it. | ||
// | ||
// ┌─────────┐ | ||
// │ 4 3 │ | ||
// │ │ | ||
// │ 1 2 │ | ||
// └─────────┘ | ||
// | ||
vertices.emplace_back(CircleProgram::vertex(point, -1, -1)); // 1 | ||
vertices.emplace_back(CircleProgram::vertex(point, 1, -1)); // 2 | ||
vertices.emplace_back(CircleProgram::vertex(point, 1, 1)); // 3 | ||
vertices.emplace_back(CircleProgram::vertex(point, -1, 1)); // 4 | ||
|
||
auto& segment = segments.back(); | ||
assert(segment.vertexLength <= std::numeric_limits<uint16_t>::max()); | ||
uint16_t index = segment.vertexLength; | ||
|
||
// 1, 2, 3 | ||
// 1, 4, 3 | ||
triangles.emplace_back(index, index + 1, index + 2); | ||
triangles.emplace_back(index, index + 3, index + 2); | ||
|
||
segment.vertexLength += vertexLength; | ||
segment.indexLength += 6; | ||
} | ||
} | ||
|
||
for (auto& pair : bucket.paintPropertyBinders) { | ||
pair.second.populateVertexVectors(feature, vertices.elements(), featureIndex, {}, {}, canonical); | ||
} | ||
} | ||
|
||
std::map<std::string, Immutable<style::LayerProperties>> layerPropertiesMap; | ||
std::string bucketLeaderID; | ||
|
||
const std::unique_ptr<GeometryTileLayer> sourceLayer; | ||
std::list<CircleFeature> features; | ||
pozdnyakov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const float zoom; | ||
const MapMode mode; | ||
std::string sourceLayerID; | ||
}; | ||
|
||
} // namespace mbgl |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also remove these lines from the Darwin version of this script:
mapbox-gl-native/platform/darwin/scripts/generate-style-code.js
Lines 18 to 19 in 1f266b4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, these properties should be deleted in scripts/style-spec.js instead of both scripts/generate-style-code.js and platform/darwin/scripts/generate-style-code.js. This is the reason for the layout of indirection in scripts/style-spec.js.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that most of platform/darwin/ has been deleted in favor of the sources in the mapbox/mapbox-gl-native-ios repository, mapbox/mapbox-gl-native-ios#272 will take care of deleting the remnants of this override.