Skip to content

Commit adb6fd3

Browse files
m-kuhnm-kuhn
committedMar 18, 2019
Avoid working on reference to temporary objects
fixes a couple of crashes in geometry validation
·
final-3_4_15final-3_4_6
1 parent 0ed7a62 commit adb6fd3

File tree

7 files changed

+40
-23
lines changed

7 files changed

+40
-23
lines changed
 

‎python/analysis/auto_generated/vector/geometry_checker/qgsgeometrycheckerutils.sip.in

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ If ``useMapCrs`` is True, geometries will be reprojected to the mapCrs defined
4444
in ``context``.
4545
%End
4646

47-
const QgsFeature &feature() const;
47+
QgsFeature feature() const;
4848
%Docstring
4949
Returns the feature.
5050
The geometry will not be reprojected regardless of useMapCrs.
@@ -56,13 +56,17 @@ The geometry will not be reprojected regardless of useMapCrs.
5656
The layer id.
5757
%End
5858

59-
const QgsGeometry &geometry() const;
59+
QgsGeometry geometry() const;
6060
%Docstring
6161
Returns the geometry of this feature.
6262
If useMapCrs was specified, it will already be reprojected into the
6363
CRS specified in the context specified in the constructor.
6464
%End
65+
6566
QString id() const;
67+
%Docstring
68+
Returns a combination of the layerId and the feature id.
69+
%End
6670
bool operator==( const QgsGeometryCheckerUtils::LayerFeature &other ) const;
6771
bool operator!=( const QgsGeometryCheckerUtils::LayerFeature &other ) const;
6872

‎src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ QgsGeometryCheckerUtils::LayerFeature::LayerFeature( const QgsFeaturePool *pool,
5252
}
5353
}
5454

55-
const QgsFeature &QgsGeometryCheckerUtils::LayerFeature::feature() const
55+
QgsFeature QgsGeometryCheckerUtils::LayerFeature::feature() const
5656
{
5757
return mFeature;
5858
}
@@ -67,7 +67,7 @@ QString QgsGeometryCheckerUtils::LayerFeature::layerId() const
6767
return mFeaturePool->layerId();
6868
}
6969

70-
const QgsGeometry &QgsGeometryCheckerUtils::LayerFeature::geometry() const
70+
QgsGeometry QgsGeometryCheckerUtils::LayerFeature::geometry() const
7171
{
7272
return mGeometry;
7373
}

‎src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckerUtils
5656
* Returns the feature.
5757
* The geometry will not be reprojected regardless of useMapCrs.
5858
*/
59-
const QgsFeature &feature() const;
59+
QgsFeature feature() const;
6060

6161
/**
6262
* The layer.
@@ -73,7 +73,11 @@ class ANALYSIS_EXPORT QgsGeometryCheckerUtils
7373
* If useMapCrs was specified, it will already be reprojected into the
7474
* CRS specified in the context specified in the constructor.
7575
*/
76-
const QgsGeometry &geometry() const;
76+
QgsGeometry geometry() const;
77+
78+
/**
79+
* Returns a combination of the layerId and the feature id.
80+
*/
7781
QString id() const;
7882
bool operator==( const QgsGeometryCheckerUtils::LayerFeature &other ) const;
7983
bool operator!=( const QgsGeometryCheckerUtils::LayerFeature &other ) const;

‎src/analysis/vector/geometry_checker/qgsgeometrygapcheck.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,11 @@ void QgsGeometryGapCheck::collectErrors( const QMap<QString, QgsFeaturePool *> &
117117
const QgsGeometryCheckerUtils::LayerFeatures layerFeatures( featurePools, featureIds.keys(), gapAreaBBox, compatibleGeometryTypes(), mContext );
118118
for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
119119
{
120-
if ( QgsGeometryCheckerUtils::sharedEdgeLength( gapGeom.get(), layerFeature.geometry().constGet(), mContext->reducedTolerance ) > 0 )
120+
const QgsGeometry geom = layerFeature.geometry();
121+
if ( QgsGeometryCheckerUtils::sharedEdgeLength( gapGeom.get(), geom.constGet(), mContext->reducedTolerance ) > 0 )
121122
{
122123
neighboringIds[layerFeature.layer()->id()].insert( layerFeature.feature().id() );
123-
gapAreaBBox.combineExtentWith( layerFeature.geometry().constGet()->boundingBox() );
124+
gapAreaBBox.combineExtentWith( layerFeature.geometry().boundingBox() );
124125
}
125126
}
126127

@@ -195,7 +196,7 @@ bool QgsGeometryGapCheck::mergeWithNeighbor( const QMap<QString, QgsFeaturePool
195196
{
196197
continue;
197198
}
198-
QgsGeometry featureGeom = testFeature.geometry();
199+
const QgsGeometry featureGeom = testFeature.geometry();
199200
const QgsAbstractGeometry *testGeom = featureGeom.constGet();
200201
for ( int iPart = 0, nParts = testGeom->partCount(); iPart < nParts; ++iPart )
201202
{
@@ -221,7 +222,7 @@ bool QgsGeometryGapCheck::mergeWithNeighbor( const QMap<QString, QgsFeaturePool
221222
std::unique_ptr<QgsAbstractGeometry> errLayerGeom( errGeometry->clone() );
222223
QgsCoordinateTransform ct( featurePool->crs(), mContext->mapCrs, mContext->transformContext );
223224
errLayerGeom->transform( ct, QgsCoordinateTransform::ReverseTransform );
224-
QgsGeometry mergeFeatureGeom = mergeFeature.geometry();
225+
const QgsGeometry mergeFeatureGeom = mergeFeature.geometry();
225226
const QgsAbstractGeometry *mergeGeom = mergeFeatureGeom.constGet();
226227
std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( errLayerGeom.get(), mContext->reducedTolerance );
227228
std::unique_ptr<QgsAbstractGeometry> combinedGeom( geomEngine->combine( QgsGeometryCheckerUtils::getGeomPart( mergeGeom, mergePartIdx ), &errMsg ) );

‎src/analysis/vector/geometry_checker/qgsgeometrylinelayerintersectioncheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ void QgsGeometryLineLayerIntersectionCheck::collectErrors( const QMap<QString, Q
5757
}
5858
else if ( const QgsPolygon *polygon = dynamic_cast<const QgsPolygon *>( part ) )
5959
{
60-
QList< const QgsLineString * > rings = QgsGeometryCheckerUtils::polygonRings( polygon );
60+
const QList< const QgsLineString * > rings = QgsGeometryCheckerUtils::polygonRings( polygon );
6161
for ( const QgsLineString *ring : rings )
6262
{
6363
const QList< QgsPoint > intersections = QgsGeometryCheckerUtils::lineIntersections( line, ring, mContext->tolerance );

‎src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ void QgsGeometryMissingVertexCheck::collectErrors( const QMap<QString, QgsFeatur
4848
break;
4949
}
5050

51-
const QgsAbstractGeometry *geom = layerFeature.geometry().constGet();
51+
const QgsGeometry geometry = layerFeature.geometry();
52+
const QgsAbstractGeometry *geom = geometry.constGet();
5253

5354
if ( QgsCurvePolygon *polygon = qgsgeometry_cast<QgsCurvePolygon *>( geom ) )
5455
{
@@ -129,7 +130,7 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
129130
const QgsFeature &currentFeature = layerFeature.feature();
130131
std::unique_ptr<QgsMultiPolygon> boundaries = qgis::make_unique<QgsMultiPolygon>();
131132

132-
std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( polygon->exteriorRing(), mContext->tolerance );
133+
std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( polygon->exteriorRing()->clone(), mContext->tolerance );
133134
boundaries->addGeometry( geomEngine->buffer( mContext->tolerance, 5 ) );
134135

135136
const int numRings = polygon->numInteriorRings();
@@ -155,7 +156,8 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
155156
if ( feedback && feedback->isCanceled() )
156157
break;
157158

158-
QgsVertexIterator vertexIterator = compareFeature.geometry().vertices();
159+
const QgsGeometry compareGeometry = compareFeature.geometry();
160+
QgsVertexIterator vertexIterator = compareGeometry.vertices();
159161
while ( vertexIterator.hasNext() )
160162
{
161163
const QgsPoint &pt = vertexIterator.next();

‎src/analysis/vector/geometry_checker/qgsgeometryoverlapcheck.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,10 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
4141
// Ensure each pair of layers only gets compared once: remove the current layer from the layerIds, but add it to the layerList for layerFeaturesB
4242
layerIds.removeOne( layerFeatureA.layer()->id() );
4343

44-
QgsRectangle bboxA = layerFeatureA.geometry().constGet()->boundingBox();
45-
std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry().constGet(), mContext->tolerance );
44+
const QgsGeometry geomA = layerFeatureA.geometry();
45+
QgsRectangle bboxA = geomA.boundingBox();
46+
std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geomA.constGet(), mContext->tolerance );
47+
geomEngineA->prepareGeometry();
4648
if ( !geomEngineA->isValid() )
4749
{
4850
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
5658
break;
5759

5860
// > : only report overlaps within same layer once
59-
if ( layerFeatureA.layer()->id() == layerFeatureB.layer()->id() && layerFeatureB.feature().id() >= layerFeatureA.feature().id() )
61+
if ( layerFeatureA.layerId() == layerFeatureB.layerId() && layerFeatureB.feature().id() >= layerFeatureA.feature().id() )
6062
{
6163
continue;
6264
}
6365

6466
QString errMsg;
65-
if ( geomEngineA->overlaps( layerFeatureB.geometry().constGet(), &errMsg ) )
67+
const QgsGeometry geometryB = layerFeatureB.geometry();
68+
const QgsAbstractGeometry *geomB = geometryB.constGet();
69+
if ( geomEngineA->overlaps( geomB, &errMsg ) )
6670
{
67-
std::unique_ptr<QgsAbstractGeometry> interGeom( geomEngineA->intersection( layerFeatureB.geometry().constGet() ) );
71+
std::unique_ptr<QgsAbstractGeometry> interGeom( geomEngineA->intersection( geomB ) );
6872
if ( interGeom && !interGeom->isEmpty() )
6973
{
7074
QgsGeometryCheckerUtils::filter1DTypes( interGeom.get() );
@@ -106,14 +110,16 @@ void QgsGeometryOverlapCheck::fixError( const QMap<QString, QgsFeaturePool *> &f
106110
// Check if error still applies
107111
QgsGeometryCheckerUtils::LayerFeature layerFeatureA( featurePoolA, featureA, mContext, true );
108112
QgsGeometryCheckerUtils::LayerFeature layerFeatureB( featurePoolB, featureB, mContext, true );
109-
std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry().constGet(), mContext->reducedTolerance );
113+
const QgsGeometry geometryA = layerFeatureA.geometry();
114+
std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geometryA.constGet(), mContext->reducedTolerance );
110115

111-
if ( !geomEngineA->overlaps( layerFeatureB.geometry().constGet() ) )
116+
const QgsGeometry geometryB = layerFeatureB.geometry();
117+
if ( !geomEngineA->overlaps( geometryB.constGet() ) )
112118
{
113119
error->setObsolete();
114120
return;
115121
}
116-
std::unique_ptr< QgsAbstractGeometry > interGeom( geomEngineA->intersection( layerFeatureB.geometry().constGet(), &errMsg ) );
122+
std::unique_ptr< QgsAbstractGeometry > interGeom( geomEngineA->intersection( geometryB.constGet(), &errMsg ) );
117123
if ( !interGeom )
118124
{
119125
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
154160
{
155161
QgsGeometryCheckerUtils::filter1DTypes( diff1.get() );
156162
}
157-
std::unique_ptr< QgsGeometryEngine > geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureB.geometry().constGet(), mContext->reducedTolerance );
163+
std::unique_ptr< QgsGeometryEngine > geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( geometryB.constGet(), mContext->reducedTolerance );
158164
std::unique_ptr< QgsAbstractGeometry > diff2( geomEngineB->difference( interPart, &errMsg ) );
159165
if ( !diff2 || diff2->isEmpty() )
160166
{

0 commit comments

Comments
 (0)
Please sign in to comment.