From d734f670e8127c7600beb6dad7b7f08b1401e7c0 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Tue, 26 Jun 2018 15:05:37 -0700 Subject: [core] Fix sort order for queryRenderedFeatures when features are filtered. Fixes issue #12104. --- platform/node/test/ignores.json | 2 -- src/mbgl/geometry/feature_index.cpp | 11 ++++++----- src/mbgl/layout/symbol_instance.cpp | 8 ++++---- src/mbgl/layout/symbol_instance.hpp | 8 ++++---- src/mbgl/layout/symbol_layout.cpp | 7 +++---- src/mbgl/renderer/buckets/symbol_bucket.cpp | 8 ++++---- test/text/cross_tile_symbol_index.test.cpp | 2 +- 7 files changed, 22 insertions(+), 24 deletions(-) diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json index 148469ef3a..51a50ff387 100644 --- a/platform/node/test/ignores.json +++ b/platform/node/test/ignores.json @@ -14,8 +14,6 @@ "query-tests/line-width/feature-state": "skip - port https://github.com/mapbox/mapbox-gl-js/pull/6263 - needs issue", "query-tests/feature-state/default": "skip - port https://github.com/mapbox/mapbox-gl-js/pull/6263 - needs issue", "query-tests/regressions/mapbox-gl-js#6555": "skip - no querySourceFeatures in mbgl-node; needs issue", - "query-tests/symbol/filtered-rotated-after-insert": "skip - https://github.com/mapbox/mapbox-gl-native/pull/12225", - "query-tests/symbol/rotated-sort": "skip - https://github.com/mapbox/mapbox-gl-native/pull/12225", "render-tests/background-color/transition": "https://github.com/mapbox/mapbox-gl-native/issues/10619", "render-tests/collator/resolved-locale": "Some test platforms don't resolve 'en' locale", "render-tests/collator/default": "Some test platforms don't resolve 'en' locale", diff --git a/src/mbgl/geometry/feature_index.cpp b/src/mbgl/geometry/feature_index.cpp index cc69a60ca0..651e3b9c56 100644 --- a/src/mbgl/geometry/feature_index.cpp +++ b/src/mbgl/geometry/feature_index.cpp @@ -77,11 +77,12 @@ void FeatureIndex::query( } } -std::unordered_map> FeatureIndex::lookupSymbolFeatures(const std::vector& symbolFeatures, - const RenderedQueryOptions& queryOptions, - const std::vector& layers, - const OverscaledTileID& tileID, - const std::shared_ptr>& featureSortOrder) const { +std::unordered_map> +FeatureIndex::lookupSymbolFeatures(const std::vector& symbolFeatures, + const RenderedQueryOptions& queryOptions, + const std::vector& layers, + const OverscaledTileID& tileID, + const std::shared_ptr>& featureSortOrder) const { std::unordered_map> result; if (!tileData) { return result; diff --git a/src/mbgl/layout/symbol_instance.cpp b/src/mbgl/layout/symbol_instance.cpp index 7dfa8edf43..a9b4b929ec 100644 --- a/src/mbgl/layout/symbol_instance.cpp +++ b/src/mbgl/layout/symbol_instance.cpp @@ -11,7 +11,6 @@ SymbolInstance::SymbolInstance(Anchor& anchor_, optional shapedIcon, const SymbolLayoutProperties::Evaluated& layout, const float layoutTextSize, - const uint32_t index_, const float textBoxScale, const float textPadding, const SymbolPlacementType textPlacement, @@ -21,19 +20,20 @@ SymbolInstance::SymbolInstance(Anchor& anchor_, const std::array iconOffset_, const GlyphPositionMap& positions, const IndexedSubfeature& indexedFeature, - const std::size_t featureIndex_, + const std::size_t layoutFeatureIndex_, + const std::size_t dataFeatureIndex_, const std::u16string& key_, const float overscaling) : anchor(anchor_), line(line_), - index(index_), hasText(false), hasIcon(shapedIcon), // Create the collision features that will be used to check whether this symbol instance can be placed textCollisionFeature(line_, anchor, shapedTextOrientations.first, textBoxScale, textPadding, textPlacement, indexedFeature, overscaling), iconCollisionFeature(line_, anchor, shapedIcon, iconBoxScale, iconPadding, indexedFeature), - featureIndex(featureIndex_), + layoutFeatureIndex(layoutFeatureIndex_), + dataFeatureIndex(dataFeatureIndex_), textOffset(textOffset_), iconOffset(iconOffset_), key(key_) { diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp index 827a5dbbdb..ae79311790 100644 --- a/src/mbgl/layout/symbol_instance.hpp +++ b/src/mbgl/layout/symbol_instance.hpp @@ -19,7 +19,6 @@ public: optional shapedIcon, const style::SymbolLayoutProperties::Evaluated&, const float layoutTextSize, - const uint32_t index, const float textBoxScale, const float textPadding, style::SymbolPlacementType textPlacement, @@ -29,13 +28,13 @@ public: const std::array iconOffset, const GlyphPositionMap&, const IndexedSubfeature&, - const std::size_t featureIndex, + const std::size_t layoutFeatureIndex, + const std::size_t dataFeatureIndex, const std::u16string& key, const float overscaling); Anchor anchor; GeometryCoordinates line; - uint32_t index; bool hasText; bool hasIcon; SymbolQuads horizontalGlyphQuads; @@ -44,7 +43,8 @@ public: CollisionFeature textCollisionFeature; CollisionFeature iconCollisionFeature; WritingModeType writingModes; - std::size_t featureIndex; + std::size_t layoutFeatureIndex; // Index into the set of features included at layout time + std::size_t dataFeatureIndex; // Index into the underlying tile data feature set std::array textOffset; std::array iconOffset; std::u16string key; diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index 4249ffc24e..e71cfb93b0 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -260,7 +260,7 @@ void SymbolLayout::prepare(const GlyphMap& glyphMap, const GlyphPositions& glyph compareText.clear(); } -void SymbolLayout::addFeature(const std::size_t index, +void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex, const SymbolFeature& feature, const std::pair& shapedTextOrientations, optional shapedIcon, @@ -317,10 +317,9 @@ void SymbolLayout::addFeature(const std::size_t index, if (mode == MapMode::Tile || withinPlus0) { symbolInstances.emplace_back(anchor, line, shapedTextOrientations, shapedIcon, layout.evaluate(zoom, feature), layoutTextSize, - symbolInstances.size(), textBoxScale, textPadding, textPlacement, textOffset, iconBoxScale, iconPadding, iconOffset, - glyphPositionMap, indexedFeature, index, feature.text.value_or(std::u16string()), overscaling); + glyphPositionMap, indexedFeature, layoutFeatureIndex, feature.index, feature.text.value_or(std::u16string()), overscaling); } }; @@ -426,7 +425,7 @@ std::unique_ptr SymbolLayout::place(const bool showCollisionBoxes) const bool hasText = symbolInstance.hasText; const bool hasIcon = symbolInstance.hasIcon; - const auto& feature = features.at(symbolInstance.featureIndex); + const auto& feature = features.at(symbolInstance.layoutFeatureIndex); // Insert final placement into collision tree and add glyphs/icons to buffers diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp index 4fe03eb453..51fcd2437c 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.cpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp @@ -192,11 +192,11 @@ void SymbolBucket::sortFeatures(const float angle) { std::sort(symbolInstanceIndexes.begin(), symbolInstanceIndexes.end(), [sin, cos, this](size_t &aIndex, size_t &bIndex) { const SymbolInstance& a = symbolInstances[aIndex]; const SymbolInstance& b = symbolInstances[bIndex]; - const int32_t aRotated = sin * a.anchor.point.x + cos * a.anchor.point.y; - const int32_t bRotated = sin * b.anchor.point.x + cos * b.anchor.point.y; + const int32_t aRotated = static_cast(std::lround(sin * a.anchor.point.x + cos * a.anchor.point.y)); + const int32_t bRotated = static_cast(std::lround(sin * b.anchor.point.x + cos * b.anchor.point.y)); return aRotated != bRotated ? aRotated < bRotated : - a.index > b.index; + a.dataFeatureIndex > b.dataFeatureIndex; }); text.triangles.clear(); @@ -207,7 +207,7 @@ void SymbolBucket::sortFeatures(const float angle) { for (auto i : symbolInstanceIndexes) { const SymbolInstance& symbolInstance = symbolInstances[i]; - featureSortOrder->push_back(symbolInstance.featureIndex); + featureSortOrder->push_back(symbolInstance.dataFeatureIndex); if (symbolInstance.placedTextIndex) { addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.placedTextIndex]); diff --git a/test/text/cross_tile_symbol_index.test.cpp b/test/text/cross_tile_symbol_index.test.cpp index 5d255a6105..4060d1665c 100644 --- a/test/text/cross_tile_symbol_index.test.cpp +++ b/test/text/cross_tile_symbol_index.test.cpp @@ -11,7 +11,7 @@ SymbolInstance makeSymbolInstance(float x, float y, std::u16string key) { style::SymbolLayoutProperties::Evaluated layout_; IndexedSubfeature subfeature(0, "", "", 0); Anchor anchor(x, y, 0, 0); - return {anchor, line, shaping, {}, layout_, 0, 0, 0, 0, style::SymbolPlacementType::Point, {{0, 0}}, 0, 0, {{0, 0}}, gpm, subfeature, 0, key, 0 }; + return {anchor, line, shaping, {}, layout_, 0, 0, 0, style::SymbolPlacementType::Point, {{0, 0}}, 0, 0, {{0, 0}}, gpm, subfeature, 0, 0, key, 0 }; } -- cgit v1.2.1