From 2bf2246a38cae1b004a2b154b00f95dff5ce1317 Mon Sep 17 00:00:00 2001 From: Alban Kraus <11193174+alkra@users.noreply.github.com> Date: Tue, 19 Jan 2021 17:02:22 +0100 Subject: [PATCH] Handle exact distances in curveSubstring Fixes #41081 These exact distances may be obtained with distance_to_vertex. Before this change, the code considered the line segments as open intervals, so that vertices could not ever be considered as the start of a substring. This change considers them as semi-open intervals (closed at the beginning) instead. (With a special case when starting the substring at the last vertex) Before this change, vertices could not be considered as the end of a substring, so an other loop was required, adding a duplicate node. Similar behaviour is observed for QgsCircularString and corrected similarly. Double equality is performed as exact equality, because it does not matter on which segment the start of the substring is. Except where startDistance is -0.0, which is already handled before the for loop, or when startDistance is at the last vertex. (cherry picked from commit 5b2685d616e2a8abde33747fae3cd5f9a416190b) --- src/core/geometry/qgscircularstring.cpp | 29 ++++++++++++++----------- src/core/geometry/qgslinestring.cpp | 29 ++++++++++++++----------- tests/src/core/testqgsgeometry.cpp | 4 ++++ 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/core/geometry/qgscircularstring.cpp b/src/core/geometry/qgscircularstring.cpp index 767822e23000..a4e9fc0c8c11 100644 --- a/src/core/geometry/qgscircularstring.cpp +++ b/src/core/geometry/qgscircularstring.cpp @@ -1278,12 +1278,12 @@ QgsCircularString *QgsCircularString::curveSubstring( double startDistance, doub endDistance = std::max( startDistance, endDistance ); - double distanceTraversed = 0; const int totalPoints = numPoints(); if ( totalPoints == 0 ) return clone(); QVector< QgsPoint > substringPoints; + substringPoints.reserve( totalPoints ); QgsWkbTypes::Type pointType = QgsWkbTypes::Point; if ( is3D() ) @@ -1296,19 +1296,15 @@ QgsCircularString *QgsCircularString::curveSubstring( double startDistance, doub const double *z = is3D() ? mZ.constData() : nullptr; const double *m = isMeasure() ? mM.constData() : nullptr; + double distanceTraversed = 0; double prevX = *x++; double prevY = *y++; double prevZ = z ? *z++ : 0.0; double prevM = m ? *m++ : 0.0; bool foundStart = false; - if ( qgsDoubleNear( startDistance, 0.0 ) || startDistance < 0 ) - { - substringPoints << QgsPoint( pointType, prevX, prevY, prevZ, prevM ); - foundStart = true; - } - - substringPoints.reserve( totalPoints ); + if ( startDistance < 0 ) + startDistance = 0; for ( int i = 0; i < ( totalPoints - 2 ) ; i += 2 ) { @@ -1329,7 +1325,7 @@ QgsCircularString *QgsCircularString::curveSubstring( double startDistance, doub bool addedSegmentEnd = false; const double segmentLength = QgsGeometryUtils::circleLength( x1, y1, x2, y2, x3, y3 ); - if ( distanceTraversed < startDistance && distanceTraversed + segmentLength > startDistance ) + if ( distanceTraversed <= startDistance && startDistance < distanceTraversed + segmentLength ) { // start point falls on this segment const double distanceToStart = startDistance - distanceTraversed; @@ -1383,14 +1379,21 @@ QgsCircularString *QgsCircularString::curveSubstring( double startDistance, doub << QgsPoint( pointType, x3, y3, z3, m3 ); } - distanceTraversed += segmentLength; - if ( distanceTraversed > endDistance ) - break; - prevX = x3; prevY = y3; prevZ = z3; prevM = m3; + distanceTraversed += segmentLength; + if ( distanceTraversed >= endDistance ) + break; + } + + // start point is the last node + if ( !foundStart && qgsDoubleNear( distanceTraversed, startDistance ) ) + { + substringPoints << QgsPoint( pointType, prevX, prevY, prevZ, prevM ) + << QgsPoint( pointType, prevX, prevY, prevZ, prevM ) + << QgsPoint( pointType, prevX, prevY, prevZ, prevM ); } std::unique_ptr< QgsCircularString > result = qgis::make_unique< QgsCircularString >(); diff --git a/src/core/geometry/qgslinestring.cpp b/src/core/geometry/qgslinestring.cpp index e6dc5a3343f5..c94273fabd46 100644 --- a/src/core/geometry/qgslinestring.cpp +++ b/src/core/geometry/qgslinestring.cpp @@ -1030,12 +1030,12 @@ QgsLineString *QgsLineString::curveSubstring( double startDistance, double endDi endDistance = std::max( startDistance, endDistance ); - double distanceTraversed = 0; const int totalPoints = numPoints(); if ( totalPoints == 0 ) return clone(); QVector< QgsPoint > substringPoints; + substringPoints.reserve( totalPoints ); QgsWkbTypes::Type pointType = QgsWkbTypes::Point; if ( is3D() ) @@ -1048,19 +1048,15 @@ QgsLineString *QgsLineString::curveSubstring( double startDistance, double endDi const double *z = is3D() ? mZ.constData() : nullptr; const double *m = isMeasure() ? mM.constData() : nullptr; + double distanceTraversed = 0; double prevX = *x++; double prevY = *y++; double prevZ = z ? *z++ : 0.0; double prevM = m ? *m++ : 0.0; bool foundStart = false; - if ( qgsDoubleNear( startDistance, 0.0 ) || startDistance < 0 ) - { - substringPoints << QgsPoint( pointType, prevX, prevY, prevZ, prevM ); - foundStart = true; - } - - substringPoints.reserve( totalPoints ); + if ( startDistance < 0 ) + startDistance = 0; for ( int i = 1; i < totalPoints; ++i ) { @@ -1070,7 +1066,8 @@ QgsLineString *QgsLineString::curveSubstring( double startDistance, double endDi double thisM = m ? *m++ : 0.0; const double segmentLength = std::sqrt( ( thisX - prevX ) * ( thisX - prevX ) + ( thisY - prevY ) * ( thisY - prevY ) ); - if ( distanceTraversed < startDistance && distanceTraversed + segmentLength > startDistance ) + + if ( distanceTraversed <= startDistance && startDistance < distanceTraversed + segmentLength ) { // start point falls on this segment const double distanceToStart = startDistance - distanceTraversed; @@ -1100,14 +1097,20 @@ QgsLineString *QgsLineString::curveSubstring( double startDistance, double endDi substringPoints << QgsPoint( pointType, thisX, thisY, thisZ, thisM ); } - distanceTraversed += segmentLength; - if ( distanceTraversed > endDistance ) - break; - prevX = thisX; prevY = thisY; prevZ = thisZ; prevM = thisM; + distanceTraversed += segmentLength; + if ( distanceTraversed >= endDistance ) + break; + } + + // start point is the last node + if ( !foundStart && qgsDoubleNear( distanceTraversed, startDistance ) ) + { + substringPoints << QgsPoint( pointType, prevX, prevY, prevZ, prevM ) + << QgsPoint( pointType, prevX, prevY, prevZ, prevM ); } return new QgsLineString( substringPoints ); diff --git a/tests/src/core/testqgsgeometry.cpp b/tests/src/core/testqgsgeometry.cpp index f982677d60d6..23d1a1bcf637 100644 --- a/tests/src/core/testqgsgeometry.cpp +++ b/tests/src/core/testqgsgeometry.cpp @@ -2846,6 +2846,8 @@ void TestQgsGeometry::circularString() QCOMPARE( substringResult->asWkt( 2 ), QStringLiteral( "CircularStringZM (13.51 -0.98 13 14, 14.01 -1 13.03 14.03, 14.5 -0.9 14.65 15.65)" ) ); substringResult.reset( substring.curveSubstring( 5, 1000 ) ); QCOMPARE( substringResult->asWkt( 2 ), QStringLiteral( "CircularStringZM (13.51 -0.98 13 14, 15.19 -0.53 17.2 18.2, 16 1 23 24)" ) ); + substringResult.reset( substring.curveSubstring( QgsGeometryUtils::distanceToVertex( substring, QgsVertexId( 0, 0, 2 ) ), QgsGeometryUtils::distanceToVertex( substring, QgsVertexId( 0, 0, 4 ) ) ) ); + QCOMPARE( substringResult->asWkt( 2 ), QStringLiteral( "CircularStringZM (12 0 13 14, 14.36 -0.94 14.19 15.19, 16 1 23 24)" ) ); substring.setPoints( QgsPointSequence() << QgsPoint( 10, 0, 1 ) << QgsPoint( 11, 1, 3 ) << QgsPoint( 12, 0, 13 ) << QgsPoint( 14, -1, 13 ) << QgsPoint( 16, 1, 23 ) ); @@ -4930,6 +4932,8 @@ void TestQgsGeometry::lineString() QCOMPARE( substringResult->asWkt( 2 ), QStringLiteral( "LineStringZM (11 3 4 5, 11 12 13 14, 111 12 23 24)" ) ); substringResult.reset( substring.curveSubstring( 1, 20 ) ); QCOMPARE( substringResult->asWkt( 2 ), QStringLiteral( "LineStringZM (11 3 4 5, 11 12 13 14, 21 12 14 15)" ) ); + substringResult.reset( substring.curveSubstring( QgsGeometryUtils::distanceToVertex( substring, QgsVertexId( 0, 0, 1 ) ), QgsGeometryUtils::distanceToVertex( substring, QgsVertexId( 0, 0, 2 ) ) ) ); + QCOMPARE( substringResult->asWkt( 2 ), QStringLiteral( "LineStringZM (11 12 13 14, 111 12 23 24)" ) ); substring.setPoints( QgsPointSequence() << QgsPoint( 11, 2, 3, 0, QgsWkbTypes::PointZ ) << QgsPoint( 11, 12, 13, 0, QgsWkbTypes::PointZ ) << QgsPoint( 111, 12, 23, 0, QgsWkbTypes::PointZ ) ); substringResult.reset( substring.curveSubstring( 1, 20 ) ); QCOMPARE( substringResult->asWkt( 2 ), QStringLiteral( "LineStringZ (11 3 4, 11 12 13, 21 12 14)" ) );