From b1d22b9e0a6e8a2fe6c393817fb1319de44d9a21 Mon Sep 17 00:00:00 2001 From: Mathieu Pellerin Date: Tue, 20 Feb 2024 20:27:45 +0700 Subject: [PATCH 1/2] [geometry snapper] Fix wrong point-to-segment distance within maths --- src/analysis/vector/qgsgeometrysnapper.cpp | 9 +++++---- src/analysis/vector/qgsgeometrysnapper.h | 2 +- tests/src/analysis/testqgsgeometrysnapper.cpp | 4 ++++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/analysis/vector/qgsgeometrysnapper.cpp b/src/analysis/vector/qgsgeometrysnapper.cpp index daea5921819f..82bfa8817e8c 100644 --- a/src/analysis/vector/qgsgeometrysnapper.cpp +++ b/src/analysis/vector/qgsgeometrysnapper.cpp @@ -99,7 +99,7 @@ bool QgsSnapIndex::SegmentSnapItem::getProjection( const QgsPoint &p, QgsPoint & return true; } -bool QgsSnapIndex::SegmentSnapItem::withinDistance( const QgsPoint &p, const double tolerance ) +bool QgsSnapIndex::SegmentSnapItem::withinSqrDistance( const QgsPoint &p, const double tolerance ) { double minDistX, minDistY; const double distance = QgsGeometryUtilsBase::sqrDistToLine( p.x(), p.y(), idxFrom->point().x(), idxFrom->point().y(), idxTo->point().x(), idxTo->point().y(), minDistX, minDistY, 4 * std::numeric_limits::epsilon() ); @@ -250,6 +250,7 @@ QgsSnapIndex::SnapItem *QgsSnapIndex::getSnapItem( const QgsPoint &pos, const do QgsSnapIndex::SegmentSnapItem *snapSegment = nullptr; QgsSnapIndex::PointSnapItem *snapPoint = nullptr; + const double sqrTolerance = tolerance * tolerance; const auto constItems = items; for ( QgsSnapIndex::SnapItem *item : constItems ) { @@ -264,7 +265,7 @@ QgsSnapIndex::SnapItem *QgsSnapIndex::getSnapItem( const QgsPoint &pos, const do } else if ( item->type == SnapSegment && !endPointOnly ) { - if ( !static_cast( item )->withinDistance( pos, tolerance ) ) + if ( !static_cast( item )->withinSqrDistance( pos, sqrTolerance ) ) continue; QgsPoint pProj; @@ -279,8 +280,8 @@ QgsSnapIndex::SnapItem *QgsSnapIndex::getSnapItem( const QgsPoint &pos, const do } } } - snapPoint = minDistPoint < tolerance * tolerance ? snapPoint : nullptr; - snapSegment = minDistSegment < tolerance * tolerance ? snapSegment : nullptr; + snapPoint = minDistPoint < sqrTolerance ? snapPoint : nullptr; + snapSegment = minDistSegment < sqrTolerance ? snapSegment : nullptr; if ( pSnapPoint ) *pSnapPoint = snapPoint; if ( pSnapSegment ) *pSnapSegment = snapSegment; return minDistPoint < minDistSegment ? static_cast( snapPoint ) : static_cast( snapSegment ); diff --git a/src/analysis/vector/qgsgeometrysnapper.h b/src/analysis/vector/qgsgeometrysnapper.h index 0972d693a266..5f689f6b3bc3 100644 --- a/src/analysis/vector/qgsgeometrysnapper.h +++ b/src/analysis/vector/qgsgeometrysnapper.h @@ -209,7 +209,7 @@ class QgsSnapIndex QgsPoint getSnapPoint( const QgsPoint &p ) const override; bool getIntersection( const QgsPoint &p1, const QgsPoint &p2, QgsPoint &inter ) const; bool getProjection( const QgsPoint &p, QgsPoint &pProj ) const; - bool withinDistance( const QgsPoint &p, const double distance ); + bool withinSqrDistance( const QgsPoint &p, const double distance ); const CoordIdx *idxFrom = nullptr; const CoordIdx *idxTo = nullptr; }; diff --git a/tests/src/analysis/testqgsgeometrysnapper.cpp b/tests/src/analysis/testqgsgeometrysnapper.cpp index ba0e19bd4972..8c788283eed2 100644 --- a/tests/src/analysis/testqgsgeometrysnapper.cpp +++ b/tests/src/analysis/testqgsgeometrysnapper.cpp @@ -378,6 +378,10 @@ void TestQgsGeometrySnapper::snapPointToLine() pointGeom = QgsGeometry::fromWkt( QStringLiteral( "Point(0.5 0.5)" ) ); result = snapper.snapGeometry( pointGeom, 1 ); QCOMPARE( result.asWkt(), QStringLiteral( "Point (0 0)" ) ); + + pointGeom = QgsGeometry::fromWkt( QStringLiteral( "Point(3 3)" ) ); + result = snapper.snapGeometry( pointGeom, 4 ); + QCOMPARE( result.asWkt(), QStringLiteral( "Point (3 0)" ) ); } void TestQgsGeometrySnapper::snapPointToLinePreferNearest() From bc957d68b004c078a163e315d292fdc548edccdd Mon Sep 17 00:00:00 2001 From: Mathieu Pellerin Date: Wed, 21 Feb 2024 09:50:01 +0700 Subject: [PATCH 2/2] Address review --- src/analysis/vector/qgsgeometrysnapper.cpp | 13 ++++++------- src/analysis/vector/qgsgeometrysnapper.h | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/analysis/vector/qgsgeometrysnapper.cpp b/src/analysis/vector/qgsgeometrysnapper.cpp index 82bfa8817e8c..3d6b9c34548c 100644 --- a/src/analysis/vector/qgsgeometrysnapper.cpp +++ b/src/analysis/vector/qgsgeometrysnapper.cpp @@ -99,11 +99,10 @@ bool QgsSnapIndex::SegmentSnapItem::getProjection( const QgsPoint &p, QgsPoint & return true; } -bool QgsSnapIndex::SegmentSnapItem::withinSqrDistance( const QgsPoint &p, const double tolerance ) +bool QgsSnapIndex::SegmentSnapItem::withinSquaredDistance( const QgsPoint &p, const double squaredDistance ) { double minDistX, minDistY; - const double distance = QgsGeometryUtilsBase::sqrDistToLine( p.x(), p.y(), idxFrom->point().x(), idxFrom->point().y(), idxTo->point().x(), idxTo->point().y(), minDistX, minDistY, 4 * std::numeric_limits::epsilon() ); - return distance <= tolerance; + return QgsGeometryUtilsBase::sqrDistToLine( p.x(), p.y(), idxFrom->point().x(), idxFrom->point().y(), idxTo->point().x(), idxTo->point().y(), minDistX, minDistY, 4 * std::numeric_limits::epsilon() ) <= squaredDistance; } /////////////////////////////////////////////////////////////////////////////// @@ -250,7 +249,7 @@ QgsSnapIndex::SnapItem *QgsSnapIndex::getSnapItem( const QgsPoint &pos, const do QgsSnapIndex::SegmentSnapItem *snapSegment = nullptr; QgsSnapIndex::PointSnapItem *snapPoint = nullptr; - const double sqrTolerance = tolerance * tolerance; + const double squaredTolerance = tolerance * tolerance; const auto constItems = items; for ( QgsSnapIndex::SnapItem *item : constItems ) { @@ -265,7 +264,7 @@ QgsSnapIndex::SnapItem *QgsSnapIndex::getSnapItem( const QgsPoint &pos, const do } else if ( item->type == SnapSegment && !endPointOnly ) { - if ( !static_cast( item )->withinSqrDistance( pos, sqrTolerance ) ) + if ( !static_cast( item )->withinSquaredDistance( pos, squaredTolerance ) ) continue; QgsPoint pProj; @@ -280,8 +279,8 @@ QgsSnapIndex::SnapItem *QgsSnapIndex::getSnapItem( const QgsPoint &pos, const do } } } - snapPoint = minDistPoint < sqrTolerance ? snapPoint : nullptr; - snapSegment = minDistSegment < sqrTolerance ? snapSegment : nullptr; + snapPoint = minDistPoint < squaredTolerance ? snapPoint : nullptr; + snapSegment = minDistSegment < squaredTolerance ? snapSegment : nullptr; if ( pSnapPoint ) *pSnapPoint = snapPoint; if ( pSnapSegment ) *pSnapSegment = snapSegment; return minDistPoint < minDistSegment ? static_cast( snapPoint ) : static_cast( snapSegment ); diff --git a/src/analysis/vector/qgsgeometrysnapper.h b/src/analysis/vector/qgsgeometrysnapper.h index 5f689f6b3bc3..235e39baf9df 100644 --- a/src/analysis/vector/qgsgeometrysnapper.h +++ b/src/analysis/vector/qgsgeometrysnapper.h @@ -209,7 +209,7 @@ class QgsSnapIndex QgsPoint getSnapPoint( const QgsPoint &p ) const override; bool getIntersection( const QgsPoint &p1, const QgsPoint &p2, QgsPoint &inter ) const; bool getProjection( const QgsPoint &p, QgsPoint &pProj ) const; - bool withinSqrDistance( const QgsPoint &p, const double distance ); + bool withinSquaredDistance( const QgsPoint &p, const double squaredDistance ); const CoordIdx *idxFrom = nullptr; const CoordIdx *idxTo = nullptr; };