From c2899f76b68ebaa2bba61e4407e128ab261ab212 Mon Sep 17 00:00:00 2001 From: Alexander Shalamov Date: Wed, 4 Dec 2019 14:53:32 +0200 Subject: [core] Don't use signed int type for anchor segment --- src/mbgl/geometry/anchor.hpp | 5 ++-- src/mbgl/layout/symbol_layout.cpp | 37 ++++++++++++++++++++--------- src/mbgl/renderer/buckets/symbol_bucket.hpp | 28 ++++++++++++++++------ src/mbgl/text/check_max_angle.cpp | 11 ++++----- src/mbgl/text/collision_feature.cpp | 20 +++++++++------- src/mbgl/text/collision_feature.hpp | 8 +++++-- src/mbgl/text/get_anchors.cpp | 6 ++--- 7 files changed, 75 insertions(+), 40 deletions(-) diff --git a/src/mbgl/geometry/anchor.hpp b/src/mbgl/geometry/anchor.hpp index b24d8d04e0..1a2cbd3dac 100644 --- a/src/mbgl/geometry/anchor.hpp +++ b/src/mbgl/geometry/anchor.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include @@ -11,9 +12,9 @@ public: Point point; float angle = 0.0f; float scale = 0.0f; - int segment = -1; + optional segment; - Anchor(float x_, float y_, float angle_, float scale_, int segment_ = -1) + Anchor(float x_, float y_, float angle_, float scale_, optional segment_ = nullopt) : point(x_, y_), angle(angle_), scale(scale_), segment(segment_) {} }; diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp index ca61844417..2e339af482 100644 --- a/src/mbgl/layout/symbol_layout.cpp +++ b/src/mbgl/layout/symbol_layout.cpp @@ -695,18 +695,20 @@ bool SymbolLayout::anchorIsTooClose(const std::u16string& text, const float repe // JS version does, but it uses the same logic to calculate tile distances. std::vector CalculateTileDistances(const GeometryCoordinates& line, const Anchor& anchor) { std::vector tileDistances(line.size()); - if (anchor.segment != -1) { - auto sumForwardLength = util::dist(anchor.point, line[anchor.segment + 1]); - auto sumBackwardLength = util::dist(anchor.point, line[anchor.segment]); - for (size_t i = anchor.segment + 1; i < line.size(); i++) { + if (anchor.segment) { + std::size_t segment = *anchor.segment; + assert(segment < line.size()); + auto sumForwardLength = util::dist(anchor.point, line[segment + 1]); + auto sumBackwardLength = util::dist(anchor.point, line[segment]); + for (std::size_t i = segment + 1; i < line.size(); ++i) { tileDistances[i] = sumForwardLength; if (i < line.size() - 1) { sumForwardLength += util::dist(line[i + 1], line[i]); } } - for (auto i = anchor.segment; i >= 0; i--) { + for (std::size_t i = ++segment; i-- > 0;) { tileDistances[i] = sumBackwardLength; - if (i > 0) { + if (i != 0u) { sumBackwardLength += util::dist(line[i - 1], line[i]); } } @@ -743,9 +745,15 @@ void SymbolLayout::createBucket(const ImagePositions&, std::unique_ptr sizeData = bucket->iconSizeBinder->getVertexSizeData(feature); auto& iconBuffer = symbolInstance.hasSdfIcon() ? bucket->sdfIcon : bucket->icon; - const auto placeIcon = [&] (const SymbolQuad& iconQuad, auto& index, const WritingModeType writingMode) { - iconBuffer.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max, - symbolInstance.iconOffset, writingMode, symbolInstance.line(), std::vector()); + const auto placeIcon = [&](const SymbolQuad& iconQuad, auto& index, const WritingModeType writingMode) { + iconBuffer.placedSymbols.emplace_back(symbolInstance.anchor.point, + symbolInstance.anchor.segment.value_or(0u), + sizeData.min, + sizeData.max, + symbolInstance.iconOffset, + writingMode, + symbolInstance.line(), + std::vector()); index = iconBuffer.placedSymbols.size() - 1; PlacedSymbol& iconSymbol = iconBuffer.placedSymbols.back(); iconSymbol.angle = (allowVerticalPlacement && writingMode == WritingModeType::Vertical) ? M_PI_2 : 0; @@ -827,8 +835,15 @@ std::size_t SymbolLayout::addSymbolGlyphQuads(SymbolBucket& bucket, 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, sizeData.min, sizeData.max, - symbolInstance.textOffset, writingMode, symbolInstance.line(), CalculateTileDistances(symbolInstance.line(), symbolInstance.anchor), placedIconIndex); + bucket.text.placedSymbols.emplace_back(symbolInstance.anchor.point, + symbolInstance.anchor.segment.value_or(0u), + sizeData.min, + sizeData.max, + symbolInstance.textOffset, + writingMode, + symbolInstance.line(), + CalculateTileDistances(symbolInstance.line(), symbolInstance.anchor), + placedIconIndex); placedIndex = bucket.text.placedSymbols.size() - 1; PlacedSymbol& placedSymbol = bucket.text.placedSymbols.back(); placedSymbol.angle = (allowVerticalPlacement && writingMode == WritingModeType::Vertical) ? M_PI_2 : 0; diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp index 7a60a2c1e4..1d1aa13dd0 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.hpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp @@ -20,14 +20,28 @@ class CrossTileSymbolLayerIndex; class PlacedSymbol { public: - PlacedSymbol(Point anchorPoint_, uint16_t segment_, float lowerSize_, float upperSize_, - std::array lineOffset_, WritingModeType writingModes_, GeometryCoordinates line_, std::vector tileDistances_, optional placedIconIndex_ = nullopt) : - anchorPoint(anchorPoint_), segment(segment_), lowerSize(lowerSize_), upperSize(upperSize_), - lineOffset(lineOffset_), writingModes(writingModes_), line(std::move(line_)), tileDistances(std::move(tileDistances_)), hidden(false), vertexStartIndex(0), placedIconIndex(std::move(placedIconIndex_)) - { - } + PlacedSymbol(Point anchorPoint_, + std::size_t segment_, + float lowerSize_, + float upperSize_, + std::array lineOffset_, + WritingModeType writingModes_, + GeometryCoordinates line_, + std::vector tileDistances_, + optional placedIconIndex_ = nullopt) + : anchorPoint(anchorPoint_), + segment(segment_), + lowerSize(lowerSize_), + upperSize(upperSize_), + lineOffset(lineOffset_), + writingModes(writingModes_), + line(std::move(line_)), + tileDistances(std::move(tileDistances_)), + hidden(false), + vertexStartIndex(0), + placedIconIndex(std::move(placedIconIndex_)) {} Point anchorPoint; - uint16_t segment; + std::size_t segment; float lowerSize; float upperSize; std::array lineOffset; diff --git a/src/mbgl/text/check_max_angle.cpp b/src/mbgl/text/check_max_angle.cpp index 9109235798..0569767634 100644 --- a/src/mbgl/text/check_max_angle.cpp +++ b/src/mbgl/text/check_max_angle.cpp @@ -19,20 +19,19 @@ bool checkMaxAngle(const GeometryCoordinates& line, const float windowSize, const float maxAngle) { // horizontal labels always pass - if (anchor.segment < 0) return true; + if (!anchor.segment) return true; GeometryCoordinate anchorPoint = convertPoint(anchor.point); GeometryCoordinate &p = anchorPoint; - int index = anchor.segment + 1; + std::size_t index = *anchor.segment + 1; float anchorDistance = 0; // move backwards along the line to the first segment the label appears on while (anchorDistance > -labelLength / 2) { - index--; - // there isn't enough room for the label after the beginning of the line - if (index < 0) return false; + if (index == 0u) return false; + index--; anchorDistance -= util::dist(line[index], p); p = line[index]; } @@ -48,7 +47,7 @@ bool checkMaxAngle(const GeometryCoordinates& line, while (anchorDistance < labelLength / 2) { // there isn't enough room for the label before the end of the line - if (index + 1 >= (int)line.size()) return false; + if (index + 1 >= line.size()) return false; auto& prev = line[index - 1]; auto& current = line[index]; diff --git a/src/mbgl/text/collision_feature.cpp b/src/mbgl/text/collision_feature.cpp index f9f6b3a4a5..744ba916c6 100644 --- a/src/mbgl/text/collision_feature.cpp +++ b/src/mbgl/text/collision_feature.cpp @@ -34,7 +34,7 @@ CollisionFeature::CollisionFeature(const GeometryCoordinates& line, height = std::max(10.0f * boxScale, height); GeometryCoordinate anchorPoint = convertPoint(anchor.point); - bboxifyLabel(line, anchorPoint, anchor.segment, length, height, overscaling); + bboxifyLabel(line, anchorPoint, anchor.segment.value_or(0u), length, height, overscaling); } else { if (rotate) { // Account for *-rotate in point collision boxes @@ -61,8 +61,12 @@ CollisionFeature::CollisionFeature(const GeometryCoordinates& line, } } -void CollisionFeature::bboxifyLabel(const GeometryCoordinates& line, GeometryCoordinate& anchorPoint, - const int segment, const float labelLength, const float boxSize, const float overscaling) { +void CollisionFeature::bboxifyLabel(const GeometryCoordinates& line, + GeometryCoordinate& anchorPoint, + std::size_t segment, + const float labelLength, + const float boxSize, + const float overscaling) { const float step = boxSize / 2; const int nBoxes = std::max(static_cast(std::floor(labelLength / step)), 1); @@ -82,16 +86,14 @@ void CollisionFeature::bboxifyLabel(const GeometryCoordinates& line, GeometryCoo const float firstBoxOffset = -boxSize / 2; GeometryCoordinate &p = anchorPoint; - int index = segment + 1; + std::size_t index = segment + 1; float anchorDistance = firstBoxOffset; const float labelStartDistance = -labelLength / 2; const float paddingStartDistance = labelStartDistance - labelLength / 8; // move backwards along the line to the first segment the label appears on do { - index--; - - if (index < 0) { + if (index == 0u) { if (anchorDistance > labelStartDistance) { // there isn't enough room for the label after the beginning of the line // checkMaxAngle should have already caught this @@ -104,6 +106,7 @@ void CollisionFeature::bboxifyLabel(const GeometryCoordinates& line, GeometryCoo } } + index--; anchorDistance -= util::dist(line[index], p); p = line[index]; } while (anchorDistance > paddingStartDistance); @@ -131,7 +134,7 @@ void CollisionFeature::bboxifyLabel(const GeometryCoordinates& line, GeometryCoo index++; // There isn't enough room before the end of the line. - if (index + 1 >= (int)line.size()) return; + if (index + 1 >= line.size()) return; segmentLength = util::dist(line[index], line[index + 1]); } @@ -159,5 +162,4 @@ void CollisionFeature::bboxifyLabel(const GeometryCoordinates& line, GeometryCoo } } - } // namespace mbgl diff --git a/src/mbgl/text/collision_feature.hpp b/src/mbgl/text/collision_feature.hpp index 8767a5b6f7..ebda4a5673 100644 --- a/src/mbgl/text/collision_feature.hpp +++ b/src/mbgl/text/collision_feature.hpp @@ -122,8 +122,12 @@ public: bool alongLine; private: - void bboxifyLabel(const GeometryCoordinates& line, GeometryCoordinate& anchorPoint, - const int segment, const float length, const float height, const float overscaling); + void bboxifyLabel(const GeometryCoordinates& line, + GeometryCoordinate& anchorPoint, + std::size_t segment, + const float length, + const float height, + const float overscaling); }; } // namespace mbgl diff --git a/src/mbgl/text/get_anchors.cpp b/src/mbgl/text/get_anchors.cpp index c38e181181..a2401a020a 100644 --- a/src/mbgl/text/get_anchors.cpp +++ b/src/mbgl/text/get_anchors.cpp @@ -40,7 +40,7 @@ static Anchors resample(const GeometryCoordinates& line, assert(spacing > 0.0); - int i = 0; + std::size_t i = 0u; for (auto it = line.begin(), end = line.end() - 1; it != end; it++, i++) { const GeometryCoordinate& a = *(it); const GeometryCoordinate& b = *(it + 1); @@ -147,8 +147,8 @@ optional getCenterAnchor(const GeometryCoordinates& line, float prevDistance = 0; const float centerDistance = getLineLength(line) / 2; - - int i = 0; + + std::size_t i = 0u; for (auto it = line.begin(), end = line.end() - 1; it != end; it++, i++) { const GeometryCoordinate& a = *(it); const GeometryCoordinate& b = *(it + 1); -- cgit v1.2.1