From 642baf2d9c8e7e51abf876898df7b37dab19f4c3 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Mon, 14 Oct 2024 07:16:51 -0700 Subject: [PATCH 1/3] Cherry-pick dd104ded Add guard blocks and checks to SymbolInstance (#2744) --- .gitignore | 1 + include/mbgl/util/logging.hpp | 6 +- include/mbgl/util/string.hpp | 1 + src/mbgl/layout/symbol_instance.cpp | 78 ++++ src/mbgl/layout/symbol_instance.hpp | 165 +++++++- src/mbgl/layout/symbol_layout.cpp | 130 ++++--- 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 | 6 +- src/mbgl/text/cross_tile_symbol_index.cpp | 43 ++- src/mbgl/text/cross_tile_symbol_index.hpp | 3 + src/mbgl/text/placement.cpp | 361 ++++++++++-------- src/mbgl/tile/geometry_tile.cpp | 16 + src/mbgl/tile/geometry_tile.hpp | 6 +- src/mbgl/util/logging.cpp | 11 +- src/mbgl/util/string.cpp | 5 + test/text/cross_tile_symbol_index.test.cpp | 58 +-- 18 files changed, 692 insertions(+), 291 deletions(-) diff --git a/.gitignore b/.gitignore index a752ed2606f..05f30b56dd0 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ *.code-workspace .DS_Store +/.cache /build **/.idea /platform/android/MapboxGLAndroidSDKTestApp/local.properties diff --git a/include/mbgl/util/logging.hpp b/include/mbgl/util/logging.hpp index f393c7e9313..f1b512a10a7 100644 --- a/include/mbgl/util/logging.hpp +++ b/include/mbgl/util/logging.hpp @@ -40,7 +40,11 @@ class Log { Log(); ~Log(); - static void useLogThread(bool enable); + /// @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) { diff --git a/include/mbgl/util/string.hpp b/include/mbgl/util/string.hpp index df205b9072d..f00b70ff42f 100644 --- a/include/mbgl/util/string.hpp +++ b/include/mbgl/util/string.hpp @@ -76,6 +76,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) { std::string toString(const std::exception_ptr &); template diff --git a/src/mbgl/layout/symbol_instance.cpp b/src/mbgl/layout/symbol_instance.cpp index 9eda80d4eaa..19dadc2bb20 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 { @@ -199,4 +201,80 @@ optional SymbolInstance::getDefaultHorizontalPlacedTextIndex() const { if (placedLeftTextIndex) return placedLeftTextIndex; return 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 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 b10a3acaee8..014279943ed 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; @@ -47,6 +90,9 @@ struct SymbolInstanceSharedData { optional verticalIconQuads; }; +// With guards, clang complains about excessive padding here +// NOLINTBEGIN(clang-analyzer-optin.performance.Padding) + class SymbolInstance { public: SymbolInstance(Anchor& anchor_, @@ -85,44 +131,149 @@ class SymbolInstance { const 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 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 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 optional& getVerticalTextCollisionFeature() const { + return verticalTextCollisionFeature; + } + const 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; } + optional getPlacedRightTextIndex() const { return placedRightTextIndex; } + optional getPlacedCenterTextIndex() const { return placedCenterTextIndex; } + optional getPlacedLeftTextIndex() const { return placedLeftTextIndex; } + optional getPlacedVerticalTextIndex() const { return placedVerticalTextIndex; } + optional getPlacedIconIndex() const { return placedIconIndex; } + 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; } + + optional& refPlacedRightTextIndex() { return placedRightTextIndex; } + optional& refPlacedCenterTextIndex() { return placedCenterTextIndex; } + optional& refPlacedLeftTextIndex() { return placedLeftTextIndex; } + optional& refPlacedVerticalTextIndex() { return placedVerticalTextIndex; } + optional& refPlacedIconIndex() { return placedIconIndex; } + optional& refPlacedVerticalIconIndex() { return placedVerticalIconIndex; } + + void setPlacedRightTextIndex(optional x) { placedRightTextIndex = x; } + void setPlacedCenterTextIndex(optional x) { placedCenterTextIndex = x; } + void setPlacedLeftTextIndex(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; - optional verticalTextCollisionFeature = nullopt; - optional verticalIconCollisionFeature = nullopt; + SYM_GUARD_VALUE(10) + optional verticalTextCollisionFeature = std::nullopt; + SYM_GUARD_VALUE(11) + 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; - bool isDuplicate; + SYM_GUARD_VALUE(18) optional placedRightTextIndex; + SYM_GUARD_VALUE(19) optional placedCenterTextIndex; + SYM_GUARD_VALUE(20) optional placedLeftTextIndex; + SYM_GUARD_VALUE(21) optional placedVerticalTextIndex; + SYM_GUARD_VALUE(22) optional placedIconIndex; + SYM_GUARD_VALUE(23) 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 366ad82954e..31dc6f90c19 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -248,10 +249,9 @@ 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 sqrt2 = 1.41421356237f; - const float hypotenuse = radialOffset / sqrt2; + const float hypotenuse = radialOffset / std::numbers::sqrt2_v; switch (anchor) { case SymbolAnchorType::TopRight: @@ -494,7 +494,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, @@ -543,7 +543,7 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex, const float textMaxAngle = layout->get() * util::DEG2RAD_F; 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}}; @@ -775,11 +775,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 @@ -789,31 +793,35 @@ 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()); index = iconBuffer.placedSymbols.size() - 1; PlacedSymbol& iconSymbol = iconBuffer.placedSymbols.back(); - iconSymbol.angle = (allowVerticalPlacement && writingMode == WritingModeType::Vertical) ? static_cast(M_PI_2) : 0.0f; - iconSymbol.vertexStartIndex = - addSymbols(iconBuffer, sizeData, iconQuads, symbolInstance.anchor, iconSymbol, feature.sortKey); + iconSymbol.angle = (allowVerticalPlacement && writingMode == WritingModeType::Vertical) + ? pi_v / 2 + : 0.0f; + iconSymbol.vertexStartIndex = addSymbols( + 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); } } @@ -824,56 +832,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); } @@ -915,15 +925,16 @@ std::size_t SymbolLayout::addSymbolGlyphQuads(SymbolBucket& bucket, 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(); @@ -937,7 +948,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; @@ -1017,16 +1029,16 @@ size_t SymbolLayout::addSymbol(SymbolBucket::Buffer& buffer, // Dynamic/Opacity vertices are initialized so that the vertex count always agrees with // the layout vertex buffer, but they will always be updated before rendering happens auto dynamicVertex = SymbolSDFIconProgram::dynamicLayoutVertex(labelAnchor.point, 0); - buffer.dynamicVertices.emplace_back(dynamicVertex); - buffer.dynamicVertices.emplace_back(dynamicVertex); - buffer.dynamicVertices.emplace_back(dynamicVertex); - buffer.dynamicVertices.emplace_back(dynamicVertex); + buffer.dynamicVertices().emplace_back(dynamicVertex); + buffer.dynamicVertices().emplace_back(dynamicVertex); + buffer.dynamicVertices().emplace_back(dynamicVertex); + buffer.dynamicVertices().emplace_back(dynamicVertex); - auto opacityVertex = SymbolSDFIconProgram::opacityVertex(1.0, 1.0); - buffer.opacityVertices.emplace_back(opacityVertex); - buffer.opacityVertices.emplace_back(opacityVertex); - buffer.opacityVertices.emplace_back(opacityVertex); - buffer.opacityVertices.emplace_back(opacityVertex); + auto opacityVertex = SymbolSDFIconProgram::opacityVertex(true, 1.0); + buffer.opacityVertices().emplace_back(opacityVertex); + buffer.opacityVertices().emplace_back(opacityVertex); + buffer.opacityVertices().emplace_back(opacityVertex); + buffer.opacityVertices().emplace_back(opacityVertex); // add the two triangles, referencing the four coordinates we just inserted. buffer.triangles.emplace_back(index + 0, index + 1, index + 2); @@ -1094,10 +1106,14 @@ void SymbolLayout::addToDebugBuffers(SymbolBucket& bucket) { auto& segment = collisionBuffer.segments.back(); auto index = static_cast(segment.vertexLength); - collisionBuffer.vertices.emplace_back(CollisionBoxProgram::layoutVertex(anchor, symbolInstance.anchor.point, tl)); - collisionBuffer.vertices.emplace_back(CollisionBoxProgram::layoutVertex(anchor, symbolInstance.anchor.point, tr)); - collisionBuffer.vertices.emplace_back(CollisionBoxProgram::layoutVertex(anchor, symbolInstance.anchor.point, br)); - collisionBuffer.vertices.emplace_back(CollisionBoxProgram::layoutVertex(anchor, symbolInstance.anchor.point, bl)); + collisionBuffer.vertices().emplace_back( + CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, tl)); + collisionBuffer.vertices().emplace_back( + CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, tr)); + collisionBuffer.vertices().emplace_back( + CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, br)); + collisionBuffer.vertices().emplace_back( + 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 always be updated before rendering happens @@ -1123,14 +1139,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 68d6c46b7f8..1c3dabf4dd0 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 #include namespace mbgl { @@ -68,9 +69,20 @@ class Bucket { virtual void updateVertices( const Placement&, bool /*updateOpacities*/, const TransformState&, const RenderTile&, std::set&) {} +#if MLN_SYMBOL_GUARDS + virtual bool check(std::source_location) { return true; } +#else + bool check(std::string_view = {}) { return true; } +#endif + + const optional& getRenderThreadID() const { return renderThreadID; } + void setRenderThreadID(optional id) { renderThreadID = id; } + protected: Bucket() = default; std::atomic uploaded { false }; + + optional renderThreadID; }; } // namespace mbgl diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp index b8a135dc240..856209d1547 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.cpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp @@ -246,31 +246,37 @@ void SymbolBucket::sortFeatures(const float angle) { // 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()]); } } @@ -283,25 +289,48 @@ 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 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_) { diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp index c6a07dde1c9..5b55dbec040 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.hpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp @@ -103,6 +103,10 @@ class SymbolBucket final : public Bucket { // returns references to all the symbols if |sortKeyRange| is `nullopt`. SymbolInstanceReferences getSymbols(const optional& sortKeyRange = 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 2080b04c8ed..2e66502e1d6 100644 --- a/src/mbgl/renderer/layers/render_symbol_layer.cpp +++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp @@ -424,6 +424,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; @@ -579,7 +583,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); diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp index f2785dd1203..905c0b6e5d9 100644 --- a/src/mbgl/text/cross_tile_symbol_index.cpp +++ b/src/mbgl/text/cross_tile_symbol_index.cpp @@ -10,11 +10,16 @@ TileLayerIndex::TileLayerIndex(OverscaledTileID coord_, std::vector& symbolInstances, uint32_t bucketInstanceId_, std::string bucketLeaderId_) - : coord(coord_), bucketInstanceId(bucketInstanceId_), bucketLeaderId(std::move(bucketLeaderId_)) { - for (SymbolInstance& symbolInstance : symbolInstances) { - if (symbolInstance.crossTileID == SymbolInstance::invalidCrossTileID()) continue; - indexedSymbolInstances[symbolInstance.key].emplace_back(symbolInstance.crossTileID, - getScaledCoordinates(symbolInstance, coord)); + : coord(coord_), + bucketInstanceId(bucketInstanceId_), + bucketLeaderId(std::move(bucketLeaderId_)) { + for (const SymbolInstance& symbolInstance : symbolInstances) { + if (!symbolInstance.check(SYM_GUARD_LOC) || + symbolInstance.getCrossTileID() == SymbolInstance::invalidCrossTileID) { + continue; + } + indexedSymbolInstances[symbolInstance.getKey()].emplace_back(symbolInstance.getCrossTileID(), + getScaledCoordinates(symbolInstance, coord)); } } @@ -23,10 +28,10 @@ Point TileLayerIndex::getScaledCoordinates(SymbolInstance& symbolInstan // Round anchor positions to roughly 4 pixel grid 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)), - static_cast(std::floor((childTileCoord.canonical.y * util::EXTENT + symbolInstance.anchor.point.y) * scale)) - }; + return {static_cast( + std::floor((childTileCoord.canonical.x * util::EXTENT + symbolInstance.getAnchor().point.x) * scale)), + static_cast( + std::floor((childTileCoord.canonical.y * util::EXTENT + symbolInstance.getAnchor().point.y) * scale))}; } void TileLayerIndex::findMatches(SymbolBucket& bucket, @@ -38,12 +43,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; @@ -61,7 +66,7 @@ void TileLayerIndex::findMatches(SymbolBucket& bucket, // 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; } } @@ -134,16 +139,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); } } @@ -168,10 +173,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 8dfdb8466f9..542f297fd83 100644 --- a/src/mbgl/text/cross_tile_symbol_index.hpp +++ b/src/mbgl/text/cross_tile_symbol_index.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include namespace mbgl { @@ -75,6 +76,8 @@ class CrossTileSymbolIndex { void pruneUnusedLayers(const std::set&); 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 0439babcb47..f0a56f9f192 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -61,7 +61,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; @@ -239,12 +239,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) { @@ -335,18 +337,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}; @@ -357,13 +359,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); @@ -381,15 +383,15 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons } } - const bool doVariableIconPlacement = - ctx.hasIconTextFit && !ctx.iconAllowOverlap && symbolInstance.placedIconIndex; + const bool doVariableIconPlacement = ctx.hasIconTextFit && !ctx.iconAllowOverlap && + 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; @@ -399,7 +401,7 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons shift = calculateVariableLayoutOffset(anchor, width, height, - symbolInstance.variableTextOffset, + symbolInstance.getVariableTextOffset(), textBoxScale, ctx.rotateTextWithMap, ctx.pitchTextWithMap, @@ -446,14 +448,14 @@ JointPlacement Placement::placeSymbol(const SymbolInstance& symbolInstance, cons } if (placedFeature.first) { - assert(symbolInstance.crossTileID != 0u); + assert(symbolInstance.getCrossTileID() != 0u); 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. @@ -461,13 +463,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; } @@ -477,18 +482,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}; }; @@ -503,21 +509,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, @@ -536,11 +542,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; @@ -559,14 +565,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, @@ -575,14 +581,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, @@ -593,24 +599,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 + 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; } @@ -649,7 +665,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(); @@ -720,13 +736,18 @@ void Placement::updateLayerBuckets(const RenderLayer& layer, const TransformStat } namespace { -Point calculateVariableRenderShift(style::SymbolAnchorType anchor, float width, float height, std::array textOffset, float textBoxScale, float renderTextSize) { - AnchorAlignment alignment = AnchorAlignment::getAnchorAlignment(anchor); - float shiftX = -(alignment.horizontalAlign - 0.5f) * width; - float shiftY = -(alignment.verticalAlign - 0.5f) * height; - auto variablOffset = SymbolLayout::evaluateVariableOffset(anchor, textOffset); - return { (shiftX / textBoxScale + variablOffset[0]) * renderTextSize, - (shiftY / textBoxScale + variablOffset[1]) * renderTextSize }; +Point calculateVariableRenderShift(style::SymbolAnchorType anchor, + float width, + float height, + std::array textOffset, + float textBoxScale, + float renderTextSize) { + const AnchorAlignment alignment = AnchorAlignment::getAnchorAlignment(anchor); + const float shiftX = -(alignment.horizontalAlign - 0.5f) * width; + const float shiftY = -(alignment.verticalAlign - 0.5f) * height; + auto variableOffset = SymbolLayout::evaluateVariableOffset(anchor, textOffset); + return {(shiftX / textBoxScale + variableOffset[0]) * renderTextSize, + (shiftY / textBoxScale + variableOffset[1]) * renderTextSize}; } } // namespace @@ -919,9 +940,10 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, true); for (SymbolInstance& symbolInstance : bucket.symbolInstances) { - bool isDuplicate = seenCrossTileIDs.count(symbolInstance.crossTileID) > 0; + 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; @@ -929,43 +951,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]; + const auto& opacityVertex = SymbolSDFTextProgram::opacityVertex(opacityState.text.placed, + opacityState.text.opacity); + 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); } @@ -976,14 +1007,14 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, SymbolIconProgram::opacityVertex(opacityState.icon.placed, 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); @@ -1006,7 +1037,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 the last @@ -1054,28 +1085,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, opacityState.text.placed); + 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, opacityState.icon.placed, hasIconTextFit ? textShift : Point{0.0f, 0.0f}); - if (bucket.allowVerticalPlacement && symbolInstance.verticalIconCollisionFeature) { - updateIconCollisionBox(*symbolInstance.verticalIconCollisionFeature, opacityState.text.placed, hasIconTextFit ? verticalTextShift : Point{0.0f, 0.0f}); + updateIconCollisionBox(symbolInstance.getIconCollisionFeature(), + opacityState.icon.placed, + hasIconTextFit ? textShift : Point{0.0f, 0.0f}); + 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; } @@ -1085,14 +1123,18 @@ namespace { optional justificationToIndex(style::TextJustifyType justify, const SymbolInstance& symbolInstance, 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; - case style::TextJustifyType::Center: return symbolInstance.placedCenterTextIndex; - case style::TextJustifyType::Left: return symbolInstance.placedLeftTextIndex; - case style::TextJustifyType::Auto: break; + case style::TextJustifyType::Right: + return symbolInstance.getPlacedRightTextIndex(); + case style::TextJustifyType::Center: + return symbolInstance.getPlacedCenterTextIndex(); + case style::TextJustifyType::Left: + return symbolInstance.getPlacedLeftTextIndex(); + case style::TextJustifyType::Auto: + break; } assert(false); return nullopt; @@ -1115,12 +1157,13 @@ void Placement::markUsedJustification(SymbolBucket& bucket, const 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(); } } } @@ -1129,34 +1172,43 @@ void Placement::markUsedJustification(SymbolBucket& bucket, void Placement::markUsedOrientation(SymbolBucket& bucket, style::TextWritingModeType orientation, const SymbolInstance& symbolInstance) const { - auto horizontal = orientation == style::TextWritingModeType::Horizontal ? - optional(orientation) : nullopt; - auto vertical = orientation == style::TextWritingModeType::Vertical ? - optional(orientation) : nullopt; + const auto horizontal = orientation == style::TextWritingModeType::Horizontal + ? optional(orientation) + : nullopt; + const auto vertical = orientation == style::TextWritingModeType::Vertical + ? optional(orientation) + : nullopt; + + if (!symbolInstance.checkIndexes(bucket.text.placedSymbols.size(), + bucket.icon.placedSymbols.size(), + bucket.sdfIcon.placedSymbols.size(), + SYM_GUARD_LOC)) { + return; + } - if (symbolInstance.placedRightTextIndex) { - bucket.text.placedSymbols.at(*symbolInstance.placedRightTextIndex).placedOrientation = horizontal; + if (symbolInstance.getPlacedRightTextIndex()) { + bucket.text.placedSymbols.at(*symbolInstance.getPlacedRightTextIndex()).placedOrientation = horizontal; } - if (symbolInstance.placedCenterTextIndex && !symbolInstance.singleLine) { - bucket.text.placedSymbols.at(*symbolInstance.placedCenterTextIndex).placedOrientation = horizontal; + if (symbolInstance.getPlacedCenterTextIndex() && !symbolInstance.getSingleLine()) { + bucket.text.placedSymbols.at(*symbolInstance.getPlacedCenterTextIndex()).placedOrientation = horizontal; } - if (symbolInstance.placedLeftTextIndex && !symbolInstance.singleLine) { - bucket.text.placedSymbols.at(*symbolInstance.placedLeftTextIndex).placedOrientation = horizontal; + if (symbolInstance.getPlacedLeftTextIndex() && !symbolInstance.getSingleLine()) { + bucket.text.placedSymbols.at(*symbolInstance.getPlacedLeftTextIndex()).placedOrientation = horizontal; } - if (symbolInstance.placedVerticalTextIndex) { - bucket.text.placedSymbols.at(*symbolInstance.placedVerticalTextIndex).placedOrientation = vertical; + 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; } } @@ -1181,8 +1233,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; } @@ -1296,7 +1348,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); @@ -1309,29 +1361,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.count(symbol.crossTileID) != 0u) 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; @@ -1417,17 +1470,16 @@ void TilePlacement::placeSymbolBucket(const BucketPlacementData& params, std::se return intersects; }; - auto symbolIntersectsTileEdges = [ - &collisionBoxIntersectsTileEdges, - variableAnchor, - pitchTextWithMap = ctx.pitchTextWithMap, - rotateTextWithMap = ctx.rotateTextWithMap, - variableIconPlacement = ctx.hasIconTextFit && !ctx.iconAllowOverlap, - bearing = static_cast(ctx.getTransformState().getBearing()) - ](const SymbolInstance& symbol) noexcept->IntersectStatus { + auto symbolIntersectsTileEdges = [&collisionBoxIntersectsTileEdges, + variableAnchor, + pitchTextWithMap = ctx.pitchTextWithMap, + rotateTextWithMap = ctx.rotateTextWithMap, + variableIconPlacement = ctx.hasIconTextFit && !ctx.iconAllowOverlap, + 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) { @@ -1436,8 +1488,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); @@ -1445,8 +1497,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; @@ -1454,8 +1506,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); @@ -1469,8 +1521,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); } @@ -1525,7 +1582,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 e0fd15595b7..689e2022233 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -250,7 +250,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 ac55c17904f..1cdd13f8cdd 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -27,9 +27,9 @@ class TileAtlasTextures; class GeometryTile : public Tile, public GlyphRequestor, public ImageRequestor { public: - GeometryTile(const OverscaledTileID&, - std::string sourceID, - const TileParameters&); + 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 00409b974a0..1dd42f1e35f 100644 --- a/src/mbgl/util/logging.cpp +++ b/src/mbgl/util/logging.cpp @@ -44,8 +44,15 @@ Log* Log::get() noexcept { return &instance; } -void Log::useLogThread(bool enable) { - useThread = enable; +void Log::useLogThread(bool enable, 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); + } } void Log::setObserver(std::unique_ptr observer) { diff --git a/src/mbgl/util/string.cpp b/src/mbgl/util/string.cpp index 718da5f1dea..5969826e382 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 938c4ae4c95..6cdc41137ef 100644 --- a/test/text/cross_tile_symbol_index.test.cpp +++ b/test/text/cross_tile_symbol_index.test.cpp @@ -68,8 +68,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); @@ -97,13 +97,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; @@ -127,7 +127,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); @@ -157,9 +157,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); } @@ -217,7 +217,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; @@ -225,11 +225,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) { @@ -288,14 +288,17 @@ 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, 1u); // A' copies from A - ASSERT_EQ(childBucket.symbolInstances.at(1).crossTileID, 2u); // B' copies from B - ASSERT_EQ(childBucket.symbolInstances.at(2).crossTileID, 3u); // C' gets new ID + ASSERT_EQ(childBucket.symbolInstances.at(0).getCrossTileID(), + 1u); // A' copies from A + ASSERT_EQ(childBucket.symbolInstances.at(1).getCrossTileID(), + 2u); // B' copies from B + ASSERT_EQ(childBucket.symbolInstances.at(2).getCrossTileID(), + 3u); // C' gets new ID } TEST(CrossTileSymbolLayerIndex, bucketReplacement) { @@ -353,14 +356,17 @@ 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, 1u); // A' copies from A - ASSERT_EQ(secondBucket.symbolInstances.at(1).crossTileID, 2u); // B' copies from B - ASSERT_EQ(secondBucket.symbolInstances.at(2).crossTileID, 3u); // C' gets new ID + ASSERT_EQ(secondBucket.symbolInstances.at(0).getCrossTileID(), + 1u); // A' copies from A + ASSERT_EQ(secondBucket.symbolInstances.at(1).getCrossTileID(), + 2u); // B' copies from B + ASSERT_EQ(secondBucket.symbolInstances.at(2).getCrossTileID(), + 3u); // C' gets new ID } namespace { @@ -408,12 +414,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); } From 95673cbb1aba0842132c020d3b4b6e66850cb314 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Mon, 14 Oct 2024 08:24:19 -0700 Subject: [PATCH 2/3] Fix merge errors, C++20 stuff --- include/mbgl/util/event.hpp | 1 + include/mbgl/util/logging.hpp | 2 +- include/mbgl/util/string.hpp | 8 +++--- src/mbgl/layout/symbol_instance.hpp | 6 +++-- src/mbgl/layout/symbol_layout.cpp | 30 ++++++++++----------- src/mbgl/renderer/buckets/symbol_bucket.cpp | 12 ++++----- src/mbgl/text/cross_tile_symbol_index.cpp | 2 +- src/mbgl/text/cross_tile_symbol_index.hpp | 2 +- src/mbgl/text/placement.cpp | 6 ++--- src/mbgl/util/logging.cpp | 23 +++++++++++----- 10 files changed, 54 insertions(+), 38 deletions(-) diff --git a/include/mbgl/util/event.hpp b/include/mbgl/util/event.hpp index 15583e956d4..2bc4e9a1850 100644 --- a/include/mbgl/util/event.hpp +++ b/include/mbgl/util/event.hpp @@ -9,6 +9,7 @@ enum class EventSeverity : uint8_t { Info, Warning, Error, + SeverityCount, }; enum class Event : uint8_t { diff --git a/include/mbgl/util/logging.hpp b/include/mbgl/util/logging.hpp index f1b512a10a7..3cea6872395 100644 --- a/include/mbgl/util/logging.hpp +++ b/include/mbgl/util/logging.hpp @@ -44,7 +44,7 @@ class Log { /// /// 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 = {}); + static void useLogThread(bool enable, optional = {}); template static void Debug(Event event, Args&& ...args) { diff --git a/include/mbgl/util/string.hpp b/include/mbgl/util/string.hpp index f00b70ff42f..36b37ff9a91 100644 --- a/include/mbgl/util/string.hpp +++ b/include/mbgl/util/string.hpp @@ -1,9 +1,10 @@ #pragma once -#include #include -#include #include +#include +#include +#include // Polyfill needed by Qt when building for Android with GCC #if defined(__ANDROID__) && defined(__GLIBCXX__) @@ -76,7 +77,8 @@ inline std::string toString(long double t, bool decimal = false) { return toString(static_cast(t), decimal); } -inline std::string toString(std::thread::id threadId) { +std::string toString(std::thread::id); + std::string toString(const std::exception_ptr &); template diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp index 014279943ed..2e155fdd631 100644 --- a/src/mbgl/layout/symbol_instance.hpp +++ b/src/mbgl/layout/symbol_instance.hpp @@ -6,7 +6,9 @@ #include #include +#if __cplusplus >= 202002L // C++20 #include +#endif #if !defined(MLN_SYMBOL_GUARDS) #define MLN_SYMBOL_GUARDS 1 @@ -231,9 +233,9 @@ class SymbolInstance { SYM_GUARD_VALUE(09) CollisionFeature iconCollisionFeature; SYM_GUARD_VALUE(10) - optional verticalTextCollisionFeature = std::nullopt; + optional verticalTextCollisionFeature = nullopt; SYM_GUARD_VALUE(11) - optional verticalIconCollisionFeature = std::nullopt; + optional verticalIconCollisionFeature = nullopt; SYM_GUARD_VALUE(12) WritingModeType writingModes; SYM_GUARD_VALUE(13) diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index 31dc6f90c19..df303b93dd1 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -251,7 +251,7 @@ std::array evaluateRadialOffset(style::SymbolAnchorType anchor, float std::array result{{0.0f, 0.0f}}; 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; + const float hypotenuse = static_cast(radialOffset / M_SQRT2); switch (anchor) { case SymbolAnchorType::TopRight: @@ -804,7 +804,7 @@ void SymbolLayout::createBucket(const ImagePositions&, index = iconBuffer.placedSymbols.size() - 1; PlacedSymbol& iconSymbol = iconBuffer.placedSymbols.back(); iconSymbol.angle = (allowVerticalPlacement && writingMode == WritingModeType::Vertical) - ? pi_v / 2 + ? static_cast(M_PI_2) : 0.0f; iconSymbol.vertexStartIndex = addSymbols( iconBuffer, sizeData, iconQuads, symbolInstance.getAnchor(), iconSymbol, feature.sortKey); @@ -821,7 +821,7 @@ void SymbolLayout::createBucket(const ImagePositions&, for (auto& pair : bucket->paintProperties) { pair.second.iconBinders.populateVertexVectors( - feature, iconBuffer.vertices().elements(), symbolInstance.getDataFeatureIndex(), {}, {}, canonical); + feature, iconBuffer.vertices.elements(), symbolInstance.getDataFeatureIndex(), {}, {}, canonical); } } @@ -1029,16 +1029,16 @@ size_t SymbolLayout::addSymbol(SymbolBucket::Buffer& buffer, // Dynamic/Opacity vertices are initialized so that the vertex count always agrees with // the layout vertex buffer, but they will always be updated before rendering happens auto dynamicVertex = SymbolSDFIconProgram::dynamicLayoutVertex(labelAnchor.point, 0); - buffer.dynamicVertices().emplace_back(dynamicVertex); - buffer.dynamicVertices().emplace_back(dynamicVertex); - buffer.dynamicVertices().emplace_back(dynamicVertex); - buffer.dynamicVertices().emplace_back(dynamicVertex); + buffer.dynamicVertices.emplace_back(dynamicVertex); + buffer.dynamicVertices.emplace_back(dynamicVertex); + buffer.dynamicVertices.emplace_back(dynamicVertex); + buffer.dynamicVertices.emplace_back(dynamicVertex); auto opacityVertex = SymbolSDFIconProgram::opacityVertex(true, 1.0); - buffer.opacityVertices().emplace_back(opacityVertex); - buffer.opacityVertices().emplace_back(opacityVertex); - buffer.opacityVertices().emplace_back(opacityVertex); - buffer.opacityVertices().emplace_back(opacityVertex); + buffer.opacityVertices.emplace_back(opacityVertex); + buffer.opacityVertices.emplace_back(opacityVertex); + buffer.opacityVertices.emplace_back(opacityVertex); + buffer.opacityVertices.emplace_back(opacityVertex); // add the two triangles, referencing the four coordinates we just inserted. buffer.triangles.emplace_back(index + 0, index + 1, index + 2); @@ -1106,13 +1106,13 @@ void SymbolLayout::addToDebugBuffers(SymbolBucket& bucket) { auto& segment = collisionBuffer.segments.back(); auto index = static_cast(segment.vertexLength); - collisionBuffer.vertices().emplace_back( + collisionBuffer.vertices.emplace_back( CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, tl)); - collisionBuffer.vertices().emplace_back( + collisionBuffer.vertices.emplace_back( CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, tr)); - collisionBuffer.vertices().emplace_back( + collisionBuffer.vertices.emplace_back( CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, br)); - collisionBuffer.vertices().emplace_back( + collisionBuffer.vertices.emplace_back( CollisionBoxProgram::layoutVertex(anchor, symbolInstance.getAnchor().point, bl)); // Dynamic vertices are initialized so that the vertex count always agrees with diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp index 856209d1547..ce7b99d173c 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.cpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp @@ -313,12 +313,12 @@ SymbolInstanceReferences SymbolBucket::getSymbols(const optional& #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()) { + 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; } diff --git a/src/mbgl/text/cross_tile_symbol_index.cpp b/src/mbgl/text/cross_tile_symbol_index.cpp index 905c0b6e5d9..381d20c9a6b 100644 --- a/src/mbgl/text/cross_tile_symbol_index.cpp +++ b/src/mbgl/text/cross_tile_symbol_index.cpp @@ -23,7 +23,7 @@ TileLayerIndex::TileLayerIndex(OverscaledTileID coord_, } } -Point TileLayerIndex::getScaledCoordinates(SymbolInstance& symbolInstance, +Point TileLayerIndex::getScaledCoordinates(const SymbolInstance& symbolInstance, const OverscaledTileID& childTileCoord) const { // Round anchor positions to roughly 4 pixel grid const double roundingFactor = 512.0 / util::EXTENT / 2.0; diff --git a/src/mbgl/text/cross_tile_symbol_index.hpp b/src/mbgl/text/cross_tile_symbol_index.hpp index 542f297fd83..dda58ceb494 100644 --- a/src/mbgl/text/cross_tile_symbol_index.hpp +++ b/src/mbgl/text/cross_tile_symbol_index.hpp @@ -38,7 +38,7 @@ class TileLayerIndex { uint32_t bucketInstanceId, std::string bucketLeaderId); - Point getScaledCoordinates(SymbolInstance&, const OverscaledTileID&) const; + Point getScaledCoordinates(const SymbolInstance&, const OverscaledTileID&) const; void findMatches(SymbolBucket&, const OverscaledTileID&, std::set&) const; OverscaledTileID coord; diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp index f0a56f9f192..c49993f1bb8 100644 --- a/src/mbgl/text/placement.cpp +++ b/src/mbgl/text/placement.cpp @@ -240,7 +240,7 @@ void Placement::placeSymbolBucket(const BucketPlacementData& params, std::set #include #include +#include #include #include @@ -13,7 +14,8 @@ namespace mbgl { namespace { std::unique_ptr currentObserver; -std::atomic useThread(true); +constexpr auto SeverityCount = underlying_type(EventSeverity::SeverityCount); +std::atomic useThread[SeverityCount] = {true, true, true, false}; std::mutex mutex; } // namespace @@ -23,11 +25,20 @@ class Log::Impl { Impl() : scheduler(Scheduler::GetSequenced()) {} void record(EventSeverity severity, Event event, int64_t code, const std::string& msg) { - if (useThread) { - auto threadName = platform::getCurrentThreadName(); - scheduler->schedule([=]() { Log::record(severity, event, code, msg, threadName); }); - } else { - Log::record(severity, event, code, msg, {}); + try { + if (useThread[underlying_type(severity)]) { + auto threadName = platform::getCurrentThreadName(); + scheduler->schedule([=]() { Log::record(severity, event, code, msg, threadName); }); + } else { + Log::record(severity, event, code, msg, {}); + } + } catch (...) { + // ignore exceptions during logging + // What would we do, log them? +#if !defined(NDEBUG) + [[maybe_unused]] auto ex = std::current_exception(); + assert(!"unhandled exception while logging"); +#endif } } From fd872db7f8a8d42c0f69e5e1e9a41d1c1bbd1ad6 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Mon, 14 Oct 2024 11:34:32 -0700 Subject: [PATCH 3/3] tweak version check for Linux build --- src/mbgl/layout/symbol_instance.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp index 2e155fdd631..ad04eb61c14 100644 --- a/src/mbgl/layout/symbol_instance.hpp +++ b/src/mbgl/layout/symbol_instance.hpp @@ -23,7 +23,7 @@ // A temporary shim for partial C++20 support #if MLN_SYMBOL_GUARDS #if defined(__clang__) -#if __cplusplus <= 201703L || !__has_builtin(__builtin_source_location) +#if __cplusplus < 202002L || !__has_builtin(__builtin_source_location) namespace std { struct source_location { const char* fileName_;