mirror of
https://github.com/qgis/QGIS.git
synced 2025-04-14 00:07:35 -04:00
Merge pull request #9316 from m-kuhn/fix-geometry-validation-crashes
[geometry validation] Stability and performance improvements
This commit is contained in:
commit
dcc92de5d0
@ -51,7 +51,7 @@ If ``useMapCrs`` is ``True``, geometries will be reprojected to the mapCrs defin
|
||||
in ``context``.
|
||||
%End
|
||||
|
||||
const QgsFeature &feature() const;
|
||||
QgsFeature feature() const;
|
||||
%Docstring
|
||||
Returns the feature.
|
||||
The geometry will not be reprojected regardless of useMapCrs.
|
||||
@ -63,7 +63,7 @@ The geometry will not be reprojected regardless of useMapCrs.
|
||||
The layer id.
|
||||
%End
|
||||
|
||||
const QgsGeometry &geometry() const;
|
||||
QgsGeometry geometry() const;
|
||||
%Docstring
|
||||
Returns the geometry of this feature.
|
||||
If useMapCrs was specified, it will already be reprojected into the
|
||||
|
@ -53,7 +53,8 @@ QgsGeometryCheckError::QgsGeometryCheckError( const QgsGeometryCheck *check,
|
||||
{
|
||||
if ( vidx.part != -1 )
|
||||
{
|
||||
mGeometry = QgsGeometry( QgsGeometryCheckerUtils::getGeomPart( layerFeature.geometry().constGet(), vidx.part )->clone() );
|
||||
const QgsGeometry geom = layerFeature.geometry();
|
||||
mGeometry = QgsGeometry( QgsGeometryCheckerUtils::getGeomPart( geom.constGet(), vidx.part )->clone() );
|
||||
}
|
||||
else
|
||||
{
|
||||
|
@ -52,7 +52,7 @@ QgsGeometryCheckerUtils::LayerFeature::LayerFeature( const QgsFeaturePool *pool,
|
||||
}
|
||||
}
|
||||
|
||||
const QgsFeature &QgsGeometryCheckerUtils::LayerFeature::feature() const
|
||||
QgsFeature QgsGeometryCheckerUtils::LayerFeature::feature() const
|
||||
{
|
||||
return mFeature;
|
||||
}
|
||||
@ -67,7 +67,7 @@ QString QgsGeometryCheckerUtils::LayerFeature::layerId() const
|
||||
return mFeaturePool->layerId();
|
||||
}
|
||||
|
||||
const QgsGeometry &QgsGeometryCheckerUtils::LayerFeature::geometry() const
|
||||
QgsGeometry QgsGeometryCheckerUtils::LayerFeature::geometry() const
|
||||
{
|
||||
return mGeometry;
|
||||
}
|
||||
|
@ -64,7 +64,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckerUtils
|
||||
* Returns the feature.
|
||||
* The geometry will not be reprojected regardless of useMapCrs.
|
||||
*/
|
||||
const QgsFeature &feature() const;
|
||||
QgsFeature feature() const;
|
||||
|
||||
/**
|
||||
* The layer.
|
||||
@ -81,7 +81,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckerUtils
|
||||
* If useMapCrs was specified, it will already be reprojected into the
|
||||
* CRS specified in the context specified in the constructor.
|
||||
*/
|
||||
const QgsGeometry &geometry() const;
|
||||
QgsGeometry geometry() const;
|
||||
|
||||
/**
|
||||
* Returns a combination of the layerId and the feature id.
|
||||
|
@ -58,6 +58,7 @@ void QgsGeometryGapCheck::collectErrors( const QMap<QString, QgsFeaturePool *> &
|
||||
}
|
||||
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( nullptr, mContext->tolerance );
|
||||
geomEngine->prepareGeometry();
|
||||
|
||||
// Create union of geometry
|
||||
QString errMsg;
|
||||
@ -70,6 +71,7 @@ void QgsGeometryGapCheck::collectErrors( const QMap<QString, QgsFeaturePool *> &
|
||||
|
||||
// Get envelope of union
|
||||
geomEngine = QgsGeometryCheckerUtils::createGeomEngine( unionGeom.get(), mContext->tolerance );
|
||||
geomEngine->prepareGeometry();
|
||||
std::unique_ptr<QgsAbstractGeometry> envelope( geomEngine->envelope( &errMsg ) );
|
||||
if ( !envelope )
|
||||
{
|
||||
@ -79,11 +81,13 @@ void QgsGeometryGapCheck::collectErrors( const QMap<QString, QgsFeaturePool *> &
|
||||
|
||||
// Buffer envelope
|
||||
geomEngine = QgsGeometryCheckerUtils::createGeomEngine( envelope.get(), mContext->tolerance );
|
||||
geomEngine->prepareGeometry();
|
||||
QgsAbstractGeometry *bufEnvelope = geomEngine->buffer( 2, 0, GEOSBUF_CAP_SQUARE, GEOSBUF_JOIN_MITRE, 4. ); //#spellok //#spellok
|
||||
envelope.reset( bufEnvelope );
|
||||
|
||||
// Compute difference between envelope and union to obtain gap polygons
|
||||
geomEngine = QgsGeometryCheckerUtils::createGeomEngine( envelope.get(), mContext->tolerance );
|
||||
geomEngine->prepareGeometry();
|
||||
std::unique_ptr<QgsAbstractGeometry> diffGeom( geomEngine->difference( unionGeom.get(), &errMsg ) );
|
||||
if ( !diffGeom )
|
||||
{
|
||||
@ -115,10 +119,11 @@ void QgsGeometryGapCheck::collectErrors( const QMap<QString, QgsFeaturePool *> &
|
||||
const QgsGeometryCheckerUtils::LayerFeatures layerFeatures( featurePools, featureIds.keys(), gapAreaBBox, compatibleGeometryTypes(), mContext );
|
||||
for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
|
||||
{
|
||||
if ( QgsGeometryCheckerUtils::sharedEdgeLength( gapGeom.get(), layerFeature.geometry().constGet(), mContext->reducedTolerance ) > 0 )
|
||||
const QgsGeometry geom = layerFeature.geometry();
|
||||
if ( QgsGeometryCheckerUtils::sharedEdgeLength( gapGeom.get(), geom.constGet(), mContext->reducedTolerance ) > 0 )
|
||||
{
|
||||
neighboringIds[layerFeature.layer()->id()].insert( layerFeature.feature().id() );
|
||||
gapAreaBBox.combineExtentWith( layerFeature.geometry().constGet()->boundingBox() );
|
||||
gapAreaBBox.combineExtentWith( layerFeature.geometry().boundingBox() );
|
||||
}
|
||||
}
|
||||
|
||||
@ -193,7 +198,7 @@ bool QgsGeometryGapCheck::mergeWithNeighbor( const QMap<QString, QgsFeaturePool
|
||||
{
|
||||
continue;
|
||||
}
|
||||
QgsGeometry featureGeom = testFeature.geometry();
|
||||
const QgsGeometry featureGeom = testFeature.geometry();
|
||||
const QgsAbstractGeometry *testGeom = featureGeom.constGet();
|
||||
for ( int iPart = 0, nParts = testGeom->partCount(); iPart < nParts; ++iPart )
|
||||
{
|
||||
@ -219,7 +224,7 @@ bool QgsGeometryGapCheck::mergeWithNeighbor( const QMap<QString, QgsFeaturePool
|
||||
std::unique_ptr<QgsAbstractGeometry> errLayerGeom( errGeometry->clone() );
|
||||
QgsCoordinateTransform ct( featurePool->crs(), mContext->mapCrs, mContext->transformContext );
|
||||
errLayerGeom->transform( ct, QgsCoordinateTransform::ReverseTransform );
|
||||
QgsGeometry mergeFeatureGeom = mergeFeature.geometry();
|
||||
const QgsGeometry mergeFeatureGeom = mergeFeature.geometry();
|
||||
const QgsAbstractGeometry *mergeGeom = mergeFeatureGeom.constGet();
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( errLayerGeom.get(), mContext->reducedTolerance );
|
||||
std::unique_ptr<QgsAbstractGeometry> combinedGeom( geomEngine->combine( QgsGeometryCheckerUtils::getGeomPart( mergeGeom, mergePartIdx ), &errMsg ) );
|
||||
|
@ -57,7 +57,7 @@ void QgsGeometryLineLayerIntersectionCheck::collectErrors( const QMap<QString, Q
|
||||
}
|
||||
else if ( const QgsPolygon *polygon = dynamic_cast<const QgsPolygon *>( part ) )
|
||||
{
|
||||
QList< const QgsLineString * > rings = QgsGeometryCheckerUtils::polygonRings( polygon );
|
||||
const QList< const QgsLineString * > rings = QgsGeometryCheckerUtils::polygonRings( polygon );
|
||||
for ( const QgsLineString *ring : rings )
|
||||
{
|
||||
const QList< QgsPoint > intersections = QgsGeometryCheckerUtils::lineIntersections( line, ring, mContext->tolerance );
|
||||
|
@ -48,7 +48,8 @@ void QgsGeometryMissingVertexCheck::collectErrors( const QMap<QString, QgsFeatur
|
||||
break;
|
||||
}
|
||||
|
||||
const QgsAbstractGeometry *geom = layerFeature.geometry().constGet();
|
||||
const QgsGeometry geometry = layerFeature.geometry();
|
||||
const QgsAbstractGeometry *geom = geometry.constGet();
|
||||
|
||||
if ( QgsCurvePolygon *polygon = qgsgeometry_cast<QgsCurvePolygon *>( geom ) )
|
||||
{
|
||||
@ -129,7 +130,7 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
|
||||
const QgsFeature ¤tFeature = layerFeature.feature();
|
||||
std::unique_ptr<QgsMultiPolygon> boundaries = qgis::make_unique<QgsMultiPolygon>();
|
||||
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( polygon->exteriorRing(), mContext->tolerance );
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( polygon->exteriorRing()->clone(), mContext->tolerance );
|
||||
boundaries->addGeometry( geomEngine->buffer( mContext->tolerance, 5 ) );
|
||||
|
||||
const int numRings = polygon->numInteriorRings();
|
||||
@ -155,7 +156,8 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
|
||||
if ( feedback && feedback->isCanceled() )
|
||||
break;
|
||||
|
||||
QgsVertexIterator vertexIterator = compareFeature.geometry().vertices();
|
||||
const QgsGeometry compareGeometry = compareFeature.geometry();
|
||||
QgsVertexIterator vertexIterator = compareGeometry.vertices();
|
||||
while ( vertexIterator.hasNext() )
|
||||
{
|
||||
const QgsPoint &pt = vertexIterator.next();
|
||||
|
@ -41,8 +41,10 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
|
||||
// Ensure each pair of layers only gets compared once: remove the current layer from the layerIds, but add it to the layerList for layerFeaturesB
|
||||
layerIds.removeOne( layerFeatureA.layer()->id() );
|
||||
|
||||
QgsRectangle bboxA = layerFeatureA.geometry().constGet()->boundingBox();
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry().constGet(), mContext->tolerance );
|
||||
const QgsGeometry geomA = layerFeatureA.geometry();
|
||||
QgsRectangle bboxA = geomA.boundingBox();
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geomA.constGet(), mContext->tolerance );
|
||||
geomEngineA->prepareGeometry();
|
||||
if ( !geomEngineA->isValid() )
|
||||
{
|
||||
messages.append( tr( "Overlap check failed for (%1): the geometry is invalid" ).arg( layerFeatureA.id() ) );
|
||||
@ -56,15 +58,17 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
|
||||
break;
|
||||
|
||||
// > : only report overlaps within same layer once
|
||||
if ( layerFeatureA.layer()->id() == layerFeatureB.layer()->id() && layerFeatureB.feature().id() >= layerFeatureA.feature().id() )
|
||||
if ( layerFeatureA.layerId() == layerFeatureB.layerId() && layerFeatureB.feature().id() >= layerFeatureA.feature().id() )
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
QString errMsg;
|
||||
if ( geomEngineA->overlaps( layerFeatureB.geometry().constGet(), &errMsg ) )
|
||||
const QgsGeometry geometryB = layerFeatureB.geometry();
|
||||
const QgsAbstractGeometry *geomB = geometryB.constGet();
|
||||
if ( geomEngineA->overlaps( geomB, &errMsg ) )
|
||||
{
|
||||
std::unique_ptr<QgsAbstractGeometry> interGeom( geomEngineA->intersection( layerFeatureB.geometry().constGet() ) );
|
||||
std::unique_ptr<QgsAbstractGeometry> interGeom( geomEngineA->intersection( geomB ) );
|
||||
if ( interGeom && !interGeom->isEmpty() )
|
||||
{
|
||||
QgsGeometryCheckerUtils::filter1DTypes( interGeom.get() );
|
||||
@ -106,14 +110,17 @@ void QgsGeometryOverlapCheck::fixError( const QMap<QString, QgsFeaturePool *> &f
|
||||
// Check if error still applies
|
||||
QgsGeometryCheckerUtils::LayerFeature layerFeatureA( featurePoolA, featureA, mContext, true );
|
||||
QgsGeometryCheckerUtils::LayerFeature layerFeatureB( featurePoolB, featureB, mContext, true );
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry().constGet(), mContext->reducedTolerance );
|
||||
const QgsGeometry geometryA = layerFeatureA.geometry();
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geometryA.constGet(), mContext->reducedTolerance );
|
||||
geomEngineA->prepareGeometry();
|
||||
|
||||
if ( !geomEngineA->overlaps( layerFeatureB.geometry().constGet() ) )
|
||||
const QgsGeometry geometryB = layerFeatureB.geometry();
|
||||
if ( !geomEngineA->overlaps( geometryB.constGet() ) )
|
||||
{
|
||||
error->setObsolete();
|
||||
return;
|
||||
}
|
||||
std::unique_ptr< QgsAbstractGeometry > interGeom( geomEngineA->intersection( layerFeatureB.geometry().constGet(), &errMsg ) );
|
||||
std::unique_ptr< QgsAbstractGeometry > interGeom( geomEngineA->intersection( geometryB.constGet(), &errMsg ) );
|
||||
if ( !interGeom )
|
||||
{
|
||||
error->setFixFailed( tr( "Failed to compute intersection between overlapping features: %1" ).arg( errMsg ) );
|
||||
@ -154,7 +161,8 @@ void QgsGeometryOverlapCheck::fixError( const QMap<QString, QgsFeaturePool *> &f
|
||||
{
|
||||
QgsGeometryCheckerUtils::filter1DTypes( diff1.get() );
|
||||
}
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureB.geometry().constGet(), mContext->reducedTolerance );
|
||||
std::unique_ptr< QgsGeometryEngine > geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( geometryB.constGet(), mContext->reducedTolerance );
|
||||
geomEngineB->prepareGeometry();
|
||||
std::unique_ptr< QgsAbstractGeometry > diff2( geomEngineB->difference( interPart, &errMsg ) );
|
||||
if ( !diff2 || diff2->isEmpty() )
|
||||
{
|
||||
|
Loading…
x
Reference in New Issue
Block a user