Skip to content
Permalink
Browse files

Use an exact test when checking if a curve is closed

Otherwise we get cases where calling isClosed() on a curve with small
(<1E-08) differences between the first and last vertex incorrectly
returns true.

This is an issue when converting rings to GEOS, as it rejects them
as being unclosed, yet calling QgsCurve::close on them has no effect...

This is the underlying root cause of #36346. In this particular case
the calculation of the bounds of the marker symbol (used for the
label obstacle calculation) results in a very small rectangle (since
it's in map (geographic) units, and the map is very zoomed in) -- so
the 1E-08 tolerance here is completely unsuitable and calling close()
on the marker bounds had no effect, ultimately resulting in an unclosed
ring being passed to GEOS.

Fixes #36346
  • Loading branch information
nyalldawson committed Jun 2, 2020
1 parent df41d78 commit 3aa5e56d3c3dbb61f2a28c4874593aa401957ed3
Showing with 16 additions and 3 deletions.
  1. +3 −3 src/core/geometry/qgscurve.cpp
  2. +13 −0 tests/src/core/testqgsgeometry.cpp
@@ -46,10 +46,10 @@ bool QgsCurve::isClosed() const
QgsPoint start = startPoint();
QgsPoint end = endPoint();

bool closed = qgsDoubleNear( start.x(), end.x(), 1E-8 ) &&
qgsDoubleNear( start.y(), end.y(), 1E-8 );
bool closed = qgsDoubleNear( start.x(), end.x() ) &&
qgsDoubleNear( start.y(), end.y() );
if ( is3D() && closed )
closed &= qgsDoubleNear( start.z(), end.z(), 1E-8 ) || ( std::isnan( start.z() ) && std::isnan( end.z() ) );
closed &= qgsDoubleNear( start.z(), end.z() ) || ( std::isnan( start.z() ) && std::isnan( end.z() ) );
return closed;
}

@@ -3597,6 +3597,19 @@ void TestQgsGeometry::lineString()
QVERIFY( l11.isClosed() );
QCOMPARE( l11.numPoints(), 5 );
QCOMPARE( l11.pointN( 4 ), QgsPoint( 1, 2 ) );

// tiny differences
l11.setPoints( QgsPointSequence() << QgsPoint( 0.000000000000001, 0.000000000000002 )
<< QgsPoint( 0.000000000000011, 0.000000000000002 )
<< QgsPoint( 0.000000000000011, 0.000000000000022 )
<< QgsPoint( 0.000000000000001, 0.000000000000022 ) );
QVERIFY( !l11.isClosed() );
l11.close();
QVERIFY( l11.isClosed() );
QCOMPARE( l11.numPoints(), 5 );
QGSCOMPARENEAR( l11.pointN( 4 ).x(), 0.000000000000001, 0.00000000000000001 );
QGSCOMPARENEAR( l11.pointN( 4 ).y(), 0.000000000000002, 0.00000000000000001 );

//test that m values aren't considered when testing for closedness
l11.setPoints( QgsPointSequence() << QgsPoint( QgsWkbTypes::PointM, 1, 2, 0, 3 )
<< QgsPoint( QgsWkbTypes::PointM, 11, 2, 0, 4 )

0 comments on commit 3aa5e56

Please sign in to comment.
You can’t perform that action at this time.