From dd104ded89a946f9c80a58200eadb5a00e0d0d60 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Mon, 26 Aug 2024 17:06:11 -0700 Subject: [PATCH] Add guard blocks and checks to `SymbolInstance` (#2744) --- include/mbgl/util/logging.hpp | 7 +- include/mbgl/util/string.hpp | 4 +- src/mbgl/layout/symbol_instance.cpp | 78 +++++ src/mbgl/layout/symbol_instance.hpp | 160 ++++++++- src/mbgl/layout/symbol_layout.cpp | 103 +++--- src/mbgl/renderer/bucket.hpp | 16 +- src/mbgl/renderer/buckets/symbol_bucket.cpp | 73 ++-- src/mbgl/renderer/buckets/symbol_bucket.hpp | 4 + .../renderer/layers/render_symbol_layer.cpp | 9 +- src/mbgl/text/cross_tile_symbol_index.cpp | 33 +- src/mbgl/text/cross_tile_symbol_index.hpp | 3 + src/mbgl/text/placement.cpp | 316 ++++++++++-------- src/mbgl/tile/geometry_tile.cpp | 16 + src/mbgl/tile/geometry_tile.hpp | 2 + src/mbgl/util/logging.cpp | 19 +- src/mbgl/util/string.cpp | 5 + test/text/cross_tile_symbol_index.test.cpp | 52 +-- 17 files changed, 629 insertions(+), 271 deletions(-) diff --git a/include/mbgl/util/logging.hpp b/include/mbgl/util/logging.hpp index 479df84d2..ab3e7a332 100644 --- a/include/mbgl/util/logging.hpp +++ b/include/mbgl/util/logging.hpp @@ -38,8 +38,11 @@ public: Log(); ~Log(); - static void useLogThread(bool enable) noexcept; - static void useLogThread(bool enable, EventSeverity) noexcept; + /// @brief Determines whether messages of a given severity level are logged asynchronously. + /// + /// In a crash or other unexpected termination, pending asynchronous log entries will be lost. + /// The default is true (asynchronous) for all levels except `Error`. + static void useLogThread(bool enable, std::optional = {}); template static void Debug(Event event, Args&&... args) noexcept { diff --git a/include/mbgl/util/string.hpp b/include/mbgl/util/string.hpp index a6d4d2e4c..9ce3abf55 100644 --- a/include/mbgl/util/string.hpp +++ b/include/mbgl/util/string.hpp @@ -78,9 +78,7 @@ inline std::string toString(long double t, bool decimal = false) { return toString(static_cast(t), decimal); } -inline std::string toString(std::thread::id threadId) { - return (std::ostringstream() << threadId).str(); -} +std::string toString(const std::thread::id &); std::string toString(const std::exception_ptr &); diff --git a/src/mbgl/layout/symbol_instance.cpp b/src/mbgl/layout/symbol_instance.cpp index f07ff37b9..5cc9e6aae 100644 --- a/src/mbgl/layout/symbol_instance.cpp +++ b/src/mbgl/layout/symbol_instance.cpp @@ -1,5 +1,7 @@ #include #include +#include + #include namespace mbgl { @@ -219,4 +221,80 @@ std::optional SymbolInstance::getDefaultHorizontalPlacedTextIndex() cons if (placedLeftTextIndex) return placedLeftTextIndex; return std::nullopt; } + +#if MLN_SYMBOL_GUARDS +bool SymbolInstance::check(const std::source_location& source) const { + return !isFailed && check(check01, 1, source) && check(check02, 2, source) && check(check03, 3, source) && + check(check04, 4, source) && check(check05, 5, source) && check(check06, 6, source) && + check(check07, 7, source) && check(check08, 8, source) && check(check09, 9, source) && + check(check10, 10, source) && check(check11, 11, source) && check(check12, 12, source) && + check(check13, 13, source) && check(check14, 14, source) && check(check15, 15, source) && + check(check16, 16, source) && check(check17, 17, source) && check(check18, 18, source) && + check(check19, 19, source) && check(check20, 20, source) && check(check21, 21, source) && + check(check22, 22, source) && check(check23, 23, source) && check(check24, 24, source) && + check(check25, 25, source) && check(check26, 26, source) && check(check27, 27, source) && + check(check28, 28, source) && checkKey(source); +} + +bool SymbolInstance::checkIndexes(std::size_t textCount, + std::size_t iconSize, + std::size_t sdfSize, + const std::source_location& source) const { + return !isFailed && checkIndex(placedRightTextIndex, textCount, source) && + checkIndex(placedCenterTextIndex, textCount, source) && checkIndex(placedLeftTextIndex, textCount, source) && + checkIndex(placedVerticalTextIndex, textCount, source) && + checkIndex(placedIconIndex, hasSdfIcon() ? sdfSize : iconSize, source) && + checkIndex(placedVerticalIconIndex, hasSdfIcon() ? sdfSize : iconSize, source); +} + +namespace { +inline std::string locationSuffix(const std::source_location& source) { + return std::string(" from ") + source.function_name() + " (" + source.file_name() + ":" + + util::toString(source.line()) + ")"; +} +} // namespace +bool SymbolInstance::check(std::uint64_t v, int n, const std::source_location& source) const { + if (!isFailed && v != checkVal) { + isFailed = true; + Log::Error(Event::Crash, + "SymbolInstance corrupted at " + util::toString(n) + " with value " + util::toString(v) + + locationSuffix(source)); + } + return !isFailed; +} + +bool SymbolInstance::checkKey(const std::source_location& source) const { + if (!isFailed && key.size() > 10000) { // largest observed value=62 + isFailed = true; + Log::Error(Event::Crash, + "SymbolInstance key corrupted with size=" + util::toString(key.size()) + locationSuffix(source)); + } + return !isFailed; +} + +bool SymbolInstance::checkIndex(const std::optional& index, + std::size_t size, + const std::source_location& source) const { + if (index.has_value() && *index >= size) { + isFailed = true; + Log::Error(Event::Crash, + "SymbolInstance index corrupted with value=" + util::toString(*index) + + " size=" + util::toString(size) + locationSuffix(source)); + } + return !isFailed; +} + +void SymbolInstance::forceFail() const { + isFailed = true; +} + +// this is just to avoid warnings about the values never being set +void SymbolInstance::forceFailInternal() { + check01 = check02 = check03 = check04 = check05 = check06 = check07 = check08 = check09 = check10 = check11 = + check12 = check13 = check14 = check15 = check16 = check17 = check18 = check19 = check20 = check21 = check22 = + check23 = check24 = check25 = check26 = check27 = check28 = 0; +} + +#endif // MLN_SYMBOL_GUARDS + } // namespace mbgl diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp index 3d57d1d71..30753229f 100644 --- a/src/mbgl/layout/symbol_instance.hpp +++ b/src/mbgl/layout/symbol_instance.hpp @@ -6,6 +6,49 @@ #include #include +#include + +#if !defined(MLN_SYMBOL_GUARDS) +#define MLN_SYMBOL_GUARDS 1 +#endif + +#if MLN_SYMBOL_GUARDS +#define SYM_GUARD_VALUE(N) std::uint64_t check##N = checkVal; +#else +#define SYM_GUARD_VALUE(N) +#endif + +// A temporary shim for partial C++20 support +#if MLN_SYMBOL_GUARDS +#if defined(__clang__) +#if __cplusplus <= 201703L || !__has_builtin(__builtin_source_location) +namespace std { +struct source_location { + const char* fileName_; + const char* functionName_; + unsigned line_; + + constexpr uint_least32_t line() const noexcept { return line_; } + constexpr uint_least32_t column() const noexcept { return 0; } + constexpr const char* file_name() const noexcept { return fileName_; } + constexpr const char* function_name() const noexcept { return functionName_; } +}; +} // namespace std +#define SYM_GUARD_LOC \ + std::source_location { \ + __FILE__, __FUNCTION__, __LINE__ \ + } +#else +#define SYM_GUARD_LOC std::source_location::current() +#endif +#else +#define SYM_GUARD_LOC std::source_location::current() +#endif +#else +#define SYM_GUARD_LOC \ + {} +#endif + namespace mbgl { class Anchor; @@ -52,6 +95,9 @@ struct SymbolInstanceSharedData { std::optional verticalIconQuads; }; +// With guards, clang complains about excessive padding here +// NOLINTBEGIN(clang-analyzer-optin.performance.Padding) + class SymbolInstance { public: SymbolInstance(Anchor& anchor_, @@ -90,43 +136,149 @@ public: const std::optional& verticalIconQuads() const; void releaseSharedData(); +#if MLN_SYMBOL_GUARDS + /// Check all guard blocks + bool check(const std::source_location&) const; + /// Check that an index is in the valid range + bool checkIndex(const std::optional& index, std::size_t size, const std::source_location&) const; + /// Check all indexes + bool checkIndexes(std::size_t textCount, + std::size_t iconSize, + std::size_t sdfSize, + const std::source_location&) const; + /// Mark this item as failed (due to some external check) so that it cannot be used later + void forceFail() const; +#else + bool check(std::string_view = {}) const { return true; } + bool checkIndex(const std::optional&, std::size_t, std::string_view = {}) const { return true; } + bool checkIndexes(std::size_t, std::size_t, std::size_t, std::string_view = {}) const { return true; } + void forceFail() const {} +#endif + + const Anchor& getAnchor() const { return anchor; } + std::size_t getRightJustifiedGlyphQuadsSize() const { return rightJustifiedGlyphQuadsSize; } + std::size_t getCenterJustifiedGlyphQuadsSize() const { return centerJustifiedGlyphQuadsSize; } + std::size_t getLeftJustifiedGlyphQuadsSize() const { return leftJustifiedGlyphQuadsSize; } + std::size_t getVerticalGlyphQuadsSize() const { return verticalGlyphQuadsSize; } + std::size_t getIconQuadsSize() const { return iconQuadsSize; } + const CollisionFeature& getTextCollisionFeature() const { return textCollisionFeature; } + const CollisionFeature& getIconCollisionFeature() const { return iconCollisionFeature; } + const std::optional& getVerticalTextCollisionFeature() const { + return verticalTextCollisionFeature; + } + const std::optional& getVerticalIconCollisionFeature() const { + return verticalIconCollisionFeature; + } + WritingModeType getWritingModes() const { return writingModes; } + std::size_t getLayoutFeatureIndex() const { return layoutFeatureIndex; } + std::size_t getDataFeatureIndex() const { return dataFeatureIndex; } + std::array getTextOffset() const { return textOffset; } + std::array getIconOffset() const { return iconOffset; } + const std::u16string& getKey() const { return key; } + std::optional getPlacedRightTextIndex() const { return placedRightTextIndex; } + std::optional getPlacedCenterTextIndex() const { return placedCenterTextIndex; } + std::optional getPlacedLeftTextIndex() const { return placedLeftTextIndex; } + std::optional getPlacedVerticalTextIndex() const { return placedVerticalTextIndex; } + std::optional getPlacedIconIndex() const { return placedIconIndex; } + std::optional getPlacedVerticalIconIndex() const { return placedVerticalIconIndex; } + float getTextBoxScale() const { return textBoxScale; } + std::array getVariableTextOffset() const { return variableTextOffset; } + bool getSingleLine() const { return singleLine; } + + uint32_t getCrossTileID() const { return crossTileID; } + void setCrossTileID(uint32_t x) { crossTileID = x; } + + std::optional& refPlacedRightTextIndex() { return placedRightTextIndex; } + std::optional& refPlacedCenterTextIndex() { return placedCenterTextIndex; } + std::optional& refPlacedLeftTextIndex() { return placedLeftTextIndex; } + std::optional& refPlacedVerticalTextIndex() { return placedVerticalTextIndex; } + std::optional& refPlacedIconIndex() { return placedIconIndex; } + std::optional& refPlacedVerticalIconIndex() { return placedVerticalIconIndex; } + + void setPlacedRightTextIndex(std::optional x) { placedRightTextIndex = x; } + void setPlacedCenterTextIndex(std::optional x) { placedCenterTextIndex = x; } + void setPlacedLeftTextIndex(std::optional x) { placedLeftTextIndex = x; } + + static constexpr uint32_t invalidCrossTileID = std::numeric_limits::max(); + +protected: +#if MLN_SYMBOL_GUARDS + bool check(std::uint64_t v, int n, const std::source_location&) const; + bool checkKey(const std::source_location&) const; + void forceFailInternal(); // this is just to avoid warnings about the values never being set +#else + bool checkKey(std::string_view) const { return true; } +#endif + private: std::shared_ptr sharedData; -public: + static constexpr std::uint64_t checkVal = 0x123456780ABCDEFFULL; + + SYM_GUARD_VALUE(01) Anchor anchor; + SYM_GUARD_VALUE(02) SymbolContent symbolContent; + SYM_GUARD_VALUE(03) std::size_t rightJustifiedGlyphQuadsSize; + SYM_GUARD_VALUE(04) std::size_t centerJustifiedGlyphQuadsSize; + SYM_GUARD_VALUE(05) std::size_t leftJustifiedGlyphQuadsSize; + SYM_GUARD_VALUE(06) std::size_t verticalGlyphQuadsSize; + SYM_GUARD_VALUE(07) std::size_t iconQuadsSize; + SYM_GUARD_VALUE(08) CollisionFeature textCollisionFeature; + SYM_GUARD_VALUE(09) CollisionFeature iconCollisionFeature; + SYM_GUARD_VALUE(10) std::optional verticalTextCollisionFeature = std::nullopt; + SYM_GUARD_VALUE(11) std::optional verticalIconCollisionFeature = std::nullopt; + SYM_GUARD_VALUE(12) WritingModeType writingModes; + SYM_GUARD_VALUE(13) 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 + SYM_GUARD_VALUE(14) + std::size_t dataFeatureIndex; // Index into the underlying tile data feature set + SYM_GUARD_VALUE(15) std::array textOffset; + SYM_GUARD_VALUE(16) std::array iconOffset; + SYM_GUARD_VALUE(17) std::u16string key; + SYM_GUARD_VALUE(18) std::optional placedRightTextIndex; + SYM_GUARD_VALUE(19) std::optional placedCenterTextIndex; + SYM_GUARD_VALUE(20) std::optional placedLeftTextIndex; + SYM_GUARD_VALUE(21) std::optional placedVerticalTextIndex; + SYM_GUARD_VALUE(22) std::optional placedIconIndex; + SYM_GUARD_VALUE(23) std::optional placedVerticalIconIndex; + SYM_GUARD_VALUE(24) float textBoxScale; + SYM_GUARD_VALUE(25) std::array variableTextOffset; + SYM_GUARD_VALUE(26) bool singleLine; + SYM_GUARD_VALUE(27) uint32_t crossTileID = 0; - - static constexpr uint32_t invalidCrossTileID() { return std::numeric_limits::max(); } + SYM_GUARD_VALUE(28) +#if MLN_SYMBOL_GUARDS + mutable bool isFailed = false; +#endif }; +// NOLINTEND(clang-analyzer-optin.performance.Padding) + using SymbolInstanceReferences = std::vector>; } // namespace mbgl diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index 5366bd658..9ab466e13 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -264,7 +265,7 @@ Shaping& shapingForTextJustifyType(ShapedTextOrientations& shapedTextOrientation std::array evaluateRadialOffset(style::SymbolAnchorType anchor, float radialOffset) { std::array result{{0.0f, 0.0f}}; - if (radialOffset < 0.0f) radialOffset = 0.0f; // Ignore negative offset. + radialOffset = std::max(radialOffset, 0.0f); // Ignore negative offset. // solve for r where r^2 + r^2 = radialOffset^2 const float hypotenuse = radialOffset / std::numbers::sqrt2_v; @@ -520,7 +521,7 @@ void SymbolLayout::prepareSymbols(const GlyphMap& glyphMap, // if either shapedText or icon position is present, add the feature const Shaping& defaultShaping = getDefaultHorizontalShaping(shapedTextOrientations); - iconsInText = defaultShaping ? defaultShaping.iconsInText : false; + iconsInText = defaultShaping && defaultShaping.iconsInText; if (defaultShaping || shapedIcon) { addFeature(std::distance(features.begin(), it), feature, @@ -569,7 +570,7 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex, const float textMaxAngle = util::deg2radf(layout->get()); const float iconRotation = layout->evaluate(zoom, feature, canonicalID); const float textRotation = layout->evaluate(zoom, feature, canonicalID); - std::array variableTextOffset; + std::array variableTextOffset = {0, 0}; if (!textRadialOffset.isUndefined()) { variableTextOffset = { {layout->evaluate(zoom, feature, canonicalID) * util::ONE_EM, INVALID_OFFSET_VALUE}}; @@ -815,11 +816,15 @@ void SymbolLayout::createBucket(const ImagePositions&, iconsInText); for (SymbolInstance& symbolInstance : bucket->symbolInstances) { + if (!symbolInstance.check(SYM_GUARD_LOC)) { + continue; + } + const bool hasText = symbolInstance.hasText(); const bool hasIcon = symbolInstance.hasIcon(); - const bool singleLine = symbolInstance.singleLine; + const bool singleLine = symbolInstance.getSingleLine(); - const auto& feature = features.at(symbolInstance.layoutFeatureIndex); + const auto& feature = features.at(symbolInstance.getLayoutFeatureIndex()); // Insert final placement into collision tree and add glyphs/icons to buffers @@ -830,11 +835,11 @@ void SymbolLayout::createBucket(const ImagePositions&, const Range sizeData = bucket->iconSizeBinder->getVertexSizeData(feature); auto& iconBuffer = symbolInstance.hasSdfIcon() ? bucket->sdfIcon : bucket->icon; const auto placeIcon = [&](const SymbolQuads& iconQuads, auto& index, const WritingModeType writingMode) { - iconBuffer.placedSymbols.emplace_back(symbolInstance.anchor.point, - symbolInstance.anchor.segment.value_or(0u), + iconBuffer.placedSymbols.emplace_back(symbolInstance.getAnchor().point, + symbolInstance.getAnchor().segment.value_or(0u), sizeData.min, sizeData.max, - symbolInstance.iconOffset, + symbolInstance.getIconOffset(), writingMode, symbolInstance.line(), std::vector()); @@ -844,19 +849,21 @@ void SymbolLayout::createBucket(const ImagePositions&, ? pi_v / 2 : 0.0f; iconSymbol.vertexStartIndex = addSymbols( - iconBuffer, sizeData, iconQuads, symbolInstance.anchor, iconSymbol, feature.sortKey); + iconBuffer, sizeData, iconQuads, symbolInstance.getAnchor(), iconSymbol, feature.sortKey); }; - placeIcon(*symbolInstance.iconQuads(), symbolInstance.placedIconIndex, WritingModeType::None); + placeIcon(*symbolInstance.iconQuads(), symbolInstance.refPlacedIconIndex(), WritingModeType::None); + if (symbolInstance.verticalIconQuads()) { placeIcon(*symbolInstance.verticalIconQuads(), - symbolInstance.placedVerticalIconIndex, + symbolInstance.refPlacedVerticalIconIndex(), WritingModeType::Vertical); + symbolInstance.check(SYM_GUARD_LOC); } for (auto& pair : bucket->paintProperties) { pair.second.iconBinders.populateVertexVectors( - feature, iconBuffer.vertices().elements(), symbolInstance.dataFeatureIndex, {}, {}, canonical); + feature, iconBuffer.vertices().elements(), symbolInstance.getDataFeatureIndex(), {}, {}, canonical); } } @@ -867,56 +874,58 @@ void SymbolLayout::createBucket(const ImagePositions&, lastAddedSection = addSymbolGlyphQuads(*bucket, symbolInstance, feature, - symbolInstance.writingModes, + symbolInstance.getWritingModes(), placedTextIndex, symbolInstance.rightJustifiedGlyphQuads(), canonical, lastAddedSection); - symbolInstance.placedRightTextIndex = placedTextIndex; - symbolInstance.placedCenterTextIndex = placedTextIndex; - symbolInstance.placedLeftTextIndex = placedTextIndex; + symbolInstance.setPlacedRightTextIndex(placedTextIndex); + symbolInstance.setPlacedCenterTextIndex(placedTextIndex); + symbolInstance.setPlacedLeftTextIndex(placedTextIndex); } else { - if (symbolInstance.rightJustifiedGlyphQuadsSize) { + if (symbolInstance.getRightJustifiedGlyphQuadsSize()) { lastAddedSection = addSymbolGlyphQuads(*bucket, symbolInstance, feature, - symbolInstance.writingModes, - symbolInstance.placedRightTextIndex, + symbolInstance.getWritingModes(), + symbolInstance.refPlacedRightTextIndex(), symbolInstance.rightJustifiedGlyphQuads(), canonical, lastAddedSection); } - if (symbolInstance.centerJustifiedGlyphQuadsSize) { + if (symbolInstance.getCenterJustifiedGlyphQuadsSize()) { lastAddedSection = addSymbolGlyphQuads(*bucket, symbolInstance, feature, - symbolInstance.writingModes, - symbolInstance.placedCenterTextIndex, + symbolInstance.getWritingModes(), + symbolInstance.refPlacedCenterTextIndex(), symbolInstance.centerJustifiedGlyphQuads(), canonical, lastAddedSection); } - if (symbolInstance.leftJustifiedGlyphQuadsSize) { + if (symbolInstance.getLeftJustifiedGlyphQuadsSize()) { lastAddedSection = addSymbolGlyphQuads(*bucket, symbolInstance, feature, - symbolInstance.writingModes, - symbolInstance.placedLeftTextIndex, + symbolInstance.getWritingModes(), + symbolInstance.refPlacedLeftTextIndex(), symbolInstance.leftJustifiedGlyphQuads(), canonical, lastAddedSection); } } - if (symbolInstance.writingModes & WritingModeType::Vertical && symbolInstance.verticalGlyphQuadsSize) { + if ((symbolInstance.getWritingModes() & WritingModeType::Vertical) && + symbolInstance.getVerticalGlyphQuadsSize()) { lastAddedSection = addSymbolGlyphQuads(*bucket, symbolInstance, feature, WritingModeType::Vertical, - symbolInstance.placedVerticalTextIndex, + symbolInstance.refPlacedVerticalTextIndex(), symbolInstance.verticalGlyphQuads(), canonical, lastAddedSection); } + symbolInstance.check(SYM_GUARD_LOC); assert(lastAddedSection); // True, as hasText == true; updatePaintPropertiesForSection(*bucket, feature, *lastAddedSection, canonical); } @@ -958,16 +967,16 @@ std::size_t SymbolLayout::addSymbolGlyphQuads(SymbolBucket& bucket, std::optional lastAddedSection) { const Range sizeData = bucket.textSizeBinder->getVertexSizeData(feature); const bool hasFormatSectionOverrides = bucket.hasFormatSectionOverrides(); - const auto& placedIconIndex = writingMode == WritingModeType::Vertical ? symbolInstance.placedVerticalIconIndex - : symbolInstance.placedIconIndex; - bucket.text.placedSymbols.emplace_back(symbolInstance.anchor.point, - symbolInstance.anchor.segment.value_or(0u), + const auto& placedIconIndex = writingMode == WritingModeType::Vertical ? symbolInstance.getPlacedVerticalIconIndex() + : symbolInstance.getPlacedIconIndex(); + bucket.text.placedSymbols.emplace_back(symbolInstance.getAnchor().point, + symbolInstance.getAnchor().segment.value_or(0u), sizeData.min, sizeData.max, - symbolInstance.textOffset, + symbolInstance.getTextOffset(), writingMode, symbolInstance.line(), - calculateTileDistances(symbolInstance.line(), symbolInstance.anchor), + calculateTileDistances(symbolInstance.line(), symbolInstance.getAnchor()), placedIconIndex); placedIndex = bucket.text.placedSymbols.size() - 1; PlacedSymbol& placedSymbol = bucket.text.placedSymbols.back(); @@ -981,8 +990,8 @@ std::size_t SymbolLayout::addSymbolGlyphQuads(SymbolBucket& bucket, } lastAddedSection = symbolQuad.sectionIndex; } - size_t index = addSymbol( - bucket.text, sizeData, symbolQuad, symbolInstance.anchor, placedSymbol, feature.sortKey); + const std::size_t index = addSymbol( + bucket.text, sizeData, symbolQuad, symbolInstance.getAnchor(), placedSymbol, feature.sortKey); if (firstSymbol) { placedSymbol.vertexStartIndex = index; firstSymbol = false; @@ -1069,7 +1078,7 @@ size_t SymbolLayout::addSymbol(SymbolBucket::Buffer& buffer, buffer.dynamicVertices().emplace_back(dynamicVertex); buffer.dynamicVertices().emplace_back(dynamicVertex); - auto opacityVertex = SymbolSDFIconProgram::opacityVertex(1.0, 1.0); + auto opacityVertex = SymbolSDFIconProgram::opacityVertex(true, 1.0); buffer.opacityVertices().emplace_back(opacityVertex); buffer.opacityVertices().emplace_back(opacityVertex); buffer.opacityVertices().emplace_back(opacityVertex); @@ -1145,13 +1154,13 @@ void SymbolLayout::addToDebugBuffers(SymbolBucket& bucket) { auto index = static_cast(segment.vertexLength); collisionBuffer.vertices().emplace_back( - CollisionBoxProgram::layoutVertex(anchor, symbolInstance.anchor.point, tl)); + CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, tl)); collisionBuffer.vertices().emplace_back( - CollisionBoxProgram::layoutVertex(anchor, symbolInstance.anchor.point, tr)); + CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, tr)); collisionBuffer.vertices().emplace_back( - CollisionBoxProgram::layoutVertex(anchor, symbolInstance.anchor.point, br)); + CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, br)); collisionBuffer.vertices().emplace_back( - CollisionBoxProgram::layoutVertex(anchor, symbolInstance.anchor.point, bl)); + CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, bl)); // Dynamic vertices are initialized so that the vertex count // always agrees with the layout vertex buffer, but they will @@ -1178,14 +1187,16 @@ void SymbolLayout::addToDebugBuffers(SymbolBucket& bucket) { segment.indexLength += indexLength; } }; - populateCollisionBox(symbolInstance.textCollisionFeature, true /*isText*/); - if (symbolInstance.verticalTextCollisionFeature) { - populateCollisionBox(*symbolInstance.verticalTextCollisionFeature, true /*isText*/); + symbolInstance.check(SYM_GUARD_LOC); + populateCollisionBox(symbolInstance.getTextCollisionFeature(), true /*isText*/); + if (symbolInstance.getVerticalTextCollisionFeature()) { + populateCollisionBox(*symbolInstance.getVerticalTextCollisionFeature(), true /*isText*/); } - if (symbolInstance.verticalIconCollisionFeature) { - populateCollisionBox(*symbolInstance.verticalIconCollisionFeature, false /*isText*/); + if (symbolInstance.getVerticalIconCollisionFeature()) { + populateCollisionBox(*symbolInstance.getVerticalIconCollisionFeature(), false /*isText*/); } - populateCollisionBox(symbolInstance.iconCollisionFeature, false /*isText*/); + populateCollisionBox(symbolInstance.getIconCollisionFeature(), false /*isText*/); + symbolInstance.check(SYM_GUARD_LOC); } } diff --git a/src/mbgl/renderer/bucket.hpp b/src/mbgl/renderer/bucket.hpp index ed67a7bfa..15b7f3b5d 100644 --- a/src/mbgl/renderer/bucket.hpp +++ b/src/mbgl/renderer/bucket.hpp @@ -1,9 +1,10 @@ #pragma once -#include -#include +#include #include +#include #include +#include #if MLN_DRAWABLE_RENDERER #include @@ -74,6 +75,15 @@ public: const util::SimpleIdentity& getID() const { return bucketID; } #endif +#if MLN_SYMBOL_GUARDS + virtual bool check(std::source_location) { return true; } +#else + bool check(std::string_view = {}) { return true; } +#endif + + const std::optional& getRenderThreadID() const { return renderThreadID; } + void setRenderThreadID(std::optional id) { renderThreadID = id; } + protected: Bucket() = default; std::atomic uploaded{false}; @@ -81,6 +91,8 @@ protected: #if MLN_DRAWABLE_RENDERER util::SimpleIdentity bucketID; #endif + + std::optional renderThreadID; }; } // namespace mbgl diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp index 2ee049653..98428132a 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.cpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp @@ -263,31 +263,37 @@ void SymbolBucket::sortFeatures(const float angle) { // position. The index array buffer is rewritten to reference the // (unchanged) vertices in the sorted order. for (const SymbolInstance& symbolInstance : getSortedSymbols(angle)) { - symbolsSortOrder->push_back(symbolInstance.dataFeatureIndex); + if (!symbolInstance.check(SYM_GUARD_LOC) || + !symbolInstance.checkIndexes( + text.placedSymbols.size(), icon.placedSymbols.size(), sdfIcon.placedSymbols.size(), SYM_GUARD_LOC)) { + continue; + } + symbolsSortOrder->push_back(symbolInstance.getDataFeatureIndex()); - if (symbolInstance.placedRightTextIndex) { - addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.placedRightTextIndex]); + if (symbolInstance.getPlacedRightTextIndex()) { + addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.getPlacedRightTextIndex()]); } - if (symbolInstance.placedCenterTextIndex && !symbolInstance.singleLine) { - addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.placedCenterTextIndex]); + if (symbolInstance.getPlacedCenterTextIndex() && !symbolInstance.getSingleLine()) { + addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.getPlacedCenterTextIndex()]); } - if (symbolInstance.placedLeftTextIndex && !symbolInstance.singleLine) { - addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.placedLeftTextIndex]); + if (symbolInstance.getPlacedLeftTextIndex() && !symbolInstance.getSingleLine()) { + addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.getPlacedLeftTextIndex()]); } - if (symbolInstance.placedVerticalTextIndex) { - addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.placedVerticalTextIndex]); + if (symbolInstance.getPlacedVerticalTextIndex()) { + addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.getPlacedVerticalTextIndex()]); } auto& iconBuffer = symbolInstance.hasSdfIcon() ? sdfIcon : icon; - if (symbolInstance.placedIconIndex) { - addPlacedSymbol(iconBuffer.triangles, iconBuffer.placedSymbols[*symbolInstance.placedIconIndex]); + if (symbolInstance.getPlacedIconIndex()) { + addPlacedSymbol(iconBuffer.triangles, iconBuffer.placedSymbols[*symbolInstance.getPlacedIconIndex()]); } - if (symbolInstance.placedVerticalIconIndex) { - addPlacedSymbol(iconBuffer.triangles, iconBuffer.placedSymbols[*symbolInstance.placedVerticalIconIndex]); + if (symbolInstance.getPlacedVerticalIconIndex()) { + addPlacedSymbol(iconBuffer.triangles, + iconBuffer.placedSymbols[*symbolInstance.getPlacedVerticalIconIndex()]); } } @@ -300,26 +306,49 @@ SymbolInstanceReferences SymbolBucket::getSortedSymbols(const float angle) const const float cos = std::cos(angle); std::sort(result.begin(), result.end(), [sin, cos](const SymbolInstance& a, const SymbolInstance& b) { - const auto aRotated = std::lround(sin * a.anchor.point.x + cos * a.anchor.point.y); - const auto bRotated = std::lround(sin * b.anchor.point.x + cos * b.anchor.point.y); + const auto aRotated = std::lround(sin * a.getAnchor().point.x + cos * a.getAnchor().point.y); + const auto bRotated = std::lround(sin * b.getAnchor().point.x + cos * b.getAnchor().point.y); if (aRotated != bRotated) { return aRotated < bRotated; } - return a.dataFeatureIndex > b.dataFeatureIndex; // aRotated == bRotated + return a.getDataFeatureIndex() > b.getDataFeatureIndex(); // aRotated == bRotated }); return result; } SymbolInstanceReferences SymbolBucket::getSymbols(const std::optional& range) const { - if (!range) return SymbolInstanceReferences(symbolInstances.begin(), symbolInstances.end()); - assert(range->start < range->end); - assert(range->end <= symbolInstances.size()); - auto begin = symbolInstances.begin() + range->start; - auto end = symbolInstances.begin() + range->end; - return SymbolInstanceReferences(begin, end); + assert(!range || range->start < range->end); + assert(!range || range->end <= symbolInstances.size()); + if (!range || range->start >= range->end || range->end > symbolInstances.size()) { + return {symbolInstances.begin(), symbolInstances.end()}; + } + using offset_t = decltype(symbolInstances)::difference_type; + return {symbolInstances.begin() + static_cast(range->start), + symbolInstances.begin() + static_cast(range->end)}; } +#if MLN_SYMBOL_GUARDS +bool SymbolBucket::check(std::source_location source) { + if (text.vertices().elements() != text.dynamicVertices().elements() || + text.vertices().elements() != text.opacityVertices().elements() || + icon.vertices().elements() != icon.dynamicVertices().elements() || + icon.vertices().elements() != icon.opacityVertices().elements() || + sdfIcon.vertices().elements() != sdfIcon.dynamicVertices().elements() || + sdfIcon.vertices().elements() != sdfIcon.opacityVertices().elements()) { + // This bucket was left in a partial state and it cannot be used + return false; + } + + for (std::size_t i = 0; i < symbolInstances.size(); ++i) { + if (!symbolInstances[i].check(source)) { + return false; + } + } + return true; +} +#endif + bool SymbolBucket::hasFormatSectionOverrides() const { if (!hasFormatSectionOverrides_) { hasFormatSectionOverrides_ = SymbolLayerPaintPropertyOverrides::hasOverrides(layout->get()); diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp index 896eee45e..6fdfe1041 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.hpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp @@ -105,6 +105,10 @@ public: // |sortKeyRange| is `std::nullopt`. SymbolInstanceReferences getSymbols(const std::optional& sortKeyRange = std::nullopt) const; +#if MLN_SYMBOL_GUARDS + bool check(std::source_location) override; +#endif + Immutable layout; const std::string bucketLeaderID; float sortedAngle = std::numeric_limits::max(); diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp index 81b8b4471..f30bef482 100644 --- a/src/mbgl/renderer/layers/render_symbol_layer.cpp +++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp @@ -489,6 +489,10 @@ void RenderSymbolLayer::render(PaintParameters& parameters) { assert(bucket.paintProperties.find(getID()) != bucket.paintProperties.end()); const auto& bucketPaintProperties = bucket.paintProperties.at(getID()); + if (!static_cast(bucket).check(SYM_GUARD_LOC)) { + continue; + } + // Prevent a flickering issue when a symbol is moved. // bucket.justReloaded = false; @@ -693,7 +697,7 @@ void RenderSymbolLayer::prepare(const LayerPrepareParameters& params) { for (const RenderTile& renderTile : *renderTiles) { auto* bucket = static_cast(renderTile.getBucket(*baseImpl)); - if (bucket && bucket->bucketLeaderID == getID()) { + if (bucket && bucket->bucketLeaderID == getID() && static_cast(bucket)->check(SYM_GUARD_LOC)) { // Only place this layer if it's the "group leader" for the bucket const Tile* tile = params.source->getRenderedTile(renderTile.id); assert(tile); @@ -1059,7 +1063,8 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders, const auto& tileID = tile.getOverscaledTileID(); const auto* optRenderData = getRenderDataForPass(tile, passes); - if (!optRenderData || !optRenderData->bucket || !optRenderData->bucket->hasData()) { + if (!optRenderData || !optRenderData->bucket || !optRenderData->bucket->hasData() || + !optRenderData->bucket->check(SYM_GUARD_LOC)) { removeTile(passes, tileID); continue; } diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp index df1534c7c..e6ac09270 100644 --- a/src/mbgl/text/cross_tile_symbol_index.cpp +++ b/src/mbgl/text/cross_tile_symbol_index.cpp @@ -16,9 +16,12 @@ TileLayerIndex::TileLayerIndex(OverscaledTileID coord_, bucketInstanceId(bucketInstanceId_), bucketLeaderId(std::move(bucketLeaderId_)) { for (const SymbolInstance& symbolInstance : symbolInstances) { - if (symbolInstance.crossTileID == SymbolInstance::invalidCrossTileID()) continue; - indexedSymbolInstances[symbolInstance.key].emplace_back(symbolInstance.crossTileID, - getScaledCoordinates(symbolInstance, coord)); + if (!symbolInstance.check(SYM_GUARD_LOC) || + symbolInstance.getCrossTileID() == SymbolInstance::invalidCrossTileID) { + continue; + } + indexedSymbolInstances[symbolInstance.getKey()].emplace_back(symbolInstance.getCrossTileID(), + getScaledCoordinates(symbolInstance, coord)); } } @@ -28,9 +31,9 @@ Point TileLayerIndex::getScaledCoordinates(const SymbolInstance& symbol const double roundingFactor = 512.0 / util::EXTENT / 2.0; const double scale = roundingFactor / std::pow(2, childTileCoord.canonical.z - coord.canonical.z); return {static_cast( - std::floor((childTileCoord.canonical.x * util::EXTENT + symbolInstance.anchor.point.x) * scale)), + std::floor((childTileCoord.canonical.x * util::EXTENT + symbolInstance.getAnchor().point.x) * scale)), static_cast( - std::floor((childTileCoord.canonical.y * util::EXTENT + symbolInstance.anchor.point.y) * scale))}; + std::floor((childTileCoord.canonical.y * util::EXTENT + symbolInstance.getAnchor().point.y) * scale))}; } void TileLayerIndex::findMatches(SymbolBucket& bucket, @@ -44,12 +47,12 @@ void TileLayerIndex::findMatches(SymbolBucket& bucket, if (bucket.bucketLeaderID != bucketLeaderId) return; for (auto& symbolInstance : symbolInstances) { - if (symbolInstance.crossTileID) { + if (symbolInstance.getCrossTileID() || !symbolInstance.check(SYM_GUARD_LOC)) { // already has a match, skip continue; } - auto it = indexedSymbolInstances.find(symbolInstance.key); + auto it = indexedSymbolInstances.find(symbolInstance.getKey()); if (it == indexedSymbolInstances.end()) { // No symbol with this key in this bucket continue; @@ -67,7 +70,7 @@ void TileLayerIndex::findMatches(SymbolBucket& bucket, // symbol, don't let any other symbols at the same zoom level // duplicate against the same parent (see issue #10844) zoomCrossTileIDs.insert(thisTileSymbol.crossTileID); - symbolInstance.crossTileID = thisTileSymbol.crossTileID; + symbolInstance.setCrossTileID(thisTileSymbol.crossTileID); break; } } @@ -142,16 +145,16 @@ bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, // For overscaled tiles the viewport might be showing only a small part of the tile, // so we filter out the off-screen symbols to improve the performance. for (auto& symbolInstance : bucket.symbolInstances) { - if (isInVewport(tileMatrix, symbolInstance.anchor.point)) { - symbolInstance.crossTileID = 0u; + if (symbolInstance.check(SYM_GUARD_LOC) && isInVewport(tileMatrix, symbolInstance.getAnchor().point)) { + symbolInstance.setCrossTileID(0u); } else { - symbolInstance.crossTileID = SymbolInstance::invalidCrossTileID(); + symbolInstance.setCrossTileID(SymbolInstance::invalidCrossTileID); bucket.hasUninitializedSymbols = true; } } } else { for (auto& symbolInstance : bucket.symbolInstances) { - symbolInstance.crossTileID = 0u; + symbolInstance.setCrossTileID(0u); } } @@ -176,10 +179,10 @@ bool CrossTileSymbolLayerIndex::addBucket(const OverscaledTileID& tileID, } for (auto& symbolInstance : bucket.symbolInstances) { - if (!symbolInstance.crossTileID) { + if (symbolInstance.check(SYM_GUARD_LOC) && !symbolInstance.getCrossTileID()) { // symbol did not match any known symbol, assign a new id - symbolInstance.crossTileID = ++maxCrossTileID; - thisZoomUsedCrossTileIDs.insert(symbolInstance.crossTileID); + symbolInstance.setCrossTileID(++maxCrossTileID); + thisZoomUsedCrossTileIDs.insert(symbolInstance.getCrossTileID()); } } diff --git a/src/mbgl/text/cross_tile_symbol_index.hpp b/src/mbgl/text/cross_tile_symbol_index.hpp index ae7d40fc3..4b58aab90 100644 --- a/src/mbgl/text/cross_tile_symbol_index.hpp +++ b/src/mbgl/text/cross_tile_symbol_index.hpp @@ -11,6 +11,7 @@ #include #include #include +#include #include namespace mbgl { @@ -76,6 +77,8 @@ public: void reset(); + const std::thread::id renderThreadID = std::this_thread::get_id(); + private: std::map layerIndexes; uint32_t maxCrossTileID = 0; diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index f43c6cad8..8b81764ad 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -65,7 +65,7 @@ const CollisionGroups::CollisionGroup& CollisionGroups::get(const std::string& s using namespace style; -// PlacementContext implemenation +// PlacementContext implementation class PlacementContext { std::reference_wrapper bucket; std::reference_wrapper renderTile; @@ -249,12 +249,13 @@ void Placement::placeSymbolBucket(const BucketPlacementData& params, std::setplacedOrientations.find(symbolInstance.crossTileID); + auto prevOrientation = getPrevPlacement()->placedOrientations.find(symbolInstance.getCrossTileID()); if (prevOrientation != getPrevPlacement()->placedOrientations.end()) { - placedOrientations[symbolInstance.crossTileID] = prevOrientation->second; + placedOrientations[symbolInstance.getCrossTileID()] = prevOrientation->second; } } }; const auto placeTextForPlacementModes = [&](auto& placeHorizontalFn, auto& placeVerticalFn) { - if (bucket.allowVerticalPlacement && symbolInstance.writingModes & WritingModeType::Vertical) { + if (bucket.allowVerticalPlacement && symbolInstance.getWritingModes() & WritingModeType::Vertical) { assert(!bucket.placementModes.empty()); for (auto& placementMode : bucket.placementModes) { if (placementMode == TextWritingModeType::Vertical) { @@ -346,18 +348,18 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons collisionGroup.second, textBoxes); if (placedFeature.first) { - placedOrientations.emplace(symbolInstance.crossTileID, orientation); + placedOrientations.emplace(symbolInstance.getCrossTileID(), orientation); } return placedFeature; }; const auto placeHorizontal = [&] { - return placeFeature(symbolInstance.textCollisionFeature, style::TextWritingModeType::Horizontal); + return placeFeature(symbolInstance.getTextCollisionFeature(), style::TextWritingModeType::Horizontal); }; const auto placeVertical = [&] { - if (bucket.allowVerticalPlacement && symbolInstance.verticalTextCollisionFeature) { - return placeFeature(*symbolInstance.verticalTextCollisionFeature, + if (bucket.allowVerticalPlacement && symbolInstance.getVerticalTextCollisionFeature()) { + return placeFeature(*symbolInstance.getVerticalTextCollisionFeature(), style::TextWritingModeType::Vertical); } return std::pair{false, false}; @@ -368,13 +370,13 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons placeText = placed.first; offscreen &= placed.second; - } else if (!symbolInstance.textCollisionFeature.alongLine && - !symbolInstance.textCollisionFeature.boxes.empty()) { + } else if (!symbolInstance.getTextCollisionFeature().alongLine && + !symbolInstance.getTextCollisionFeature().boxes.empty()) { // If this symbol was in the last placement, shift the previously // used anchor to the front of the anchor list, only if the previous // anchor is still in the anchor list. if (getPrevPlacement()) { - auto prevOffset = getPrevPlacement()->variableOffsets.find(symbolInstance.crossTileID); + auto prevOffset = getPrevPlacement()->variableOffsets.find(symbolInstance.getCrossTileID()); if (prevOffset != getPrevPlacement()->variableOffsets.end()) { const auto prevAnchor = prevOffset->second.anchor; auto found = std::find(variableTextAnchors.begin(), variableTextAnchors.end(), prevAnchor); @@ -393,14 +395,14 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons } const bool doVariableIconPlacement = ctx.hasIconTextFit && !ctx.iconAllowOverlap && - symbolInstance.placedIconIndex; + symbolInstance.getPlacedIconIndex(); const auto placeFeatureForVariableAnchors = [&](const CollisionFeature& textCollisionFeature, style::TextWritingModeType orientation, const CollisionFeature& iconCollisionFeature) { const CollisionBox& textBox = textCollisionFeature.boxes[0]; const float width = textBox.x2 - textBox.x1; const float height = textBox.y2 - textBox.y1; - const float textBoxScale = symbolInstance.textBoxScale; + const float textBoxScale = symbolInstance.getTextBoxScale(); std::pair placedFeature = {false, false}; const size_t anchorsSize = variableTextAnchors.size(); const size_t placementAttempts = ctx.textAllowOverlap ? anchorsSize * 2 : anchorsSize; @@ -413,7 +415,7 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons shift = calculateVariableLayoutOffset(anchor, width, height, - symbolInstance.variableTextOffset, + symbolInstance.getVariableTextOffset(), textBoxScale, ctx.rotateTextWithMap, ctx.pitchTextWithMap, @@ -460,15 +462,15 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons } if (placedFeature.first) { - assert(symbolInstance.crossTileID != 0u); + assert(symbolInstance.getCrossTileID() != 0u); std::optional prevAnchor; // If this label was placed in the previous // placement, record the anchor position to allow us // to animate the transition if (getPrevPlacement()) { - auto prevOffset = getPrevPlacement()->variableOffsets.find(symbolInstance.crossTileID); - auto prevPlacements = getPrevPlacement()->placements.find(symbolInstance.crossTileID); + auto prevOffset = getPrevPlacement()->variableOffsets.find(symbolInstance.getCrossTileID()); + auto prevPlacements = getPrevPlacement()->placements.find(symbolInstance.getCrossTileID()); if (prevOffset != getPrevPlacement()->variableOffsets.end() && prevPlacements != getPrevPlacement()->placements.end() && prevPlacements->second.text) { // TODO: The prevAnchor seems to be unused, needs to be fixed. @@ -476,13 +478,16 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons } } - variableOffsets.insert(std::make_pair( - symbolInstance.crossTileID, - VariableOffset{ - symbolInstance.variableTextOffset, width, height, anchor, textBoxScale, prevAnchor})); + variableOffsets.insert(std::make_pair(symbolInstance.getCrossTileID(), + VariableOffset{symbolInstance.getVariableTextOffset(), + width, + height, + anchor, + textBoxScale, + prevAnchor})); if (bucket.allowVerticalPlacement) { - placedOrientations.emplace(symbolInstance.crossTileID, orientation); + placedOrientations.emplace(symbolInstance.getCrossTileID(), orientation); } break; } @@ -492,18 +497,19 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons }; const auto placeHorizontal = [&] { - return placeFeatureForVariableAnchors(symbolInstance.textCollisionFeature, + return placeFeatureForVariableAnchors(symbolInstance.getTextCollisionFeature(), style::TextWritingModeType::Horizontal, - symbolInstance.iconCollisionFeature); + symbolInstance.getIconCollisionFeature()); }; const auto placeVertical = [&] { - if (bucket.allowVerticalPlacement && !placed.first && symbolInstance.verticalTextCollisionFeature) { - return placeFeatureForVariableAnchors(*symbolInstance.verticalTextCollisionFeature, + if (bucket.allowVerticalPlacement && !placed.first && + symbolInstance.getVerticalTextCollisionFeature()) { + return placeFeatureForVariableAnchors(*symbolInstance.getVerticalTextCollisionFeature(), style::TextWritingModeType::Vertical, - symbolInstance.verticalIconCollisionFeature - ? *symbolInstance.verticalIconCollisionFeature - : symbolInstance.iconCollisionFeature); + symbolInstance.getVerticalTextCollisionFeature() + ? *symbolInstance.getVerticalTextCollisionFeature() + : symbolInstance.getIconCollisionFeature()); } return std::pair{false, false}; }; @@ -518,21 +524,21 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons // If we didn't get placed, we still need to copy our position from // the last placement for fade animations if (!placeText && getPrevPlacement()) { - auto prevOffset = getPrevPlacement()->variableOffsets.find(symbolInstance.crossTileID); + auto prevOffset = getPrevPlacement()->variableOffsets.find(symbolInstance.getCrossTileID()); if (prevOffset != getPrevPlacement()->variableOffsets.end()) { - variableOffsets[symbolInstance.crossTileID] = prevOffset->second; + variableOffsets[symbolInstance.getCrossTileID()] = prevOffset->second; } } } } - if (symbolInstance.placedIconIndex) { + if (symbolInstance.getPlacedIconIndex()) { if (!ctx.hasIconTextFit || !placeText || variableTextAnchors.empty()) { shift = {0.0f, 0.0f}; } const auto& iconBuffer = symbolInstance.hasSdfIcon() ? bucket.sdfIcon : bucket.icon; - const PlacedSymbol& placedSymbol = iconBuffer.placedSymbols.at(*symbolInstance.placedIconIndex); + const PlacedSymbol& placedSymbol = iconBuffer.placedSymbols.at(*symbolInstance.getPlacedIconIndex()); const float fontSize = evaluateSizeForFeature(ctx.partiallyEvaluatedIconSize, placedSymbol); const auto& placeIconFeature = [&](const CollisionFeature& collisionFeature) { return collisionIndex.placeFeature(collisionFeature, @@ -551,11 +557,11 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons iconBoxes); }; - std::pair placedIcon = {false, false}; - if (placedVerticalText.first && symbolInstance.verticalIconCollisionFeature) { - placedIcon = placedVerticalIcon = placeIconFeature(*symbolInstance.verticalIconCollisionFeature); + std::pair placedIcon; + if (placedVerticalText.first && symbolInstance.getVerticalIconCollisionFeature()) { + placedIcon = placedVerticalIcon = placeIconFeature(*symbolInstance.getVerticalIconCollisionFeature()); } else { - placedIcon = placeIconFeature(symbolInstance.iconCollisionFeature); + placedIcon = placeIconFeature(symbolInstance.getIconCollisionFeature()); } placeIcon = placedIcon.first; offscreen &= placedIcon.second; @@ -574,14 +580,14 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons } if (placeText) { - if (placedVerticalText.first && symbolInstance.verticalTextCollisionFeature) { - collisionIndex.insertFeature(*symbolInstance.verticalTextCollisionFeature, + if (placedVerticalText.first && symbolInstance.getVerticalTextCollisionFeature()) { + collisionIndex.insertFeature(*symbolInstance.getVerticalTextCollisionFeature(), textBoxes, ctx.getLayout().get(), bucket.bucketInstanceId, collisionGroup.first); } else { - collisionIndex.insertFeature(symbolInstance.textCollisionFeature, + collisionIndex.insertFeature(symbolInstance.getTextCollisionFeature(), textBoxes, ctx.getLayout().get(), bucket.bucketInstanceId, @@ -590,14 +596,14 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons } if (placeIcon) { - if (placedVerticalIcon.first && symbolInstance.verticalIconCollisionFeature) { - collisionIndex.insertFeature(*symbolInstance.verticalIconCollisionFeature, + if (placedVerticalIcon.first && symbolInstance.getVerticalIconCollisionFeature()) { + collisionIndex.insertFeature(*symbolInstance.getVerticalIconCollisionFeature(), iconBoxes, ctx.getLayout().get(), bucket.bucketInstanceId, collisionGroup.first); } else { - collisionIndex.insertFeature(symbolInstance.iconCollisionFeature, + collisionIndex.insertFeature(symbolInstance.getIconCollisionFeature(), iconBoxes, ctx.getLayout().get(), bucket.bucketInstanceId, @@ -608,25 +614,34 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons const bool hasIconCollisionCircleData = bucket.hasIconCollisionCircleData(); const bool hasTextCollisionCircleData = bucket.hasTextCollisionCircleData(); - if (hasIconCollisionCircleData && symbolInstance.iconCollisionFeature.alongLine && !iconBoxes.empty()) { - collisionCircles[&symbolInstance.iconCollisionFeature] = iconBoxes; + if (hasIconCollisionCircleData && symbolInstance.getIconCollisionFeature().alongLine && !iconBoxes.empty()) { + collisionCircles[&symbolInstance.getIconCollisionFeature()] = iconBoxes; } - if (hasTextCollisionCircleData && symbolInstance.textCollisionFeature.alongLine && !textBoxes.empty()) { - collisionCircles[&symbolInstance.textCollisionFeature] = textBoxes; + if (hasTextCollisionCircleData && symbolInstance.getTextCollisionFeature().alongLine && !textBoxes.empty()) { + collisionCircles[&symbolInstance.getTextCollisionFeature()] = textBoxes; } - assert(symbolInstance.crossTileID != 0); + if (!symbolInstance.check(SYM_GUARD_LOC)) { + return kUnplaced; + } - if (placements.find(symbolInstance.crossTileID) != placements.end()) { - // If there's a previous placement with this ID, it comes from a tile - // that's fading out Erase it so that the placement result from the - // non-fading tile supersedes it - placements.erase(symbolInstance.crossTileID); + if (symbolInstance.getCrossTileID() != 0) { + const auto hit = placements.find(symbolInstance.getCrossTileID()); + if (hit != placements.end()) { + // If there's a previous placement with this ID, it comes from a tile that's fading out + // Erase it so that the placement result from the non-fading tile supersedes it + placements.erase(hit); + } + } else { + assert(false); + // We skipped some setup, don't use this one or we might run into inconsistencies + symbolInstance.forceFail(); + return kUnplaced; } JointPlacement result( placeText || ctx.alwaysShowText, placeIcon || ctx.alwaysShowIcon, offscreen || bucket.justReloaded); - placements.emplace(symbolInstance.crossTileID, result); + placements.emplace(symbolInstance.getCrossTileID(), result); newSymbolPlaced(symbolInstance, ctx, result, ctx.placementType, textBoxes, iconBoxes); return result; } @@ -664,7 +679,7 @@ SymbolInstanceReferences Placement::getSortedSymbols(const BucketPlacementData& } if (!bPlacement) { // a < b, if 'b' is new and 'a' was previously shown. - return aPlacement && aPlacement->placed(); + return aPlacement->placed(); } // a < b, if 'a' was shown and 'b' was hidden. return aPlacement->placed() && !bPlacement->placed(); @@ -757,7 +772,7 @@ Point calculateVariableRenderShift(style::SymbolAnchorType anchor, const AnchorAlignment alignment = AnchorAlignment::getAnchorAlignment(anchor); const float shiftX = -(alignment.horizontalAlign - 0.5f) * width; const float shiftY = -(alignment.verticalAlign - 0.5f) * height; - const auto variableOffset = SymbolLayout::evaluateVariableOffset(anchor, textOffset); + auto variableOffset = SymbolLayout::evaluateVariableOffset(anchor, textOffset); return {(shiftX / textBoxScale + variableOffset[0]) * renderTextSize, (shiftY / textBoxScale + variableOffset[1]) * renderTextSize}; } @@ -983,9 +998,10 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, true); for (SymbolInstance& symbolInstance : bucket.symbolInstances) { - bool isDuplicate = seenCrossTileIDs.contains(symbolInstance.crossTileID); + if (!symbolInstance.check(SYM_GUARD_LOC)) continue; + bool isDuplicate = seenCrossTileIDs.contains(symbolInstance.getCrossTileID()); - auto it = opacities.find(symbolInstance.crossTileID); + auto it = opacities.find(symbolInstance.getCrossTileID()); auto opacityState = defaultOpacityState; if (isDuplicate) { opacityState = duplicateOpacityState; @@ -993,44 +1009,52 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, opacityState = it->second; } - seenCrossTileIDs.insert(symbolInstance.crossTileID); + seenCrossTileIDs.insert(symbolInstance.getCrossTileID()); + if (symbolInstance.hasText() || symbolInstance.hasIcon()) { + if (!symbolInstance.checkIndexes(bucket.text.placedSymbols.size(), + bucket.icon.placedSymbols.size(), + bucket.sdfIcon.placedSymbols.size(), + SYM_GUARD_LOC)) + return; + } if (symbolInstance.hasText()) { size_t textOpacityVerticesSize = 0u; const auto& opacityVertex = SymbolSDFTextProgram::opacityVertex(opacityState.text.placed, opacityState.text.opacity); - if (symbolInstance.placedRightTextIndex) { - textOpacityVerticesSize += symbolInstance.rightJustifiedGlyphQuadsSize * 4; - PlacedSymbol& placed = bucket.text.placedSymbols[*symbolInstance.placedRightTextIndex]; + if (symbolInstance.getPlacedRightTextIndex()) { + textOpacityVerticesSize += symbolInstance.getRightJustifiedGlyphQuadsSize() * 4; + PlacedSymbol& placed = bucket.text.placedSymbols[*symbolInstance.getPlacedRightTextIndex()]; placed.hidden = opacityState.isHidden(); } - if (symbolInstance.placedCenterTextIndex && !symbolInstance.singleLine) { - textOpacityVerticesSize += symbolInstance.centerJustifiedGlyphQuadsSize * 4; - PlacedSymbol& placed = bucket.text.placedSymbols[*symbolInstance.placedCenterTextIndex]; + if (symbolInstance.getPlacedCenterTextIndex() && !symbolInstance.getSingleLine()) { + textOpacityVerticesSize += symbolInstance.getCenterJustifiedGlyphQuadsSize() * 4; + PlacedSymbol& placed = bucket.text.placedSymbols[*symbolInstance.getPlacedCenterTextIndex()]; placed.hidden = opacityState.isHidden(); } - if (symbolInstance.placedLeftTextIndex && !symbolInstance.singleLine) { - textOpacityVerticesSize += symbolInstance.leftJustifiedGlyphQuadsSize * 4; - PlacedSymbol& placed = bucket.text.placedSymbols[*symbolInstance.placedLeftTextIndex]; + if (symbolInstance.getPlacedLeftTextIndex() && !symbolInstance.getSingleLine()) { + textOpacityVerticesSize += symbolInstance.getLeftJustifiedGlyphQuadsSize() * 4; + PlacedSymbol& placed = bucket.text.placedSymbols[*symbolInstance.getPlacedLeftTextIndex()]; placed.hidden = opacityState.isHidden(); } - if (symbolInstance.placedVerticalTextIndex) { - textOpacityVerticesSize += symbolInstance.verticalGlyphQuadsSize * 4; - bucket.text.placedSymbols[*symbolInstance.placedVerticalTextIndex].hidden = opacityState.isHidden(); + if (symbolInstance.getPlacedVerticalTextIndex()) { + textOpacityVerticesSize += symbolInstance.getVerticalGlyphQuadsSize() * 4; + bucket.text.placedSymbols[*symbolInstance.getPlacedVerticalTextIndex()].hidden = + opacityState.isHidden(); } bucket.text.opacityVertices().extend(textOpacityVerticesSize, opacityVertex); style::TextWritingModeType previousOrientation = style::TextWritingModeType::Horizontal; if (bucket.allowVerticalPlacement) { - auto prevOrientation = placedOrientations.find(symbolInstance.crossTileID); + auto prevOrientation = placedOrientations.find(symbolInstance.getCrossTileID()); if (prevOrientation != placedOrientations.end()) { previousOrientation = prevOrientation->second; markUsedOrientation(bucket, prevOrientation->second, symbolInstance); } } - auto prevOffset = variableOffsets.find(symbolInstance.crossTileID); + auto prevOffset = variableOffsets.find(symbolInstance.getCrossTileID()); if (prevOffset != variableOffsets.end()) { markUsedJustification(bucket, prevOffset->second.anchor, symbolInstance, previousOrientation); } @@ -1041,14 +1065,14 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, opacityState.icon.opacity); auto& iconBuffer = symbolInstance.hasSdfIcon() ? bucket.sdfIcon : bucket.icon; - if (symbolInstance.placedIconIndex) { - iconOpacityVerticesSize += symbolInstance.iconQuadsSize * 4; - iconBuffer.placedSymbols[*symbolInstance.placedIconIndex].hidden = opacityState.isHidden(); + if (symbolInstance.getPlacedIconIndex()) { + iconOpacityVerticesSize += symbolInstance.getIconQuadsSize() * 4; + iconBuffer.placedSymbols[*symbolInstance.getPlacedIconIndex()].hidden = opacityState.isHidden(); } - if (symbolInstance.placedVerticalIconIndex) { - iconOpacityVerticesSize += symbolInstance.iconQuadsSize * 4; - iconBuffer.placedSymbols[*symbolInstance.placedVerticalIconIndex].hidden = opacityState.isHidden(); + if (symbolInstance.getPlacedVerticalIconIndex()) { + iconOpacityVerticesSize += symbolInstance.getIconQuadsSize() * 4; + iconBuffer.placedSymbols[*symbolInstance.getPlacedVerticalIconIndex()].hidden = opacityState.isHidden(); } iconBuffer.opacityVertices().extend(iconOpacityVerticesSize, opacityVertex); @@ -1071,7 +1095,7 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, } bool used = true; if (variablePlacement) { - auto foundOffset = variableOffsets.find(symbolInstance.crossTileID); + auto foundOffset = variableOffsets.find(symbolInstance.getCrossTileID()); if (foundOffset != variableOffsets.end()) { const VariableOffset& variableOffset = foundOffset->second; // This will show either the currently placed position or @@ -1121,33 +1145,35 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, Point textShift{0.0f, 0.0f}; Point verticalTextShift{0.0f, 0.0f}; if (bucket.hasTextCollisionBoxData()) { - textShift = updateTextCollisionBox(symbolInstance.textCollisionFeature, opacityState.text.placed); - if (bucket.allowVerticalPlacement && symbolInstance.verticalTextCollisionFeature) { - verticalTextShift = updateTextCollisionBox(*symbolInstance.verticalTextCollisionFeature, + textShift = updateTextCollisionBox(symbolInstance.getTextCollisionFeature(), opacityState.text.placed); + if (bucket.allowVerticalPlacement && symbolInstance.getVerticalTextCollisionFeature()) { + verticalTextShift = updateTextCollisionBox(*symbolInstance.getVerticalTextCollisionFeature(), opacityState.text.placed); } } if (bucket.hasIconCollisionBoxData()) { - updateIconCollisionBox(symbolInstance.iconCollisionFeature, + updateIconCollisionBox(symbolInstance.getIconCollisionFeature(), opacityState.icon.placed, hasIconTextFit ? textShift : Point{0.0f, 0.0f}); - if (bucket.allowVerticalPlacement && symbolInstance.verticalIconCollisionFeature) { - updateIconCollisionBox(*symbolInstance.verticalIconCollisionFeature, + if (bucket.allowVerticalPlacement && symbolInstance.getVerticalIconCollisionFeature()) { + updateIconCollisionBox(*symbolInstance.getVerticalIconCollisionFeature(), opacityState.text.placed, hasIconTextFit ? verticalTextShift : Point{0.0f, 0.0f}); } } if (bucket.hasIconCollisionCircleData()) { - updateCollisionCircles(symbolInstance.iconCollisionFeature, opacityState.icon.placed, false); + updateCollisionCircles(symbolInstance.getIconCollisionFeature(), opacityState.icon.placed, false); } if (bucket.hasTextCollisionCircleData()) { - updateCollisionCircles(symbolInstance.textCollisionFeature, opacityState.text.placed, true); + updateCollisionCircles(symbolInstance.getTextCollisionFeature(), opacityState.text.placed, true); } } bucket.sortFeatures(static_cast(state.getBearing())); - auto retainedData = retainedQueryData.find(bucket.bucketInstanceId); + static_cast(bucket).check(SYM_GUARD_LOC); + + const auto retainedData = retainedQueryData.find(bucket.bucketInstanceId); if (retainedData != retainedQueryData.end()) { retainedData->second.featureSortOrder = bucket.featureSortOrder; } @@ -1159,16 +1185,16 @@ std::optional justificationToIndex(style::TextJustifyType justify, style::TextWritingModeType orientation) { // Vertical symbol has just one justification, style::TextJustifyType::Left. if (orientation == style::TextWritingModeType::Vertical) { - return symbolInstance.placedVerticalTextIndex; + return symbolInstance.getPlacedVerticalTextIndex(); } switch (justify) { case style::TextJustifyType::Right: - return symbolInstance.placedRightTextIndex; + return symbolInstance.getPlacedRightTextIndex(); case style::TextJustifyType::Center: - return symbolInstance.placedCenterTextIndex; + return symbolInstance.getPlacedCenterTextIndex(); case style::TextJustifyType::Left: - return symbolInstance.placedLeftTextIndex; + return symbolInstance.getPlacedLeftTextIndex(); case style::TextJustifyType::Auto: break; } @@ -1193,12 +1219,13 @@ void Placement::markUsedJustification(SymbolBucket& bucket, const std::optional index = justificationToIndex(justify, symbolInstance, orientation); if (index) { assert(bucket.text.placedSymbols.size() > *index); + if (!symbolInstance.checkIndex(index, bucket.text.placedSymbols.size(), SYM_GUARD_LOC)) continue; if (autoIndex && *index != *autoIndex) { // There are multiple justifications and this one isn't it: shift offscreen bucket.text.placedSymbols.at(*index).crossTileID = 0u; } else { // Either this is the chosen justification or the justification is hardwired: use this one - bucket.text.placedSymbols.at(*index).crossTileID = symbolInstance.crossTileID; + bucket.text.placedSymbols.at(*index).crossTileID = symbolInstance.getCrossTileID(); } } } @@ -1207,36 +1234,43 @@ void Placement::markUsedJustification(SymbolBucket& bucket, void Placement::markUsedOrientation(SymbolBucket& bucket, style::TextWritingModeType orientation, const SymbolInstance& symbolInstance) const { - auto horizontal = orientation == style::TextWritingModeType::Horizontal - ? std::optional(orientation) - : std::nullopt; - auto vertical = orientation == style::TextWritingModeType::Vertical - ? std::optional(orientation) - : std::nullopt; + const auto horizontal = orientation == style::TextWritingModeType::Horizontal + ? std::optional(orientation) + : std::nullopt; + const auto vertical = orientation == style::TextWritingModeType::Vertical + ? std::optional(orientation) + : std::nullopt; - if (symbolInstance.placedRightTextIndex) { - bucket.text.placedSymbols.at(*symbolInstance.placedRightTextIndex).placedOrientation = horizontal; + if (!symbolInstance.checkIndexes(bucket.text.placedSymbols.size(), + bucket.icon.placedSymbols.size(), + bucket.sdfIcon.placedSymbols.size(), + SYM_GUARD_LOC)) { + return; } - if (symbolInstance.placedCenterTextIndex && !symbolInstance.singleLine) { - bucket.text.placedSymbols.at(*symbolInstance.placedCenterTextIndex).placedOrientation = horizontal; + if (symbolInstance.getPlacedRightTextIndex()) { + bucket.text.placedSymbols.at(*symbolInstance.getPlacedRightTextIndex()).placedOrientation = horizontal; } - if (symbolInstance.placedLeftTextIndex && !symbolInstance.singleLine) { - bucket.text.placedSymbols.at(*symbolInstance.placedLeftTextIndex).placedOrientation = horizontal; + if (symbolInstance.getPlacedCenterTextIndex() && !symbolInstance.getSingleLine()) { + bucket.text.placedSymbols.at(*symbolInstance.getPlacedCenterTextIndex()).placedOrientation = horizontal; } - if (symbolInstance.placedVerticalTextIndex) { - bucket.text.placedSymbols.at(*symbolInstance.placedVerticalTextIndex).placedOrientation = vertical; + if (symbolInstance.getPlacedLeftTextIndex() && !symbolInstance.getSingleLine()) { + bucket.text.placedSymbols.at(*symbolInstance.getPlacedLeftTextIndex()).placedOrientation = horizontal; + } + + if (symbolInstance.getPlacedVerticalTextIndex()) { + bucket.text.placedSymbols.at(*symbolInstance.getPlacedVerticalTextIndex()).placedOrientation = vertical; } auto& iconBuffer = symbolInstance.hasSdfIcon() ? bucket.sdfIcon : bucket.icon; - if (symbolInstance.placedIconIndex) { - iconBuffer.placedSymbols.at(*symbolInstance.placedIconIndex).placedOrientation = horizontal; + if (symbolInstance.getPlacedIconIndex()) { + iconBuffer.placedSymbols.at(*symbolInstance.getPlacedIconIndex()).placedOrientation = horizontal; } - if (symbolInstance.placedVerticalIconIndex) { - iconBuffer.placedSymbols.at(*symbolInstance.placedVerticalIconIndex).placedOrientation = vertical; + if (symbolInstance.getPlacedVerticalIconIndex()) { + iconBuffer.placedSymbols.at(*symbolInstance.getPlacedVerticalIconIndex()).placedOrientation = vertical; } } @@ -1264,8 +1298,8 @@ float Placement::zoomAdjustment(const float zoom) const { } const JointPlacement* Placement::getSymbolPlacement(const SymbolInstance& symbol) const { - assert(symbol.crossTileID != 0); - auto found = placements.find(symbol.crossTileID); + assert(symbol.getCrossTileID() != 0); + const auto found = placements.find(symbol.getCrossTileID()); return (found != placements.end()) ? &found->second : nullptr; } @@ -1382,7 +1416,7 @@ void TilePlacement::placeLayers(const RenderLayerReferences& layers) { seenCrossTileIDs.clear(); intersections.clear(); currentIntersectionPriority = 0u; - // Populale intersections. + // Populate intersections. populateIntersections = true; for (auto it = layers.crbegin(); it != layers.crend(); ++it) { placeLayer(*it, seenCrossTileIDs); @@ -1395,29 +1429,30 @@ void TilePlacement::placeLayers(const RenderLayerReferences& layers) { // Items arranged as: VerticalBorders & HorizontalBorders (3) -> // VerticalBorders (2) -> HorizontalBorders (1) if (flagsA != flagsB) return flagsA > flagsB; - // If both intersects the same border(s), look for a more noticable cut-off. + // If both intersects the same border(s), look for a more noticeable cut-off. if (a.status.minSectionLength != b.status.minSectionLength) { return a.status.minSectionLength > b.status.minSectionLength; } // Look at the anchor coordinates - if (a.symbol.get().anchor.point.y != b.symbol.get().anchor.point.y) { - return a.symbol.get().anchor.point.y < b.symbol.get().anchor.point.y; + if (a.symbol.get().getAnchor().point.y != b.symbol.get().getAnchor().point.y) { + return a.symbol.get().getAnchor().point.y < b.symbol.get().getAnchor().point.y; } - if (a.symbol.get().anchor.point.x != b.symbol.get().anchor.point.x) { - return a.symbol.get().anchor.point.x < b.symbol.get().anchor.point.x; + if (a.symbol.get().getAnchor().point.x != b.symbol.get().getAnchor().point.x) { + return a.symbol.get().getAnchor().point.x < b.symbol.get().getAnchor().point.x; } // Finally, looking at the key hashes. - return std::hash()(a.symbol.get().key) < std::hash()(b.symbol.get().key); + return std::hash()(a.symbol.get().getKey()) < + std::hash()(b.symbol.get().getKey()); }); // Place intersections. for (const auto& intersection : intersections) { const SymbolInstance& symbol = intersection.symbol; const PlacementContext& ctx = intersection.ctx; currentIntersectionPriority = intersection.priority; - if (seenCrossTileIDs.contains(symbol.crossTileID)) continue; + if (seenCrossTileIDs.contains(symbol.getCrossTileID())) continue; JointPlacement placement = placeSymbol(symbol, ctx); if (shouldRetryPlacement(placement, ctx)) continue; - seenCrossTileIDs.insert(symbol.crossTileID); + seenCrossTileIDs.insert(symbol.getCrossTileID()); } // Place the rest labels. populateIntersections = false; @@ -1515,8 +1550,8 @@ void TilePlacement::placeSymbolBucket(const BucketPlacementData& params, std::se bearing = static_cast(ctx.getTransformState().getBearing())]( const SymbolInstance& symbol) noexcept -> IntersectStatus { IntersectStatus result; - if (!symbol.textCollisionFeature.boxes.empty()) { - const auto& textCollisionBox = symbol.textCollisionFeature.boxes.front(); + if (!symbol.getTextCollisionFeature().boxes.empty()) { + const auto& textCollisionBox = symbol.getTextCollisionFeature().boxes.front(); Point offset{}; if (variableAnchor) { @@ -1525,8 +1560,8 @@ void TilePlacement::placeSymbolBucket(const BucketPlacementData& params, std::se offset = calculateVariableLayoutOffset(*variableAnchor, width, height, - symbol.variableTextOffset, - symbol.textBoxScale, + symbol.getVariableTextOffset(), + symbol.getTextBoxScale(), rotateTextWithMap, pitchTextWithMap, bearing); @@ -1534,8 +1569,8 @@ void TilePlacement::placeSymbolBucket(const BucketPlacementData& params, std::se result = collisionBoxIntersectsTileEdges(textCollisionBox, offset); } - if (!symbol.iconCollisionFeature.boxes.empty()) { - const auto& iconCollisionBox = symbol.iconCollisionFeature.boxes.front(); + if (!symbol.getIconCollisionFeature().boxes.empty()) { + const auto& iconCollisionBox = symbol.getIconCollisionFeature().boxes.front(); Point offset{}; if (variableAnchor && variableIconPlacement) { float width = iconCollisionBox.x2 - iconCollisionBox.x1; @@ -1543,8 +1578,8 @@ void TilePlacement::placeSymbolBucket(const BucketPlacementData& params, std::se offset = calculateVariableLayoutOffset(*variableAnchor, width, height, - symbol.variableTextOffset, - symbol.textBoxScale, + symbol.getVariableTextOffset(), + symbol.getTextBoxScale(), rotateTextWithMap, pitchTextWithMap, bearing); @@ -1558,8 +1593,13 @@ void TilePlacement::placeSymbolBucket(const BucketPlacementData& params, std::se }; for (const SymbolInstance& symbol : symbolInstances) { - auto intersectStatus = symbolIntersectsTileEdges(symbol); - if (intersectStatus.flags == IntersectStatus::None) continue; + if (!symbol.check(SYM_GUARD_LOC)) { + continue; + } + const auto intersectStatus = symbolIntersectsTileEdges(symbol); + if (intersectStatus.flags == IntersectStatus::None) { + continue; + } intersections.emplace_back(symbol, ctx, intersectStatus, currentIntersectionPriority); } @@ -1615,7 +1655,7 @@ void TilePlacement::newSymbolPlaced(const SymbolInstance& symbol, assert(box.isBox()); iconCollisionBox = box.box(); } - PlacedSymbolData symbolData{symbol.key, + PlacedSymbolData symbolData{symbol.getKey(), textCollisionBox, iconCollisionBox, placement.text, diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index b3205f629..147ec9aae 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -302,7 +302,23 @@ void GeometryTile::onLayout(std::shared_ptr result, const uint64_t atlasTextures = std::make_shared(); } + if (layoutResult) { + for (const auto& data : layoutResult->layerRenderData) { + if (data.second.bucket) { + data.second.bucket->check(SYM_GUARD_LOC); + } + } + } + observer->onTileChanged(*this); + + if (layoutResult) { + for (const auto& data : layoutResult->layerRenderData) { + if (data.second.bucket) { + data.second.bucket->check(SYM_GUARD_LOC); + } + } + } } void GeometryTile::onError(std::exception_ptr err, const uint64_t resultCorrelationID) { diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index aa9aa8319..fdbf4e970 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -28,6 +28,8 @@ class TileAtlasTextures; class GeometryTile : public Tile, public GlyphRequestor, public ImageRequestor { public: + const std::thread::id renderThreadID = std::this_thread::get_id(); + GeometryTile(const OverscaledTileID&, std::string sourceID, const TileParameters&); ~GeometryTile() override; diff --git a/src/mbgl/util/logging.cpp b/src/mbgl/util/logging.cpp index d1f0d191d..c86b881f8 100644 --- a/src/mbgl/util/logging.cpp +++ b/src/mbgl/util/logging.cpp @@ -56,17 +56,14 @@ Log* Log::get() noexcept { return &instance; } -void Log::useLogThread(bool enable) noexcept { - useLogThread(enable, EventSeverity::Debug); - useLogThread(enable, EventSeverity::Info); - useLogThread(enable, EventSeverity::Warning); - useLogThread(enable, EventSeverity::Error); -} - -void Log::useLogThread(bool enable, EventSeverity severity) noexcept { - const auto index = underlying_type(severity); - if (index < SeverityCount) { - useThread[index] = enable; +void Log::useLogThread(bool enable, std::optional severity) { + if (severity) { + useThread[underlying_type(*severity)] = enable; + } else { + useLogThread(enable, EventSeverity::Debug); + useLogThread(enable, EventSeverity::Info); + useLogThread(enable, EventSeverity::Warning); + useLogThread(enable, EventSeverity::Error); } } diff --git a/src/mbgl/util/string.cpp b/src/mbgl/util/string.cpp index 718da5f1d..5969826e3 100644 --- a/src/mbgl/util/string.cpp +++ b/src/mbgl/util/string.cpp @@ -4,6 +4,7 @@ #include #include +#include namespace mbgl { namespace util { @@ -36,6 +37,10 @@ std::string toString(uint64_t t) { return s.GetString(); } +std::string toString(const std::thread::id& t) { + return ((std::stringstream{}) << t).str(); +} + std::string toString(double t, bool decimal) { rapidjson::StringBuffer s; rapidjson::Writer writer(s); diff --git a/test/text/cross_tile_symbol_index.test.cpp b/test/text/cross_tile_symbol_index.test.cpp index 3f3ccf3d7..de41ae179 100644 --- a/test/text/cross_tile_symbol_index.test.cpp +++ b/test/text/cross_tile_symbol_index.test.cpp @@ -86,8 +86,8 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { index.addBucket(mainID, mat4{}, mainBucket); // Assigned new IDs - ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); - ASSERT_EQ(mainBucket.symbolInstances.at(1).crossTileID, 2u); + ASSERT_EQ(mainBucket.symbolInstances.at(0).getCrossTileID(), 1u); + ASSERT_EQ(mainBucket.symbolInstances.at(1).getCrossTileID(), 2u); OverscaledTileID childID(7, 0, 7, 16, 16); std::vector childInstances; @@ -114,13 +114,13 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { index.addBucket(childID, mat4{}, childBucket); // matched parent tile - ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 1u); + ASSERT_EQ(childBucket.symbolInstances.at(0).getCrossTileID(), 1u); // does not match because of different key - ASSERT_EQ(childBucket.symbolInstances.at(1).crossTileID, 3u); + ASSERT_EQ(childBucket.symbolInstances.at(1).getCrossTileID(), 3u); // does not match because of different location - ASSERT_EQ(childBucket.symbolInstances.at(2).crossTileID, 4u); + ASSERT_EQ(childBucket.symbolInstances.at(2).getCrossTileID(), 4u); // matches with a slightly different location - ASSERT_EQ(childBucket.symbolInstances.at(3).crossTileID, 2u); + ASSERT_EQ(childBucket.symbolInstances.at(3).getCrossTileID(), 2u); OverscaledTileID parentID(5, 0, 5, 4, 4); std::vector parentInstances; @@ -144,7 +144,7 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { index.addBucket(parentID, mat4{}, parentBucket); // matched child tile - ASSERT_EQ(parentBucket.symbolInstances.at(0).crossTileID, 1u); + ASSERT_EQ(parentBucket.symbolInstances.at(0).getCrossTileID(), 1u); std::unordered_set currentIDs; currentIDs.insert(mainBucket.bucketInstanceId); @@ -174,9 +174,9 @@ TEST(CrossTileSymbolLayerIndex, addBucket) { index.addBucket(grandchildID, mat4{}, grandchildBucket); // Matches the symbol in `mainBucket` - ASSERT_EQ(grandchildBucket.symbolInstances.at(0).crossTileID, 1u); + ASSERT_EQ(grandchildBucket.symbolInstances.at(0).getCrossTileID(), 1u); // Does not match the previous value for Windsor because that tile was removed - ASSERT_EQ(grandchildBucket.symbolInstances.at(1).crossTileID, 5u); + ASSERT_EQ(grandchildBucket.symbolInstances.at(1).getCrossTileID(), 5u); } TEST(CrossTileSymbolLayerIndex, resetIDs) { @@ -232,7 +232,7 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { // assigns a new id index.addBucket(mainID, mat4{}, mainBucket); - ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); + ASSERT_EQ(mainBucket.symbolInstances.at(0).getCrossTileID(), 1u); // removes the tile std::unordered_set currentIDs; @@ -240,11 +240,11 @@ TEST(CrossTileSymbolLayerIndex, resetIDs) { // assigns a new id index.addBucket(childID, mat4{}, childBucket); - ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, 2u); + ASSERT_EQ(childBucket.symbolInstances.at(0).getCrossTileID(), 2u); // overwrites the old id to match the already-added tile index.addBucket(mainID, mat4{}, mainBucket); - ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 2u); + ASSERT_EQ(mainBucket.symbolInstances.at(0).getCrossTileID(), 2u); } TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) { @@ -303,16 +303,16 @@ TEST(CrossTileSymbolLayerIndex, noDuplicatesWithinZoomLevel) { // assigns new ids index.addBucket(mainID, mat4{}, mainBucket); - ASSERT_EQ(mainBucket.symbolInstances.at(0).crossTileID, 1u); - ASSERT_EQ(mainBucket.symbolInstances.at(1).crossTileID, 2u); + ASSERT_EQ(mainBucket.symbolInstances.at(0).getCrossTileID(), 1u); + ASSERT_EQ(mainBucket.symbolInstances.at(1).getCrossTileID(), 2u); // copies parent ids without duplicate ids in this tile index.addBucket(childID, mat4{}, childBucket); - ASSERT_EQ(childBucket.symbolInstances.at(0).crossTileID, + ASSERT_EQ(childBucket.symbolInstances.at(0).getCrossTileID(), 1u); // A' copies from A - ASSERT_EQ(childBucket.symbolInstances.at(1).crossTileID, + ASSERT_EQ(childBucket.symbolInstances.at(1).getCrossTileID(), 2u); // B' copies from B - ASSERT_EQ(childBucket.symbolInstances.at(2).crossTileID, + ASSERT_EQ(childBucket.symbolInstances.at(2).getCrossTileID(), 3u); // C' gets new ID } @@ -371,16 +371,16 @@ TEST(CrossTileSymbolLayerIndex, bucketReplacement) { // assigns new ids index.addBucket(tileID, mat4{}, firstBucket); - ASSERT_EQ(firstBucket.symbolInstances.at(0).crossTileID, 1u); - ASSERT_EQ(firstBucket.symbolInstances.at(1).crossTileID, 2u); + ASSERT_EQ(firstBucket.symbolInstances.at(0).getCrossTileID(), 1u); + ASSERT_EQ(firstBucket.symbolInstances.at(1).getCrossTileID(), 2u); // copies parent ids without duplicate ids in this tile index.addBucket(tileID, mat4{}, secondBucket); - ASSERT_EQ(secondBucket.symbolInstances.at(0).crossTileID, + ASSERT_EQ(secondBucket.symbolInstances.at(0).getCrossTileID(), 1u); // A' copies from A - ASSERT_EQ(secondBucket.symbolInstances.at(1).crossTileID, + ASSERT_EQ(secondBucket.symbolInstances.at(1).getCrossTileID(), 2u); // B' copies from B - ASSERT_EQ(secondBucket.symbolInstances.at(2).crossTileID, + ASSERT_EQ(secondBucket.symbolInstances.at(2).getCrossTileID(), 3u); // C' gets new ID } @@ -429,12 +429,12 @@ TEST(CrossTileSymbolLayerIndex, offscreenSymbols) { populatePosMatrix(posMatrix, tileId, 60.0, 25.0, 7.0); index.addBucket(tileId, posMatrix, symbolBucket); - EXPECT_EQ(symbolBucket.symbolInstances.at(0).crossTileID, SymbolInstance::invalidCrossTileID()); - EXPECT_EQ(symbolBucket.symbolInstances.at(1).crossTileID, SymbolInstance::invalidCrossTileID()); + EXPECT_EQ(symbolBucket.symbolInstances.at(0).getCrossTileID(), SymbolInstance::invalidCrossTileID); + EXPECT_EQ(symbolBucket.symbolInstances.at(1).getCrossTileID(), SymbolInstance::invalidCrossTileID); populatePosMatrix(posMatrix, tileId, 39.0, -76.0, 7.0); index.addBucket(tileId, posMatrix, symbolBucket); - EXPECT_EQ(symbolBucket.symbolInstances.at(0).crossTileID, 1u); - EXPECT_EQ(symbolBucket.symbolInstances.at(1).crossTileID, 2u); + EXPECT_EQ(symbolBucket.symbolInstances.at(0).getCrossTileID(), 1u); + EXPECT_EQ(symbolBucket.symbolInstances.at(1).getCrossTileID(), 2u); }