Skip to content
Permalink
Browse files

Avoid working on reference to temporary objects

fixes a couple of crashes in geometry validation
  • Loading branch information
m-kuhn committed Mar 18, 2019
1 parent 0ed7a62 commit adb6fd3b10b977694aab5ddc0650a9423931e128
@@ -44,7 +44,7 @@ If ``useMapCrs`` is True, geometries will be reprojected to the mapCrs defined
in ``context``.
%End

const QgsFeature &feature() const;
QgsFeature feature() const;
%Docstring
Returns the feature.
The geometry will not be reprojected regardless of useMapCrs.
@@ -56,13 +56,17 @@ 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
CRS specified in the context specified in the constructor.
%End

QString id() const;
%Docstring
Returns a combination of the layerId and the feature id.
%End
bool operator==( const QgsGeometryCheckerUtils::LayerFeature &other ) const;
bool operator!=( const QgsGeometryCheckerUtils::LayerFeature &other ) const;

@@ -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;
}
@@ -56,7 +56,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.
@@ -73,7 +73,11 @@ 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.
*/
QString id() const;
bool operator==( const QgsGeometryCheckerUtils::LayerFeature &other ) const;
bool operator!=( const QgsGeometryCheckerUtils::LayerFeature &other ) const;
@@ -117,10 +117,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() );
}
}

@@ -195,7 +196,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 )
{
@@ -221,7 +222,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 &currentFeature = 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,16 @@ 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 );

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 +160,7 @@ 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 );
std::unique_ptr< QgsAbstractGeometry > diff2( geomEngineB->difference( interPart, &errMsg ) );
if ( !diff2 || diff2->isEmpty() )
{

0 comments on commit adb6fd3

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