Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Refine behavior of QgsGeometry equals tests
Before we had two checks - equals() and isGeosEqual() which
performed the exact same check (since equals() called the geos
equality test)

Since the geos equality test is a slow, topological test, which
considers two geometries equal if their component edges overlap,
but disregards ordering of vertices this is not always what we
want. There's also the issue that geos cannot consider m values
when testing the geometries, so two geometries with different
m values would be reported equal.

So, now calling QgsGeometry::equals performs a very fast, strict
equality test where geometries are only equal if the have exactly
the same vertices, type, and order.

And swap most code which was calling the slow geos test to instead
use the fast strict native test.
  • Loading branch information
nyalldawson committed Jan 4, 2018
1 parent 13aa521 commit 2d43dac
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 19 deletions.
2 changes: 2 additions & 0 deletions doc/api_break.dox
Expand Up @@ -1354,6 +1354,8 @@ maintains Z or M dimensions from the input points and is more efficient.
- exportToWkt() was renamed to asWkt()
- exportToGeoJSON() was renamed to asJson()
- closestSegmentWithContext() now returns an extra value, indicating whether the point is to the left of the geometry
- equals() now performs a fast, strict equality check, where geometries are considered equal only if they have the
exact same type, vertices and order. The slower topological test can be performed by calling isGeosEqual instead.


QgsGeometryAnalyzer {#qgis_api_break_3_0_QgsGeometryAnalyzer}
Expand Down
42 changes: 34 additions & 8 deletions python/core/geometry/qgsgeometry.sip
Expand Up @@ -247,13 +247,46 @@ return true for isEmpty().
bool isMultipart() const;
%Docstring
Returns true if WKB of the geometry is of WKBMulti* type
%End

bool equals( const QgsGeometry &geometry ) const;
%Docstring
Test if this geometry is exactly equal to another ``geometry``.

This is a strict equality check, where the underlying geometries must
have exactly the same type, component vertices and vertex order.

Calling this method is dramatically faster than the topological
equality test performed by isGeosEqual().

.. note::

Comparing two null geometries will return false.

.. versionadded:: 1.5

.. seealso:: :py:func:`isGeosEqual()`
%End

bool isGeosEqual( const QgsGeometry & ) const;
%Docstring
Compares the geometry with another geometry using GEOS
Compares the geometry with another geometry using GEOS.

This method performs a slow, topological check, where geometries
are considered equal if all of the their component edges overlap. E.g.
lines with the same vertex locations but opposite direction will be
considered equal by this method.

Consider using the much faster, stricter equality test performed
by equals() instead.

.. note::

Comparing two null geometries will return false.

.. versionadded:: 1.5

.. seealso:: :py:func:`equals()`
%End

bool isGeosValid() const;
Expand Down Expand Up @@ -749,13 +782,6 @@ Tests for if geometry is contained in another (uses GEOS)
%Docstring
Tests for if geometry is disjoint of another (uses GEOS)

.. versionadded:: 1.5
%End

bool equals( const QgsGeometry &geometry ) const;
%Docstring
Test for if geometry equals another (uses GEOS)

.. versionadded:: 1.5
%End

Expand Down
9 changes: 6 additions & 3 deletions src/core/geometry/qgsgeometry.cpp
Expand Up @@ -1147,9 +1147,12 @@ bool QgsGeometry::equals( const QgsGeometry &geometry ) const
return false;
}

QgsGeos geos( d->geometry.get() );
mLastError.clear();
return geos.isEqual( geometry.d->geometry.get(), &mLastError );
// fast check - are they shared copies of the same underlying geometry?
if ( d == geometry.d )
return true;

// slower check - actually test the geometries
return *d->geometry.get() == *geometry.d->geometry.get();
}

bool QgsGeometry::touches( const QgsGeometry &geometry ) const
Expand Down
36 changes: 29 additions & 7 deletions src/core/geometry/qgsgeometry.h
Expand Up @@ -307,8 +307,36 @@ class CORE_EXPORT QgsGeometry
bool isMultipart() const;

/**
* Compares the geometry with another geometry using GEOS
* Test if this geometry is exactly equal to another \a geometry.
*
* This is a strict equality check, where the underlying geometries must
* have exactly the same type, component vertices and vertex order.
*
* Calling this method is dramatically faster than the topological
* equality test performed by isGeosEqual().
*
* \note Comparing two null geometries will return false.
*
* \since QGIS 1.5
* \see isGeosEqual()
*/
bool equals( const QgsGeometry &geometry ) const;

/**
* Compares the geometry with another geometry using GEOS.
*
* This method performs a slow, topological check, where geometries
* are considered equal if all of the their component edges overlap. E.g.
* lines with the same vertex locations but opposite direction will be
* considered equal by this method.
*
* Consider using the much faster, stricter equality test performed
* by equals() instead.
*
* \note Comparing two null geometries will return false.
*
* \since QGIS 1.5
* \see equals()
*/
bool isGeosEqual( const QgsGeometry & ) const;

Expand Down Expand Up @@ -790,12 +818,6 @@ class CORE_EXPORT QgsGeometry
*/
bool disjoint( const QgsGeometry &geometry ) const;

/**
* Test for if geometry equals another (uses GEOS)
* \since QGIS 1.5
*/
bool equals( const QgsGeometry &geometry ) const;

/**
* Test for if geometry touch another (uses GEOS)
* \since QGIS 1.5
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsvectorlayer.cpp 100644 → 100755
Expand Up @@ -985,7 +985,7 @@ bool QgsVectorLayer::updateFeature( const QgsFeature &updatedFeature, bool skipD
bool hasChanged = false;
bool hasError = false;

if ( ( updatedFeature.hasGeometry() || currentFeature.hasGeometry() ) && !updatedFeature.geometry().isGeosEqual( currentFeature.geometry() ) )
if ( ( updatedFeature.hasGeometry() || currentFeature.hasGeometry() ) && !updatedFeature.geometry().equals( currentFeature.geometry() ) )
{
if ( changeGeometry( updatedFeature.id(), updatedFeature.geometry(), true ) )
{
Expand Down
39 changes: 39 additions & 0 deletions tests/src/core/testqgsgeometry.cpp
Expand Up @@ -80,8 +80,10 @@ class TestQgsGeometry : public QObject
void asVariant(); //test conversion to and from a QVariant
void isEmpty();
void operatorBool();
void equality();
void vertexIterator();


// geometry types
void point(); //test QgsPointV2
void lineString(); //test QgsLineString
Expand Down Expand Up @@ -431,6 +433,43 @@ void TestQgsGeometry::operatorBool()
QVERIFY( !geom );
}

void TestQgsGeometry::equality()
{
// null geometries
QVERIFY( !QgsGeometry().equals( QgsGeometry() ) );

// compare to null
QgsGeometry g1( qgis::make_unique< QgsPoint >( 1.0, 2.0 ) );
QVERIFY( !g1.equals( QgsGeometry() ) );
QVERIFY( !QgsGeometry().equals( g1 ) );

// compare implicitly shared copies
QgsGeometry g2( g1 );
QVERIFY( g2.equals( g1 ) );
QVERIFY( g1.equals( g2 ) );
QVERIFY( g1.equals( g1 ) );

// equal geometry, but different internal data
g2 = QgsGeometry::fromWkt( "Point( 1.0 2.0 )" );
QVERIFY( g2.equals( g1 ) );
QVERIFY( g1.equals( g2 ) );

// different dimensionality
g2 = QgsGeometry::fromWkt( "PointM( 1.0 2.0 3.0)" );
QVERIFY( !g2.equals( g1 ) );
QVERIFY( !g1.equals( g2 ) );

// different type
g2 = QgsGeometry::fromWkt( "LineString( 1.0 2.0, 3.0 4.0 )" );
QVERIFY( !g2.equals( g1 ) );
QVERIFY( !g1.equals( g2 ) );

// different direction
g1 = QgsGeometry::fromWkt( "LineString( 3.0 4.0, 1.0 2.0 )" );
QVERIFY( !g2.equals( g1 ) );
QVERIFY( !g1.equals( g2 ) );
}

void TestQgsGeometry::vertexIterator()
{
QgsGeometry geom;
Expand Down

0 comments on commit 2d43dac

Please sign in to comment.