Skip to content

Commit 24fbe1a

Browse files
wonder-skwonder-sk
committedOct 31, 2016
Fix crash in node tool after deleting the whole geometry (fixes #15659)
Made sure that both closestVertex() and closestSegment() return negative distance on error (e.g. with null or emtpy geometry). Also fixes snapping when dealing with layers with null/invalid geometries (cherry picked from commit c093d51)
1 parent dd4b34e commit 24fbe1a

File tree

12 files changed

+72
-22
lines changed

12 files changed

+72
-22
lines changed
 

‎python/core/geometry/qgsgeometry.sip

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ class QgsGeometry
181181
* @param afterVertex will be set to the vertex index of the next vertex after the closest one. Will be set to -1 if
182182
* not present.
183183
* @param sqrDist will be set to the square distance between the closest vertex and the specified point
184-
* @returns closest point in geometry
184+
* @returns closest point in geometry. If not found (empty geometry), returns null point nad sqrDist is negative.
185185
*/
186186
//TODO QGIS 3.0 - rename beforeVertex to previousVertex, afterVertex to nextVertex
187187
QgsPoint closestVertex( const QgsPoint& point, int& atVertex /Out/, int& beforeVertex /Out/, int& afterVertex /Out/, double& sqrDist /Out/ ) const;

‎src/app/nodetool/qgsmaptoolnodetool.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,12 +274,11 @@ void QgsMapToolNodeTool::canvasPressEvent( QgsMapMouseEvent* e )
274274

275275
// get geometry and find if snapping is near it
276276
int atVertex, beforeVertex, afterVertex;
277-
double dist;
278-
QgsPoint closestLayerVertex = mSelectedFeature->geometry()->closestVertex( layerCoordPoint, atVertex, beforeVertex, afterVertex, dist );
279-
dist = sqrt( dist );
277+
double sqrDist; // will be negative on error
278+
QgsPoint closestLayerVertex = mSelectedFeature->geometry()->closestVertex( layerCoordPoint, atVertex, beforeVertex, afterVertex, sqrDist );
280279

281280
mSnapper.snapToCurrentLayer( e->pos(), snapResults, QgsSnapper::SnapToVertex, tol );
282-
if ( dist <= tol )
281+
if ( sqrDist >= 0 && sqrt( sqrDist ) <= tol )
283282
{
284283
// some vertex selected
285284
mMoving = true;

‎src/core/geometry/qgscircularstring.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,9 @@ double QgsCircularString::closestSegment( const QgsPointV2& pt, QgsPointV2& segm
861861
}
862862
}
863863

864+
if ( minDist == std::numeric_limits<double>::max() )
865+
return -1; // error: no segments
866+
864867
segmentPt = minDistSegmentPoint;
865868
vertexAfter = minDistVertexAfter;
866869
vertexAfter.part = 0;

‎src/core/geometry/qgscurvepolygon.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ double QgsCurvePolygon::closestSegment( const QgsPointV2& pt, QgsPointV2& segmen
657657
{
658658
if ( !mExteriorRing )
659659
{
660-
return 0.0;
660+
return -1;
661661
}
662662
QList<QgsCurve*> segmentList;
663663
segmentList.append( mExteriorRing );

‎src/core/geometry/qgsgeometry.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,7 @@ QgsPoint QgsGeometry::closestVertex( const QgsPoint& point, int& atVertex, int&
369369
{
370370
if ( !d->geometry )
371371
{
372+
sqrDist = -1;
372373
return QgsPoint( 0, 0 );
373374
}
374375

@@ -609,12 +610,14 @@ double QgsGeometry::closestVertexWithContext( const QgsPoint& point, int& atVert
609610
{
610611
if ( !d->geometry )
611612
{
612-
return 0.0;
613+
return -1;
613614
}
614615

615616
QgsVertexId vId;
616617
QgsPointV2 pt( point.x(), point.y() );
617618
QgsPointV2 closestPoint = QgsGeometryUtils::closestVertex( *( d->geometry ), pt, vId );
619+
if ( !vId.isValid() )
620+
return -1;
618621
atVertex = vertexNrFromVertexId( vId );
619622
return QgsGeometryUtils::sqrDistance2D( closestPoint, pt );
620623
}
@@ -628,14 +631,16 @@ double QgsGeometry::closestSegmentWithContext(
628631
{
629632
if ( !d->geometry )
630633
{
631-
return 0;
634+
return -1;
632635
}
633636

634637
QgsPointV2 segmentPt;
635638
QgsVertexId vertexAfter;
636639
bool leftOfBool;
637640

638641
double sqrDist = d->geometry->closestSegment( QgsPointV2( point.x(), point.y() ), segmentPt, vertexAfter, &leftOfBool, epsilon );
642+
if ( sqrDist < 0 )
643+
return -1;
639644

640645
minDistPoint.setX( segmentPt.x() );
641646
minDistPoint.setY( segmentPt.y() );

‎src/core/geometry/qgsgeometry.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ class CORE_EXPORT QgsGeometry
225225
* @param afterVertex will be set to the vertex index of the next vertex after the closest one. Will be set to -1 if
226226
* not present.
227227
* @param sqrDist will be set to the square distance between the closest vertex and the specified point
228-
* @returns closest point in geometry
228+
* @returns closest point in geometry. If not found (empty geometry), returns null point nad sqrDist is negative.
229229
*/
230230
//TODO QGIS 3.0 - rename beforeVertex to previousVertex, afterVertex to nextVertex
231231
QgsPoint closestVertex( const QgsPoint& point, int& atVertex, int& beforeVertex, int& afterVertex, double& sqrDist ) const;

‎src/core/geometry/qgsgeometryutils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ QgsPointV2 QgsGeometryUtils::closestVertex( const QgsAbstractGeometry& geom, con
6565
double minDist = std::numeric_limits<double>::max();
6666
double currentDist = 0;
6767
QgsPointV2 minDistPoint;
68+
id = QgsVertexId(); // set as invalid
6869

6970
QgsVertexId vertexId;
7071
QgsPointV2 vertex;

‎src/core/geometry/qgsgeometryutils.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ class CORE_EXPORT QgsGeometryUtils
3737
*/
3838
static QList<QgsLineString*> extractLineStrings( const QgsAbstractGeometry* geom );
3939

40-
/** Returns the closest vertex to a geometry for a specified point
40+
/** Returns the closest vertex to a geometry for a specified point.
41+
* On error null point will be returned and "id" argument will be invalid.
4142
*/
4243
static QgsPointV2 closestVertex( const QgsAbstractGeometry& geom, const QgsPointV2& pt, QgsVertexId& id );
4344

@@ -263,7 +264,7 @@ class CORE_EXPORT QgsGeometryUtils
263264
for ( int i = 0; i < container.size(); ++i )
264265
{
265266
sqrDist = container.at( i )->closestSegment( pt, segmentPt, vertexAfter, leftOf, epsilon );
266-
if ( sqrDist < minDist )
267+
if ( sqrDist >= 0 && sqrDist < minDist )
267268
{
268269
minDist = sqrDist;
269270
minDistSegmentX = segmentPt.x();
@@ -293,6 +294,9 @@ class CORE_EXPORT QgsGeometryUtils
293294
}
294295
}
295296

297+
if ( minDist == std::numeric_limits<double>::max() )
298+
return -1; // error: no segments
299+
296300
segmentPt.setX( minDistSegmentX );
297301
segmentPt.setY( minDistSegmentY );
298302
vertexAfter = minDistVertexAfter;

‎src/core/geometry/qgslinestring.cpp

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -763,16 +763,10 @@ double QgsLineString::closestSegment( const QgsPointV2& pt, QgsPointV2& segmentP
763763
double segmentPtX, segmentPtY;
764764

765765
int size = mX.size();
766-
if ( size == 0 )
766+
if ( size == 0 || size == 1 )
767767
{
768768
vertexAfter = QgsVertexId( 0, 0, 0 );
769-
return sqrDist;
770-
}
771-
else if ( size == 1 )
772-
{
773-
segmentPt = pointN( 0 );
774-
vertexAfter = QgsVertexId( 0, 0, 1 );
775-
return QgsGeometryUtils::sqrDistance2D( pt, segmentPt );
769+
return -1;
776770
}
777771
for ( int i = 1; i < size; ++i )
778772
{

‎src/core/qgspointlocator.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ class QgsPointLocator_VisitorNearestVertex : public IVisitor
106106
int vertexIndex, beforeVertex, afterVertex;
107107
double sqrDist;
108108
QgsPoint pt = geom->closestVertex( mSrcPoint, vertexIndex, beforeVertex, afterVertex, sqrDist );
109+
if ( sqrDist < 0 )
110+
return; // probably empty geometry
109111

110112
QgsPointLocator::Match m( QgsPointLocator::Vertex, mLocator->mLayer, id, sqrt( sqrDist ), pt, vertexIndex );
111113
// in range queries the filter may reject some matches

‎tests/src/core/testqgsgeometry.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2047,11 +2047,10 @@ void TestQgsGeometry::lineString()
20472047
//closest segment
20482048
QgsLineString l35;
20492049
bool leftOf = false;
2050+
p = QgsPointV2(); // reset all coords to zero
20502051
( void )l35.closestSegment( QgsPointV2( 1, 2 ), p, v, 0, 0 ); //empty line, just want no crash
20512052
l35.setPoints( QgsPointSequence() << QgsPointV2( 5, 10 ) );
2052-
QVERIFY( qgsDoubleNear( l35.closestSegment( QgsPointV2( 5, 10 ), p, v, 0, 0 ), 0 ) );
2053-
QCOMPARE( p, QgsPointV2( 5, 10 ) );
2054-
QCOMPARE( v, QgsVertexId( 0, 0, 1 ) );
2053+
QVERIFY( l35.closestSegment( QgsPointV2( 5, 10 ), p, v, 0, 0 ) < 0 );
20552054
l35.setPoints( QgsPointSequence() << QgsPointV2( 5, 10 ) << QgsPointV2( 10, 10 ) );
20562055
QVERIFY( qgsDoubleNear( l35.closestSegment( QgsPointV2( 4, 11 ), p, v, &leftOf, 0 ), 2.0 ) );
20572056
QCOMPARE( p, QgsPointV2( 5, 10 ) );

‎tests/src/core/testqgspointlocator.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "qgsgeometry.h"
2424
#include "qgsmaplayerregistry.h"
2525
#include "qgspointlocator.h"
26+
#include "qgspolygon.h"
2627

2728

2829
struct FilterExcludePoint : public QgsPointLocator::MatchFilter
@@ -261,6 +262,48 @@ class TestQgsPointLocator : public QObject
261262
QVERIFY( m2.isValid() );
262263
QCOMPARE( m2.point(), QgsPoint( 1, 1 ) );
263264
}
265+
266+
void testNullGeometries()
267+
{
268+
QgsVectorLayer* vlNullGeom = new QgsVectorLayer( "Polygon", "x", "memory" );
269+
QgsFeature ff( 0 );
270+
ff.setGeometry( QgsGeometry() );
271+
QgsFeatureList flist;
272+
flist << ff;
273+
vlNullGeom->dataProvider()->addFeatures( flist );
274+
275+
QgsPointLocator loc( vlNullGeom, 0, nullptr );
276+
277+
QgsPointLocator::Match m1 = loc.nearestVertex( QgsPoint( 2, 2 ), std::numeric_limits<double>::max() );
278+
QVERIFY( !m1.isValid() );
279+
280+
QgsPointLocator::Match m2 = loc.nearestEdge( QgsPoint( 2, 2 ), std::numeric_limits<double>::max() );
281+
QVERIFY( !m2.isValid() );
282+
283+
delete vlNullGeom;
284+
}
285+
286+
void testEmptyGeometries()
287+
{
288+
QgsVectorLayer* vlEmptyGeom = new QgsVectorLayer( "Polygon", "x", "memory" );
289+
QgsFeature ff( 0 );
290+
QgsGeometry g;
291+
g.setGeometry( new QgsPolygonV2() );
292+
ff.setGeometry( g );
293+
QgsFeatureList flist;
294+
flist << ff;
295+
vlEmptyGeom->dataProvider()->addFeatures( flist );
296+
297+
QgsPointLocator loc( vlEmptyGeom, 0, nullptr );
298+
299+
QgsPointLocator::Match m1 = loc.nearestVertex( QgsPoint( 2, 2 ), std::numeric_limits<double>::max() );
300+
QVERIFY( !m1.isValid() );
301+
302+
QgsPointLocator::Match m2 = loc.nearestEdge( QgsPoint( 2, 2 ), std::numeric_limits<double>::max() );
303+
QVERIFY( !m2.isValid() );
304+
305+
delete vlEmptyGeom;
306+
}
264307
};
265308

266309
QTEST_MAIN( TestQgsPointLocator )

0 commit comments

Comments
 (0)