Fix closest segment for points (#3383)

wonder-sk · 3nids · commit c4dcfbfb9f8f · 2016-08-12T10:07:15.000+02:00
* Make closestSegment() behave as expected by QgsGeometry::closestSegmentWithContext

This fixes a bug that QgsPointLocator::nearestEdge() would return valid match even for point layers.

* Shortcut if using QgsPointLocator with a wrong layer type
diff --git a/python/core/geometry/qgsabstractgeometryv2.sip b/python/core/geometry/qgsabstractgeometryv2.sip
@@ -261,7 +261,7 @@ class QgsAbstractGeometryV2
      * @param leftOf returns whether the point lies on the left side of the nearest segment (true if point is to left of segment,
      * false if point is to right of segment)
      * @param epsilon epsilon for segment snapping
-     * @returns squared distance to closest segment
+     * @returns squared distance to closest segment or negative value on error
      */
     virtual double closestSegment( const QgsPointV2& pt, QgsPointV2& segmentPt, QgsVertexId& vertexAfter, bool* leftOf, double epsilon ) const = 0;
 
diff --git a/src/core/geometry/qgsabstractgeometryv2.h b/src/core/geometry/qgsabstractgeometryv2.h
@@ -245,7 +245,7 @@ class CORE_EXPORT QgsAbstractGeometryV2
      * @param leftOf returns whether the point lies on the left side of the nearest segment (true if point is to left of segment,
      * false if point is to right of segment)
      * @param epsilon epsilon for segment snapping
-     * @returns squared distance to closest segment
+     * @returns squared distance to closest segment or negative value on error
      */
     virtual double closestSegment( const QgsPointV2& pt, QgsPointV2& segmentPt, QgsVertexId& vertexAfter, bool* leftOf, double epsilon ) const = 0;
 
diff --git a/src/core/geometry/qgspointv2.cpp b/src/core/geometry/qgspointv2.cpp
@@ -311,11 +311,12 @@ bool QgsPointV2::moveVertex( QgsVertexId position, const QgsPointV2& newPos )
 
 double QgsPointV2::closestSegment( const QgsPointV2& pt, QgsPointV2& segmentPt,  QgsVertexId& vertexAfter, bool* leftOf, double epsilon ) const
 {
+  Q_UNUSED( pt );
+  Q_UNUSED( segmentPt );
+  Q_UNUSED( vertexAfter );
   Q_UNUSED( leftOf );
   Q_UNUSED( epsilon );
-  segmentPt = *this;
-  vertexAfter = QgsVertexId( 0, 0, 0 );
-  return QgsGeometryUtils::sqrDistance2D( *this, pt );
+  return -1;  // no segments - return error
 }
 
 bool QgsPointV2::nextVertex( QgsVertexId& id, QgsPointV2& vertex ) const
diff --git a/src/core/qgspointlocator.cpp b/src/core/qgspointlocator.cpp
@@ -858,6 +858,10 @@ QgsPointLocator::Match QgsPointLocator::nearestEdge( const QgsPoint& point, doub
       return Match();
   }
 
+  QgsWkbTypes::GeometryType geomType = mLayer->geometryType();
+  if ( geomType == QgsWkbTypes::PointGeometry )
+    return Match();
+
   Match m;
   QgsPointLocator_VisitorNearestEdge visitor( this, m, point, filter );
   QgsRectangle rect( point.x() - tolerance, point.y() - tolerance, point.x() + tolerance, point.y() + tolerance );
@@ -876,6 +880,10 @@ QgsPointLocator::MatchList QgsPointLocator::edgesInRect( const QgsRectangle& rec
       return MatchList();
   }
 
+  QgsWkbTypes::GeometryType geomType = mLayer->geometryType();
+  if ( geomType == QgsWkbTypes::PointGeometry )
+    return MatchList();
+
   MatchList lst;
   QgsPointLocator_VisitorEdgesInRect visitor( this, lst, rect, filter );
   mRTree->intersectsWithQuery( rect2region( rect ), visitor );
@@ -899,6 +907,10 @@ QgsPointLocator::MatchList QgsPointLocator::pointInPolygon( const QgsPoint& poin
       return MatchList();
   }
 
+  QgsWkbTypes::GeometryType geomType = mLayer->geometryType();
+  if ( geomType == QgsWkbTypes::PointGeometry || geomType == QgsWkbTypes::LineGeometry )
+    return MatchList();
+
   MatchList lst;
   QgsPointLocator_VisitorArea visitor( this, point, lst );
   mRTree->intersectsWithQuery( point2point( point ), visitor );
diff --git a/tests/src/core/testqgsgeometry.cpp b/tests/src/core/testqgsgeometry.cpp
@@ -675,12 +675,11 @@ void TestQgsGeometry::pointV2()
   p20.deleteVertex( QgsVertexId( 0, 0, 0 ) );
   QCOMPARE( p20, QgsPointV2( 2.0, 3.0 ) );
 
-  //closestSegment
+  // closestSegment
   QgsPointV2 closest;
   QgsVertexId after;
-  QCOMPARE( p20.closestSegment( QgsPointV2( 4.0, 6.0 ), closest, after, 0, 0 ), 13.0 );
-  QCOMPARE( closest, p20 );
-  QCOMPARE( after, QgsVertexId( 0, 0, 0 ) );
+  // return error - points have no segments
+  QVERIFY( p20.closestSegment( QgsPointV2( 4.0, 6.0 ), closest, after, 0, 0 ) < 0 );
 
   //nextVertex
   QgsPointV2 p21( 3.0, 4.0 );
@@ -3014,6 +3013,12 @@ void TestQgsGeometry::multiPoint()
   boundaryMP.addGeometry( new QgsPointV2( 0, 0 ) );
   boundaryMP.addGeometry( new QgsPointV2( 1, 1 ) );
   QVERIFY( !boundaryMP.boundary() );
+
+  // closestSegment
+  QgsPointV2 closest;
+  QgsVertexId after;
+  // return error - points have no segments
+  QVERIFY( boundaryMP.closestSegment( QgsPointV2( 0.5, 0.5 ), closest, after, 0, 0 ) < 0 );
 }
 
 void TestQgsGeometry::multiLineString()

Original file line number	Diff line number	Diff line change
`@@ -311,11 +311,12 @@ bool QgsPointV2::moveVertex( QgsVertexId position, const QgsPointV2& newPos )`
`311`	`311`
`312`	`312`	`double QgsPointV2::closestSegment( const QgsPointV2& pt, QgsPointV2& segmentPt, QgsVertexId& vertexAfter, bool* leftOf, double epsilon ) const`
`313`	`313`	`{`
	`314`	`+ Q_UNUSED( pt );`
	`315`	`+ Q_UNUSED( segmentPt );`
	`316`	`+ Q_UNUSED( vertexAfter );`
`314`	`317`	`Q_UNUSED( leftOf );`
`315`	`318`	`Q_UNUSED( epsilon );`
`316`		`- segmentPt = *this;`
`317`		`- vertexAfter = QgsVertexId( 0, 0, 0 );`
`318`		`- return QgsGeometryUtils::sqrDistance2D( *this, pt );`
	`319`	`+ return -1; // no segments - return error`
`319`	`320`	`}`
`320`	`321`
`321`	`322`	`bool QgsPointV2::nextVertex( QgsVertexId& id, QgsPointV2& vertex ) const`