Skip to content
Permalink
Browse files

Cache validity check results

For non-point geometry subclasses (points are always valid!) we
now cache the results of a geometry validity check. Subsequent
checks utilise the cached result wherever possible.

Because QgsGeometry/QgsFeature objects are implicitly shared, this
means that we avoid a *lot* of duplicate validity checks as
features and geometries are thrown around during processing model
execution.
  • Loading branch information
nyalldawson committed Feb 28, 2019
1 parent 16c114b commit 75697d77ededc8e50263a87fad090791e20f6475
@@ -607,6 +607,20 @@ Converts the geometry to a specified type.
.. versionadded:: 2.14
%End

virtual bool isValid( QString &error /Out/, int flags = 0 ) const = 0;
%Docstring
Checks validity of the geometry, and returns ``True`` if the geometry is valid.

:param flags: indicates optional flags which control the type of validity checking performed
(corresponding to QgsGeometry.ValidityFlags).

:return: - ``True`` if geometry is valid
- error: will be set to the validity error message


.. versionadded:: 3.8
%End


QgsGeometryPartIterator parts();
%Docstring
@@ -172,6 +172,8 @@ Returns a geometry without curves. Caller takes ownership

virtual QgsRectangle boundingBox() const;

virtual bool isValid( QString &error /Out/, int flags = 0 ) const;


virtual double xAt( int index ) const = 0;
%Docstring
@@ -206,6 +206,8 @@ Returns a geometry without curves. Caller takes ownership

virtual QgsPoint vertexAt( QgsVertexId id ) const;

virtual bool isValid( QString &error /Out/, int flags = 0 ) const;


virtual bool addZValue( double zValue = 0 );

@@ -50,6 +50,8 @@ Multi point geometry collection.

virtual double segmentLength( QgsVertexId startVertex ) const;

virtual bool isValid( QString &error /Out/, int flags = 0 ) const;



virtual QgsMultiPoint *createEmptyWithSameType() const /Factory/;
@@ -371,6 +371,8 @@ M value is preserved.

virtual QgsAbstractGeometry *boundary() const /Factory/;

virtual bool isValid( QString &error /Out/, int flags = 0 ) const;


virtual bool insertVertex( QgsVertexId position, const QgsPoint &vertex );

@@ -25,14 +25,16 @@ Ownership is transferred to the caller.
%End

virtual QgsRectangle boundingBox() const;
%Docstring
Returns the minimal bounding box for the geometry
%End

virtual bool isValid( QString &error /Out/, int flags = 0 ) const;



protected:

virtual void clearCache() const;


};

/************************************************************************
@@ -577,6 +577,19 @@ class CORE_EXPORT QgsAbstractGeometry
*/
virtual bool convertTo( QgsWkbTypes::Type type );

/**
* Checks validity of the geometry, and returns TRUE if the geometry is valid.
*
* \param error will be set to the validity error message
* \param flags indicates optional flags which control the type of validity checking performed
* (corresponding to QgsGeometry::ValidityFlags).
*
* \returns TRUE if geometry is valid
*
* \since QGIS 3.8
*/
virtual bool isValid( QString &error SIP_OUT, int flags = 0 ) const = 0;

#ifndef SIP_RUN

/**
@@ -21,6 +21,7 @@
#include "qgslinestring.h"
#include "qgspoint.h"
#include "qgsmultipoint.h"
#include "qgsgeos.h"

bool QgsCurve::operator==( const QgsAbstractGeometry &other ) const
{
@@ -188,6 +189,25 @@ QgsRectangle QgsCurve::boundingBox() const
return mBoundingBox;
}

bool QgsCurve::isValid( QString &error, int flags ) const
{
if ( flags == 0 && mHasCachedValidity )
{
// use cached validity results
error = mValidityFailureReason;
return error.isEmpty();
}

QgsGeos geos( this );
bool res = geos.isValid( &error, flags & QgsGeometry::FlagAllowSelfTouchingHoles, nullptr );
if ( flags == 0 )
{
mValidityFailureReason = !res ? error : QString();
mHasCachedValidity = true;
}
return res;
}

QPolygonF QgsCurve::asQPolygonF() const
{
const int nb = numPoints();
@@ -224,6 +244,8 @@ QgsCurve::Orientation QgsCurve::orientation() const
void QgsCurve::clearCache() const
{
mBoundingBox = QgsRectangle();
mHasCachedValidity = false;
mValidityFailureReason.clear();
QgsAbstractGeometry::clearCache();
}

@@ -162,6 +162,7 @@ class CORE_EXPORT QgsCurve: public QgsAbstractGeometry
QgsCurve *toCurveType() const override SIP_FACTORY;

QgsRectangle boundingBox() const override;
bool isValid( QString &error SIP_OUT, int flags = 0 ) const override;

/**
* Returns the x-coordinate of the specified node in the line string.
@@ -290,6 +291,9 @@ class CORE_EXPORT QgsCurve: public QgsAbstractGeometry
private:

mutable QgsRectangle mBoundingBox;

mutable bool mHasCachedValidity = false;
mutable QString mValidityFailureReason;
};

#endif // QGSCURVE_H
@@ -2504,15 +2504,7 @@ bool QgsGeometry::isGeosValid( const QgsGeometry::ValidityFlags flags ) const
return false;
}

// avoid calling geos for trivial point geometries
if ( QgsWkbTypes::geometryType( d->geometry->wkbType() ) == QgsWkbTypes::PointGeometry )
{
return true;
}

QgsGeos geos( d->geometry.get() );
mLastError.clear();
return geos.isValid( &mLastError, flags & FlagAllowSelfTouchingHoles, nullptr );
return d->geometry->isValid( mLastError, static_cast< int >( flags ) );
}

bool QgsGeometry::isSimple() const
@@ -26,6 +26,7 @@ email : marco.hugentobler at sourcepole dot com
#include "qgspolygon.h"
#include "qgsmultipolygon.h"
#include "qgswkbptr.h"
#include "qgsgeos.h"
#include <memory>

QgsGeometryCollection::QgsGeometryCollection()
@@ -458,6 +459,8 @@ QgsRectangle QgsGeometryCollection::calculateBoundingBox() const
void QgsGeometryCollection::clearCache() const
{
mBoundingBox = QgsRectangle();
mHasCachedValidity = false;
mValidityFailureReason.clear();
QgsAbstractGeometry::clearCache();
}

@@ -772,6 +775,25 @@ QgsPoint QgsGeometryCollection::vertexAt( QgsVertexId id ) const
return mGeometries[id.part]->vertexAt( id );
}

bool QgsGeometryCollection::isValid( QString &error, int flags ) const
{
if ( flags == 0 && mHasCachedValidity )
{
// use cached validity results
error = mValidityFailureReason;
return error.isEmpty();
}

QgsGeos geos( this );
bool res = geos.isValid( &error, flags & QgsGeometry::FlagAllowSelfTouchingHoles, nullptr );
if ( flags == 0 )
{
mValidityFailureReason = !res ? error : QString();
mHasCachedValidity = true;
}
return res;
}

bool QgsGeometryCollection::addZValue( double zValue )
{
if ( QgsWkbTypes::hasZ( mWkbType ) )
@@ -206,6 +206,7 @@ class CORE_EXPORT QgsGeometryCollection: public QgsAbstractGeometry
int ringCount( int part = 0 ) const override;
int partCount() const override;
QgsPoint vertexAt( QgsVertexId id ) const override;
bool isValid( QString &error SIP_OUT, int flags = 0 ) const override;

bool addZValue( double zValue = 0 ) override;
bool addMValue( double mValue = 0 ) override;
@@ -321,6 +322,8 @@ class CORE_EXPORT QgsGeometryCollection: public QgsAbstractGeometry
private:

mutable QgsRectangle mBoundingBox;
mutable bool mHasCachedValidity = false;
mutable QString mValidityFailureReason;
};

// clazy:excludeall=qstring-allocations
@@ -182,6 +182,11 @@ double QgsMultiPoint::segmentLength( QgsVertexId ) const
return 0.0;
}

bool QgsMultiPoint::isValid( QString &, int ) const
{
return true;
}

void QgsMultiPoint::filterVertices( const std::function<bool ( const QgsPoint & )> &filter )
{
mGeometries.erase( std::remove_if( mGeometries.begin(), mGeometries.end(), // clazy:exclude=detaching-member
@@ -45,6 +45,7 @@ class CORE_EXPORT QgsMultiPoint: public QgsGeometryCollection
QgsAbstractGeometry *boundary() const override SIP_FACTORY;
int vertexNumberFromVertexId( QgsVertexId id ) const override;
double segmentLength( QgsVertexId startVertex ) const override;
bool isValid( QString &error SIP_OUT, int flags = 0 ) const override;

#ifndef SIP_RUN
void filterVertices( const std::function< bool( const QgsPoint & ) > &filter ) override;
@@ -349,6 +349,11 @@ QgsAbstractGeometry *QgsPoint::boundary() const
return nullptr;
}

bool QgsPoint::isValid( QString &, int ) const
{
return true;
}

bool QgsPoint::insertVertex( QgsVertexId position, const QgsPoint &vertex )
{
Q_UNUSED( position );
@@ -444,6 +444,7 @@ class CORE_EXPORT QgsPoint: public QgsAbstractGeometry
int nCoordinates() const override;
int vertexNumberFromVertexId( QgsVertexId id ) const override;
QgsAbstractGeometry *boundary() const override SIP_FACTORY;
bool isValid( QString &error SIP_OUT, int flags = 0 ) const override;

//low-level editing
bool insertVertex( QgsVertexId position, const QgsPoint &vertex ) override;
@@ -18,5 +18,32 @@
#include "qgssurface.h"
#include "qgspoint.h"
#include "qgspolygon.h"

#include "qgsgeos.h"
#include <memory>

bool QgsSurface::isValid( QString &error, int flags ) const
{
if ( flags == 0 && mHasCachedValidity )
{
// use cached validity results
error = mValidityFailureReason;
return error.isEmpty();
}

QgsGeos geos( this );
bool res = geos.isValid( &error, flags & QgsGeometry::FlagAllowSelfTouchingHoles, nullptr );
if ( flags == 0 )
{
mValidityFailureReason = !res ? error : QString();
mHasCachedValidity = true;
}
return res;
}

void QgsSurface::clearCache() const
{
mBoundingBox = QgsRectangle();
mHasCachedValidity = false;
mValidityFailureReason.clear();
QgsAbstractGeometry::clearCache();
}
@@ -39,9 +39,6 @@ class CORE_EXPORT QgsSurface: public QgsAbstractGeometry
*/
virtual QgsPolygon *surfaceToPolygon() const = 0 SIP_FACTORY;

/**
* Returns the minimal bounding box for the geometry
*/
QgsRectangle boundingBox() const override
{
if ( mBoundingBox.isNull() )
@@ -51,6 +48,9 @@ class CORE_EXPORT QgsSurface: public QgsAbstractGeometry
return mBoundingBox;
}

bool isValid( QString &error SIP_OUT, int flags = 0 ) const override;


#ifndef SIP_RUN

/**
@@ -75,9 +75,11 @@ class CORE_EXPORT QgsSurface: public QgsAbstractGeometry
#endif
protected:

void clearCache() const override { mBoundingBox = QgsRectangle(); QgsAbstractGeometry::clearCache(); }
void clearCache() const override;

mutable QgsRectangle mBoundingBox;
mutable bool mHasCachedValidity = false;
mutable QString mValidityFailureReason;
};

#endif // QGSSURFACE_H
@@ -5052,15 +5052,18 @@ def testIsGeosValid(self):
['Polygon((0 3, 3 0, 3 3, 0 0, 0 3))', False, False, 'Self-intersection'],
]
for t in tests:
# run each check 2 times to allow for testing of cached value
g1 = QgsGeometry.fromWkt(t[0])
res = g1.isGeosValid()
self.assertEqual(res, t[1],
"mismatch for {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[1], res))
if not res:
self.assertEqual(g1.lastError(), t[3], t[0])
res = g1.isGeosValid(QgsGeometry.FlagAllowSelfTouchingHoles)
self.assertEqual(res, t[2],
"mismatch for {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[2], res))
for i in range(2):
res = g1.isGeosValid()
self.assertEqual(res, t[1],
"mismatch for {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[1], res))
if not res:
self.assertEqual(g1.lastError(), t[3], t[0])
for i in range(2):
res = g1.isGeosValid(QgsGeometry.FlagAllowSelfTouchingHoles)
self.assertEqual(res, t[2],
"mismatch for {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[2], res))

def testValidateGeometry(self):
tests = [

0 comments on commit 75697d7

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