Skip to content
Permalink
Browse files

Rely on upstream GEOS makevalid algorithm for GEOS 3.9+

No point having our own internal fork of this code now that it's
included (and more importantly, maintained) upstream!
  • Loading branch information
nyalldawson committed Apr 28, 2021
1 parent 1c11a91 commit f22c63ec77f565f18608c4133731b55fb4d71265
@@ -1958,7 +1958,9 @@ by calling :py:func:`~QgsGeometry.lastError` on the returned geometry.

.. note::

Ported from PostGIS ST_MakeValid() and it should return equivalent results.
For QGIS builds using GEOS library versions older than 3.8 this method calls
an internal fork of PostGIS' ST_MakeValid() function. For builds based on GEOS 3.8 or
later this method calls the GEOS MakeValid method directly.


.. versionadded:: 3.0
@@ -23,7 +23,13 @@ email : morb at ozemail dot com dot au
#include "qgsgeometry.h"
#include "qgsgeometryeditutils.h"
#include "qgsgeometryfactory.h"

#include <geos_c.h>

#if ( GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR<8 )
#include "qgsgeometrymakevalid.h"
#endif

#include "qgsgeometryutils.h"
#include "qgsinternalgeometryengine.h"
#include "qgsgeos.h"
@@ -2627,7 +2633,12 @@ QgsGeometry QgsGeometry::makeValid() const
return QgsGeometry();

mLastError.clear();
#if GEOS_VERSION_MAJOR>3 || ( GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR>=8 )
QgsGeos geos( d->geometry.get() );
std::unique_ptr< QgsAbstractGeometry > g( geos.makeValid( &mLastError ) );
#else
std::unique_ptr< QgsAbstractGeometry > g( _qgis_lwgeom_make_valid( d->geometry.get(), mLastError ) );
#endif

QgsGeometry result = QgsGeometry( std::move( g ) );
result.mLastError = mLastError;
@@ -2051,7 +2051,9 @@ class CORE_EXPORT QgsGeometry
*
* \returns new valid QgsGeometry or null geometry on error
*
* \note Ported from PostGIS ST_MakeValid() and it should return equivalent results.
* \note For QGIS builds using GEOS library versions older than 3.8 this method calls
* an internal fork of PostGIS' ST_MakeValid() function. For builds based on GEOS 3.8 or
* later this method calls the GEOS MakeValid method directly.
*
* \since QGIS 3.0
*/
@@ -18,6 +18,8 @@
//
// Ideally one day the implementation will go to GEOS library...

#if ( GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR<8 )

#include "qgsgeometry.h"
#include "qgsgeos.h"
#include "qgslogger.h"
@@ -968,3 +970,5 @@ std::unique_ptr< QgsAbstractGeometry > _qgis_lwgeom_make_valid( const QgsAbstrac

return lwgeom_out;
}

#endif
@@ -18,6 +18,8 @@

#define SIP_NO_FILE

#if ( GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR<8 )

#include <memory>

class QString;
@@ -26,4 +28,6 @@ class QgsAbstractGeometry;
//! Implementation of QgsGeometry::makeValid(). Not a public API.
std::unique_ptr< QgsAbstractGeometry > _qgis_lwgeom_make_valid( const QgsAbstractGeometry *lwgeom_in, QString &errorMessage );

#endif

#endif // QGSGEOMETRYMAKEVALID_H
@@ -160,6 +160,24 @@ QgsGeometry QgsGeos::geometryFromGeos( const geos::unique_ptr &geos )
return g;
}

#if GEOS_VERSION_MAJOR>3 || ( GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR>=8 )
std::unique_ptr<QgsAbstractGeometry> QgsGeos::makeValid( QString *errorMsg ) const
{
if ( !mGeos )
{
return nullptr;
}

geos::unique_ptr geos;
try
{
geos.reset( GEOSMakeValid_r( geosinit()->ctxt, mGeos.get() ) );
}
CATCH_GEOS_WITH_ERRMSG( nullptr )
return fromGeos( geos.get() );
}
#endif

geos::unique_ptr QgsGeos::asGeos( const QgsGeometry &geometry, double precision )
{
if ( geometry.isNull() )
@@ -122,6 +122,15 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine
*/
static QgsGeometry geometryFromGeos( const geos::unique_ptr &geos );

#if GEOS_VERSION_MAJOR>3 || ( GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR>=8 )

/**
* Repairs the geometry using GEOS make valid routine.
* \since QGIS 3.20
*/
std::unique_ptr< QgsAbstractGeometry > makeValid( QString *errorMsg = nullptr ) const;
#endif

/**
* Adds a new island polygon to a multipolygon feature
* \param geometry geometry to add part to
@@ -201,6 +201,7 @@ class TestQgsGeometry : public QObject
void directionNeutralSegmentation();
void poleOfInaccessibility();

void makeValid_data();
void makeValid();

void isSimple_data();
QGSCOMPARENEAR( distance, 10.0, 0.00001 );
}

void TestQgsGeometry::makeValid_data()
{
QTest::addColumn<QString>( "input" );
QTest::addColumn<QString>( "expected" );

QTest::newRow( "dimension collapse" ) << QStringLiteral( "LINESTRING(0 0)" ) << QStringLiteral( "" );
QTest::newRow( "unclosed ring" ) << QStringLiteral( "POLYGON((10 22,10 32,20 32,20 22))" ) << QStringLiteral( "Polygon ((10 22, 10 32, 20 32, 20 22, 10 22))" );
QTest::newRow( "butterfly polygon (self-intersecting ring)" ) << QStringLiteral( "POLYGON((0 0, 10 10, 10 0, 0 10, 0 0))" ) << QStringLiteral( "MultiPolygon (((0 0, 0 10, 5 5, 0 0)),((10 0, 5 5, 10 10, 10 0)))" );
QTest::newRow( "polygon with extra tail (a part of the ring does not form any area)" ) << QStringLiteral( "POLYGON((0 0, 1 0, 1 1, 0 1, 0 0, -1 0, 0 0))" ) << QStringLiteral( "GeometryCollection (Polygon ((0 1, 1 1, 1 0, 0 0, 0 1)),LineString (0 0, -1 0))" );
QTest::newRow( "collection with invalid geometries" ) << QStringLiteral( "GEOMETRYCOLLECTION(LINESTRING(0 0, 0 0), POLYGON((0 0, 10 10, 10 0, 0 10, 0 0)), LINESTRING(10 0, 10 10))" ) << QStringLiteral( "GeometryCollection (Point (0 0),MultiPolygon (((0 0, 0 10, 5 5, 0 0)),((10 0, 5 5, 10 10, 10 0))),LineString (10 0, 10 10))" );
QTest::newRow( "null line (#18077)" ) << QStringLiteral( "MultiLineString ((356984.0625 6300089, 356984.0625 6300089))" ) << QStringLiteral( "Point (356984.0625 6300089)" );
}

void TestQgsGeometry::makeValid()
{
typedef QPair<QString, QString> InputAndExpectedWktPair;
QList<InputAndExpectedWktPair> geoms;
// dimension collapse
geoms << qMakePair( QStringLiteral( "LINESTRING(0 0)" ),
QStringLiteral( "POINT(0 0)" ) );
// unclosed ring
geoms << qMakePair( QStringLiteral( "POLYGON((10 22,10 32,20 32,20 22))" ),
QStringLiteral( "POLYGON((10 22,10 32,20 32,20 22,10 22))" ) );
// butterfly polygon (self-intersecting ring)
geoms << qMakePair( QStringLiteral( "POLYGON((0 0, 10 10, 10 0, 0 10, 0 0))" ),
QStringLiteral( "MULTIPOLYGON(((5 5, 0 0, 0 10, 5 5)),((5 5, 10 10, 10 0, 5 5)))" ) );
// polygon with extra tail (a part of the ring does not form any area)
geoms << qMakePair( QStringLiteral( "POLYGON((0 0, 1 0, 1 1, 0 1, 0 0, -1 0, 0 0))" ),
QStringLiteral( "GEOMETRYCOLLECTION(POLYGON((0 0, 0 1, 1 1, 1 0, 0 0)), LINESTRING(0 0, -1 0))" ) );
// collection with invalid geometries
geoms << qMakePair( QStringLiteral( "GEOMETRYCOLLECTION(LINESTRING(0 0, 0 0), POLYGON((0 0, 10 10, 10 0, 0 10, 0 0)), LINESTRING(10 0, 10 10))" ),
QStringLiteral( "GEOMETRYCOLLECTION(POINT(0 0), MULTIPOLYGON(((5 5, 0 0, 0 10, 5 5)),((5 5, 10 10, 10 0, 5 5))), LINESTRING(10 0, 10 10))" ) );
// null line (#18077)
geoms << qMakePair( QStringLiteral( "MultiLineString ((356984.0625 6300089, 356984.0625 6300089))" ),
QStringLiteral( "MultiPoint ((356984.0625 6300089))" ) );

for ( const InputAndExpectedWktPair &pair : geoms )
{
QgsGeometry gInput = QgsGeometry::fromWkt( pair.first );
QgsGeometry gExp = QgsGeometry::fromWkt( pair.second );
QVERIFY( !gInput.isNull() );
QVERIFY( !gExp.isNull() );

QgsGeometry gValid = gInput.makeValid();
QVERIFY( gValid.isGeosValid() );
QVERIFY( gValid.isGeosEqual( gExp ) );
}
QFETCH( QString, input );
QFETCH( QString, expected );

QgsGeometry gInput = QgsGeometry::fromWkt( input );
QVERIFY( !gInput.isNull() );

QgsGeometry gValid = gInput.makeValid();
QCOMPARE( gValid.asWkt(), expected );
}

void TestQgsGeometry::isSimple_data()

0 comments on commit f22c63e

Please sign in to comment.