Skip to content

Commit c093d51

Browse files
wonder-skwonder-sk
committedOct 7, 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
·
ltr-2_18archive/master_2
1 parent 706431e commit c093d51

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
@@ -179,7 +179,7 @@ class QgsGeometry
179179
* @param afterVertex will be set to the vertex index of the next vertex after the closest one. Will be set to -1 if
180180
* not present.
181181
* @param sqrDist will be set to the square distance between the closest vertex and the specified point
182-
* @returns closest point in geometry
182+
* @returns closest point in geometry. If not found (empty geometry), returns null point nad sqrDist is negative.
183183
*/
184184
//TODO QGIS 3.0 - rename beforeVertex to previousVertex, afterVertex to nextVertex
185185
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
@@ -272,12 +272,11 @@ void QgsMapToolNodeTool::canvasPressEvent( QgsMapMouseEvent* e )
272272

273273
// get geometry and find if snapping is near it
274274
int atVertex, beforeVertex, afterVertex;
275-
double dist;
276-
QgsPoint closestLayerVertex = mSelectedFeature->geometry()->closestVertex( layerCoordPoint, atVertex, beforeVertex, afterVertex, dist );
277-
dist = sqrt( dist );
275+
double sqrDist; // will be negative on error
276+
QgsPoint closestLayerVertex = mSelectedFeature->geometry()->closestVertex( layerCoordPoint, atVertex, beforeVertex, afterVertex, sqrDist );
278277

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

‎src/core/geometry/qgscircularstringv2.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,9 @@ double QgsCircularStringV2::closestSegment( const QgsPointV2& pt, QgsPointV2& se
845845
}
846846
}
847847

848+
if ( minDist == std::numeric_limits<double>::max() )
849+
return -1; // error: no segments
850+
848851
segmentPt = minDistSegmentPoint;
849852
vertexAfter = minDistVertexAfter;
850853
vertexAfter.part = 0;

‎src/core/geometry/qgscurvepolygonv2.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ double QgsCurvePolygonV2::closestSegment( const QgsPointV2& pt, QgsPointV2& segm
636636
{
637637
if ( !mExteriorRing )
638638
{
639-
return 0.0;
639+
return -1;
640640
}
641641
QList<QgsCurveV2*> segmentList;
642642
segmentList.append( mExteriorRing );

‎src/core/geometry/qgsgeometry.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ QgsPoint QgsGeometry::closestVertex( const QgsPoint& point, int& atVertex, int&
347347
{
348348
if ( !d->geometry )
349349
{
350+
sqrDist = -1;
350351
return QgsPoint( 0, 0 );
351352
}
352353

@@ -587,12 +588,14 @@ double QgsGeometry::closestVertexWithContext( const QgsPoint& point, int& atVert
587588
{
588589
if ( !d->geometry )
589590
{
590-
return 0.0;
591+
return -1;
591592
}
592593

593594
QgsVertexId vId;
594595
QgsPointV2 pt( point.x(), point.y() );
595596
QgsPointV2 closestPoint = QgsGeometryUtils::closestVertex( *( d->geometry ), pt, vId );
597+
if ( !vId.isValid() )
598+
return -1;
596599
atVertex = vertexNrFromVertexId( vId );
597600
return QgsGeometryUtils::sqrDistance2D( closestPoint, pt );
598601
}
@@ -606,14 +609,16 @@ double QgsGeometry::closestSegmentWithContext(
606609
{
607610
if ( !d->geometry )
608611
{
609-
return 0;
612+
return -1;
610613
}
611614

612615
QgsPointV2 segmentPt;
613616
QgsVertexId vertexAfter;
614617
bool leftOfBool;
615618

616619
double sqrDist = d->geometry->closestSegment( QgsPointV2( point.x(), point.y() ), segmentPt, vertexAfter, &leftOfBool, epsilon );
620+
if ( sqrDist < 0 )
621+
return -1;
617622

618623
minDistPoint.setX( segmentPt.x() );
619624
minDistPoint.setY( segmentPt.y() );

‎src/core/geometry/qgsgeometry.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ class CORE_EXPORT QgsGeometry
221221
* @param afterVertex will be set to the vertex index of the next vertex after the closest one. Will be set to -1 if
222222
* not present.
223223
* @param sqrDist will be set to the square distance between the closest vertex and the specified point
224-
* @returns closest point in geometry
224+
* @returns closest point in geometry. If not found (empty geometry), returns null point nad sqrDist is negative.
225225
*/
226226
//TODO QGIS 3.0 - rename beforeVertex to previousVertex, afterVertex to nextVertex
227227
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 QgsAbstractGeometryV2& geom, c
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<QgsLineStringV2*> extractLineStrings( const QgsAbstractGeometryV2* 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 QgsAbstractGeometryV2& geom, const QgsPointV2& pt, QgsVertexId& id );
4344

@@ -250,7 +251,7 @@ class CORE_EXPORT QgsGeometryUtils
250251
for ( int i = 0; i < container.size(); ++i )
251252
{
252253
sqrDist = container.at( i )->closestSegment( pt, segmentPt, vertexAfter, leftOf, epsilon );
253-
if ( sqrDist < minDist )
254+
if ( sqrDist >= 0 && sqrDist < minDist )
254255
{
255256
minDist = sqrDist;
256257
minDistSegmentX = segmentPt.x();
@@ -280,6 +281,9 @@ class CORE_EXPORT QgsGeometryUtils
280281
}
281282
}
282283

284+
if ( minDist == std::numeric_limits<double>::max() )
285+
return -1; // error: no segments
286+
283287
segmentPt.setX( minDistSegmentX );
284288
segmentPt.setY( minDistSegmentY );
285289
vertexAfter = minDistVertexAfter;

‎src/core/geometry/qgslinestringv2.cpp

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -747,16 +747,10 @@ double QgsLineStringV2::closestSegment( const QgsPointV2& pt, QgsPointV2& segmen
747747
double segmentPtX, segmentPtY;
748748

749749
int size = mX.size();
750-
if ( size == 0 )
750+
if ( size == 0 || size == 1 )
751751
{
752752
vertexAfter = QgsVertexId( 0, 0, 0 );
753-
return sqrDist;
754-
}
755-
else if ( size == 1 )
756-
{
757-
segmentPt = pointN( 0 );
758-
vertexAfter = QgsVertexId( 0, 0, 1 );
759-
return QgsGeometryUtils::sqrDistance2D( pt, segmentPt );
753+
return -1;
760754
}
761755
for ( int i = 1; i < size; ++i )
762756
{

‎src/core/qgspointlocator.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ class QgsPointLocator_VisitorNearestVertex : public IVisitor
104104
int vertexIndex, beforeVertex, afterVertex;
105105
double sqrDist;
106106
QgsPoint pt = geom->closestVertex( mSrcPoint, vertexIndex, beforeVertex, afterVertex, sqrDist );
107+
if ( sqrDist < 0 )
108+
return; // probably empty geometry
107109

108110
QgsPointLocator::Match m( QgsPointLocator::Vertex, mLocator->mLayer, id, sqrt( sqrDist ), pt, vertexIndex );
109111
// 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
@@ -2038,11 +2038,10 @@ void TestQgsGeometry::lineStringV2()
20382038
//closest segment
20392039
QgsLineStringV2 l35;
20402040
bool leftOf = false;
2041+
p = QgsPointV2(); // reset all coords to zero
20412042
( void )l35.closestSegment( QgsPointV2( 1, 2 ), p, v, 0, 0 ); //empty line, just want no crash
20422043
l35.setPoints( QgsPointSequenceV2() << QgsPointV2( 5, 10 ) );
2043-
QVERIFY( qgsDoubleNear( l35.closestSegment( QgsPointV2( 5, 10 ), p, v, 0, 0 ), 0 ) );
2044-
QCOMPARE( p, QgsPointV2( 5, 10 ) );
2045-
QCOMPARE( v, QgsVertexId( 0, 0, 1 ) );
2044+
QVERIFY( l35.closestSegment( QgsPointV2( 5, 10 ), p, v, 0, 0 ) < 0 );
20462045
l35.setPoints( QgsPointSequenceV2() << QgsPointV2( 5, 10 ) << QgsPointV2( 10, 10 ) );
20472046
QVERIFY( qgsDoubleNear( l35.closestSegment( QgsPointV2( 4, 11 ), p, v, &leftOf, 0 ), 2.0 ) );
20482047
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 "qgspolygonv2.h"
2627

2728

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

264307
QTEST_MAIN( TestQgsPointLocator )

0 commit comments

Comments
 (0)
Please sign in to comment.