Skip to content

Commit

Permalink
Refine QgsFeature geometry getters/setters
Browse files Browse the repository at this point in the history 
All pointer based methods have been removed.

Now we have only:

  void setGeometry( const QgsGeometry& geom )

and

  QgsGeometry geometry() const

Benefits include avoiding a whole lot of tricky pointer lifetime
issues, potential memory leaks, and finally closing #777, which
has survived for over 9 years!...

Impacts on PyQGIS code:
- no more need for the messy
  g = QgsGeometry( feature.geometry() )
  workaround, just use g = feature.geometry() instead
- IMPORTANT: you can no longer test whether a feature has geometry
 using `if f.geometry():`, since QgsFeature::geometry() will
 *always* return an object. Instead, use
 `if not f.geometry().isEmpty():`, or preferably the new method
 `if not f.hasGeometry():`

Fix #777
  • Loading branch information
@nyalldawson
nyalldawson committed Aug 1, 2016
1 parent aceddae commit bd7d913
Show file tree
Hide file tree
Showing 202 changed files with 1,377 additions and 1,313 deletions.
29 changes: 29 additions & 0 deletions doc/api_break.dox
View file
Expand Up @@ -218,15 +218,34 @@ attributeIndexes(), pkAttributeIndexes(), isSaveAndLoadStyleToDBSupported()</li>
<li>geometryAndOwnership() has been removed. Use geometry() instead.</li>
<li>setGeometryAndOwnership() has been removed. Use setGeometry() instead.</li>
<li>The setGeometry( QgsGeometry* ) method has been removed, use setGeometry( const QgsGeometry& ) instead.</li>
<li>The geometry() method now returns a copy of the geometry, not a pointer. Since QgsGeometry objects are
implicitly shared this is a low-cost copy, and avoids ownership and dangling pointer issues. <b>Very important: Testing that
a feature has a geometry is now done using the new hasGeometry() method. QgsFeature::geometry() will ALWAYS return
true, as the method will return an empty geometry if the feature has no geometry.</b></li>
<li>The temporary constGeometry() method has been removed. Use geometry() instead.</li>
<li>setFields( const QgsFields*, bool ) has been removed, use setFields( const QgsFields&, bool ) instead.</li>
</ul>

\subsection qgis_api_break_3_0_QgsGeometryAnalyzer QgsGeometryAnalyzer

<ul>
<li>locateBetweenMeasures() and locateAlongMeasure() now take geometry references, not pointers</li>
</ul>

\subsection qgis_api_break_3_0_QgsGroupWMSDataDialog QgsGroupWMSDataDialog

<ul>
<li>QgsGroupWMSDataDialo has been renamed to QgsGroupWmsDataDialog</li>
</ul>

\subsection qgis_api_break_3_0_QgsHighlight QgsHighlight

<ul>
<li>The QgsHighlight constructor now takes a geometry reference, not a pointer.</li>
</ul>



\subsection qgis_api_break_3_0_QgsVectorDataProvider QgsVectorDataProvider

<ul>
Expand All @@ -236,6 +255,7 @@ only affects third party c++ providers, and does not affect PyQGIS scripts.</li>
<li>The SaveAsShapefile, SelectGeometryAtId, RandomSelectGeometryAtId and SequentialSelectGeometryAtId
capabilities have been removed, as they were unused and had no effect.</li>
<li>capabilities() now returns a typesafe QgsVectorDataProvider::Capabilities object, not an integer.</li>
<li>convertToProviderType() now takes a geometry reference, not a pointer.</li>
</ul>

\subsection qgis_api_break_3_0_QgsLabelingEngineInterface QgsLabelingEngineInterface
Expand Down Expand Up @@ -331,6 +351,7 @@ be returned instead of a null pointer if no transformation is required.</li>

<ul>
<li>init(QgsMapRenderer*) has been removed. Use init(const QgsMapSettings&) instead.</li>
<li>prepareGeometry and geometryRequiresPreparation now take geometry references, not pointers.</li>
</ul>

\subsection qgis_api_break_3_0_QgsOSMElement QgsOSMElement
Expand Down Expand Up @@ -435,6 +456,8 @@ setExcludeAttributesWms()</li>
setExcludeAttributesWfs()</li>
<li>changeGeometry() now accepts a geometry reference, not a pointer.</li>
<li>The geometryChanged() signal now uses a const QgsGeometry reference.</li>
<li>The deprecated removePolygonIntersections has been removed.</li>
<li>addTopologicalPoints() now takes a geometry reference, not a pointer.</li>
</ul>

\subsection qgis_api_break_3_0_QgsVectorLayerEditBuffer QgsVectorLayerEditBuffer
Expand All @@ -444,6 +467,12 @@ setExcludeAttributesWfs()</li>
<li>The geometryChanged() signal now uses a const QgsGeometry reference.</li>
</ul>

\subsection qgis_api_break_3_0_QgsVectorLayerEditUtils QgsVectorLayerEditUtils

<ul>
<li>addTopologicalPoints() now accepts a geometry reference, not a pointer.</li>
</ul>

\subsection qgis_api_break_3_0_QgsVectorLayerImport QgsVectorLayerImport

<ul>
Expand Down
4 changes: 2 additions & 2 deletions python/analysis/vector/qgsgeometryanalyzer.sip
View file
Expand Up @@ -95,10 +95,10 @@ class QgsGeometryAnalyzer
bool forceSingleGeometry = false, QgsVectorDataProvider* memoryProvider = 0, QProgressDialog* p = 0 );

/** Returns linear reference geometry as a multiline (or 0 if no match). Currently, the z-coordinates are considered to be the measures (no support for m-values in QGIS)*/
QgsGeometry* locateBetweenMeasures( double fromMeasure, double toMeasure, const QgsGeometry* lineGeom );
QgsGeometry* locateBetweenMeasures( double fromMeasure, double toMeasure, const QgsGeometry& lineGeom );
/** Returns linear reference geometry. Unlike the PostGIS function, this method always returns multipoint or 0 if no match (not geometry collection).
* Currently, the z-coordinates are considered to be the measures (no support for m-values in QGIS)
*/
QgsGeometry* locateAlongMeasure( double measure, const QgsGeometry* lineGeom );
QgsGeometry* locateAlongMeasure( double measure, const QgsGeometry& lineGeom );

};
8 changes: 8 additions & 0 deletions python/core/geometry/qgsgeometry.sip
View file
Expand Up @@ -393,6 +393,14 @@ class QgsGeometry
*/
int makeDifference( const QgsGeometry* other );

/** Returns the geometry formed by modifying this geometry such that it does not
* intersect the other geometry.
* @param other geometry that should not be intersect
* @return difference geometry, or empty geometry if difference could not be calculated
* @note added in QGIS 3.0
*/
QgsGeometry makeDifference( const QgsGeometry& other ) const;

/** Returns the bounding box of this feature*/
QgsRectangle boundingBox() const;

Expand Down
53 changes: 22 additions & 31 deletions python/core/qgsfeature.sip
View file
Expand Up @@ -322,41 +322,32 @@ class QgsFeature
*/
void setValid( bool validity );

/** Get the geometry object associated with this feature. If the geometry
* is not going to be modified than calling the const @link constGeometry @endlink
* method is preferable as it avoids a potentially expensive detach operation.
*
* It is possible to modify the geometry in place but this will
* be removed in 3.0 and therefore @link setGeometry @endlink should be called explicitly.
*
* @note will be modified to return by value in QGIS 3.0: `QgsGeometry geometry() const;`
*
* @returns pointer to feature's geometry
* @see constGeometry
* @see setGeometry
/** Returns true if the feature has an associated geometry.
* @see geometry()
* @note added in QGIS 3.0.
*/
QgsGeometry* geometry();

/** Gets a const pointer to the geometry object associated with this feature. If the geometry
* is not going to be modified than this method is preferable to the non-const
* @link geometry @endlink method.
* @note this is a temporary method for 2.x release cycle. Will be removed in QGIS 3.0.
* @returns const pointer to feature's geometry
* @see geometry
* @see geometryAndOwnership
* @see setGeometry
* @note added in QGIS 2.9
* @note will be removed in QGIS 3.0
bool hasGeometry() const;

/** Returns the geometry associated with this feature. If the feature has no geometry,
* an empty QgsGeometry object will be returned.
* @see hasGeometry()
* @see setGeometry()
*/
QgsGeometry geometry() const;

/** Set the feature's geometry.
* @param geometry new feature geometry
* @see geometry()
* @see clearGeometry()
*/
const QgsGeometry* constGeometry() const;
void setGeometry( const QgsGeometry& geometry );

/** Set this feature's geometry from another QgsGeometry object. This method performs a deep copy
* of the geometry.
* @param geom new feature geometry
* @see geometry
* @see constGeometry
/** Removes any geometry associated with the feature.
* @see setGeometry()
* @see hasGeometry()
* @note added in QGIS 3.0
*/
void setGeometry( const QgsGeometry& geom );
void clearGeometry();

/** Assign a field map with the feature to allow attribute access by attribute name.
* @param fields The attribute fields which this feature holds
Expand Down
8 changes: 4 additions & 4 deletions python/core/qgspallabeling.sip
View file
Expand Up @@ -991,10 +991,10 @@ class QgsPalLabeling : QgsLabelingEngineInterface
* @param context render context
* @param ct coordinate transform, or invalid transform if no transformation required
* @param clipGeometry geometry to clip features to, if applicable
* @returns prepared geometry, the caller takes ownership
* @returns prepared geometry
* @note added in QGIS 2.9
*/
static QgsGeometry* prepareGeometry( const QgsGeometry *geometry, QgsRenderContext &context, const QgsCoordinateTransform& ct, QgsGeometry *clipGeometry = 0 ) /Factory/;
static QgsGeometry prepareGeometry( const QgsGeometry &geometry, QgsRenderContext &context, const QgsCoordinateTransform& ct, QgsGeometry *clipGeometry = 0 ) /Factory/;

/** Checks whether a geometry requires preparation before registration with PAL
* @param geometry geometry to prepare
Expand All @@ -1004,7 +1004,7 @@ class QgsPalLabeling : QgsLabelingEngineInterface
* @returns true if geometry requires preparation
* @note added in QGIS 2.9
*/
static bool geometryRequiresPreparation( const QgsGeometry *geometry, QgsRenderContext &context, const QgsCoordinateTransform& ct, QgsGeometry *clipGeometry = 0 );
static bool geometryRequiresPreparation( const QgsGeometry& geometry, QgsRenderContext &context, const QgsCoordinateTransform& ct, QgsGeometry *clipGeometry = 0 );

/** Splits a text string to a list of separate lines, using a specified wrap character.
* The text string will be split on either newline characters or the wrap character.
Expand Down Expand Up @@ -1054,5 +1054,5 @@ class QgsPalLabeling : QgsLabelingEngineInterface
* @returns true if geometry exceeds minimum size
* @note added in QGIS 2.9
*/
static bool checkMinimumSizeMM( const QgsRenderContext &context, const QgsGeometry *geom, double minSize );
static bool checkMinimumSizeMM( const QgsRenderContext &context, const QgsGeometry* geom, double minSize );
};
2 changes: 1 addition & 1 deletion python/core/qgsvectordataprovider.sip
View file
Expand Up @@ -385,7 +385,7 @@ class QgsVectorDataProvider : QgsDataProvider

/** Converts the geometry to the provider type if possible / necessary
@return the converted geometry or nullptr if no conversion was necessary or possible*/
QgsGeometry* convertToProviderType( const QgsGeometry* geom ) const /Factory/;
QgsGeometry* convertToProviderType( const QgsGeometry& geom ) const /Factory/;
};

QFlags<QgsVectorDataProvider::Capability> operator|(QgsVectorDataProvider::Capability f1, QFlags<QgsVectorDataProvider::Capability> f2);
12 changes: 1 addition & 11 deletions python/core/qgsvectorlayer.sip
View file
Expand Up @@ -763,22 +763,12 @@ class QgsVectorLayer : QgsMapLayer
*/
int splitFeatures( const QList<QgsPoint>& splitLine, bool topologicalEditing = false );

/** Changes the specified geometry such that it has no intersections with other
* polygon (or multipolygon) geometries in this vector layer
* @param geom geometry to modify
* @param ignoreFeatures list of feature ids where intersections should be ignored
* @return 0 in case of success
*
* @deprecated since 2.2 - not being used for "avoid intersections" functionality anymore
*/
int removePolygonIntersections( QgsGeometry* geom, const QgsFeatureIds& ignoreFeatures = QgsFeatureIds() ) /Deprecated/;

/** Adds topological points for every vertex of the geometry.
* @param geom the geometry where each vertex is added to segments of other features
* @note geom is not going to be modified by the function
* @return 0 in case of success
*/
int addTopologicalPoints( const QgsGeometry* geom );
int addTopologicalPoints( const QgsGeometry& geom );

/** Adds a vertex to segments which intersect point p but don't
* already have a vertex there. If a feature already has a vertex at position p,
Expand Down
2 changes: 1 addition & 1 deletion python/core/qgsvectorlayereditutils.sip
View file
Expand Up @@ -138,7 +138,7 @@ class QgsVectorLayerEditUtils
* @note geom is not going to be modified by the function
* @return 0 in case of success
*/
int addTopologicalPoints( const QgsGeometry *geom );
int addTopologicalPoints( const QgsGeometry& geom );

/** Adds a vertex to segments which intersect point p but don't
* already have a vertex there. If a feature already has a vertex at position p,
Expand Down
2 changes: 1 addition & 1 deletion python/gui/qgshighlight.sip
View file
Expand Up @@ -4,7 +4,7 @@ class QgsHighlight : QgsMapCanvasItem
#include <qgshighlight.h>
%End
public:
QgsHighlight( QgsMapCanvas *mapCanvas, const QgsGeometry *geom, QgsVectorLayer *layer );
QgsHighlight( QgsMapCanvas *mapCanvas, const QgsGeometry& geom, QgsVectorLayer *layer );
~QgsHighlight();

/** Set line/outline to color, polygon fill to color with alpha = 63.
Expand Down
4 changes: 2 additions & 2 deletions python/plugins/db_manager/db_plugins/vlayers/data_model.py
View file
Expand Up @@ -48,7 +48,7 @@ def __init__(self, table, parent=None):
for f in self.layer.getFeatures():
a = f.attributes()
# add the geometry type
if f.geometry():
if f.hasGeometry():
a.append(QgsWKBTypes.displayString(Qgis.fromOldWkbType(f.geometry().wkbType())))
else:
a.append('None')
Expand Down Expand Up @@ -97,7 +97,7 @@ def __init__(self, db, sql, parent=None):
for f in p.getFeatures():
a = f.attributes()
if has_geometry:
if f.geometry():
if f.hasGeometry():
a += [f.geometry().exportToWkt()]
else:
a += [None]
Expand Down
8 changes: 4 additions & 4 deletions python/plugins/processing/algs/qgis/Buffer.py
View file
Expand Up @@ -56,8 +56,8 @@ def buffering(progress, writer, distance, field, useField, layer, dissolve,
else:
value = distance

inGeom = QgsGeometry(inFeat.geometry())
if inGeom.isGeosEmpty():
inGeom = inFeat.geometry()
if inGeom.isEmpty() or inGeom.isGeosEmpty():
ProcessingLog.addToLog(ProcessingLog.LOG_WARNING, 'Feature {} has empty geometry. Skipping...'.format(inFeat.id()))
continue
if not inGeom.isGeosValid():
Expand All @@ -84,8 +84,8 @@ def buffering(progress, writer, distance, field, useField, layer, dissolve,
value = attrs[field]
else:
value = distance
inGeom = QgsGeometry(inFeat.geometry())
if inGeom.isGeosEmpty():
inGeom = inFeat.geometry()
if inGeom.isEmpty() or inGeom.isGeosEmpty():
ProcessingLog.addToLog(ProcessingLog.LOG_WARNING, 'Feature {} has empty geometry. Skipping...'.format(inFeat.id()))
continue
if not inGeom.isGeosValid():
Expand Down
2 changes: 1 addition & 1 deletion python/plugins/processing/algs/qgis/Centroids.py
View file
Expand Up @@ -75,7 +75,7 @@ def processAlgorithm(self, progress):
inGeom = feat.geometry()
attrs = feat.attributes()

if not inGeom:
if inGeom.isEmpty():
outGeom = QgsGeometry(None)
else:
outGeom = QgsGeometry(inGeom.centroid())
Expand Down
4 changes: 2 additions & 2 deletions python/plugins/processing/algs/qgis/CheckValidity.py
View file
Expand Up @@ -135,11 +135,11 @@ def doCheck(self, progress):
features = vector.features(layer)
total = 100.0 / len(features)
for current, inFeat in enumerate(features):
geom = QgsGeometry(inFeat.geometry())
geom = inFeat.geometry()
attrs = inFeat.attributes()

valid = True
if not geom.isGeosEmpty():
if not geom.isEmpty() and not geom.isGeosEmpty():
errors = list(geom.validateGeometry())
if errors:
# QGIS method return a summary at the end
Expand Down
10 changes: 5 additions & 5 deletions python/plugins/processing/algs/qgis/Clip.py
View file
Expand Up @@ -81,7 +81,7 @@ def processAlgorithm(self, progress):
total = 100.0 / len(selectionA)

for current, inFeatA in enumerate(selectionA):
geom = QgsGeometry(inFeatA.geometry())
geom = inFeatA.geometry()
attrs = inFeatA.attributes()
intersects = index.intersects(geom.boundingBox())
first = True
Expand All @@ -91,16 +91,16 @@ def processAlgorithm(self, progress):
layerB.getFeatures(
QgsFeatureRequest().setFilterFid(i)).nextFeature(
inFeatB)
tmpGeom = QgsGeometry(inFeatB.geometry())
tmpGeom = inFeatB.geometry()
if tmpGeom.intersects(geom):
found = True
if first:
outFeat.setGeometry(QgsGeometry(tmpGeom))
first = False
else:
cur_geom = QgsGeometry(outFeat.geometry())
cur_geom = outFeat.geometry()
new_geom = QgsGeometry(cur_geom.combine(tmpGeom))
if new_geom.isGeosEmpty() or not new_geom.isGeosValid():
if new_geom.isEmpty() or new_geom.isGeosEmpty() or not new_geom.isGeosValid():
ProcessingLog.addToLog(ProcessingLog.LOG_ERROR,
self.tr('GEOS geoprocessing error: One or '
'more input features have invalid '
Expand All @@ -109,7 +109,7 @@ def processAlgorithm(self, progress):

outFeat.setGeometry(QgsGeometry(new_geom))
if found:
cur_geom = QgsGeometry(outFeat.geometry())
cur_geom = outFeat.geometry()
new_geom = QgsGeometry(geom.intersection(cur_geom))
if new_geom.wkbType() == Qgis.WKBUnknown or QgsWKBTypes.flatType(new_geom.geometry().wkbType()) == QgsWKBTypes.GeometryCollection:
int_com = QgsGeometry(geom.combine(cur_geom))
Expand Down
4 changes: 2 additions & 2 deletions python/plugins/processing/algs/qgis/ConvexHull.py
View file
Expand Up @@ -121,7 +121,7 @@ def processAlgorithm(self, progress):
val = idVar
first = False

inGeom = QgsGeometry(f.geometry())
inGeom = f.geometry()
points = vector.extractPoints(inGeom)
hull.extend(points)
current += 1
Expand All @@ -144,7 +144,7 @@ def processAlgorithm(self, progress):
total = 100.0 / layer.featureCount()
features = vector.features(layer)
for current, f in enumerate(features):
inGeom = QgsGeometry(f.geometry())
inGeom = f.geometry()
points = vector.extractPoints(inGeom)
hull.extend(points)
progress.setPercentage(int(current * total))
Expand Down
4 changes: 2 additions & 2 deletions python/plugins/processing/algs/qgis/Delaunay.py
View file
Expand Up @@ -78,7 +78,7 @@ def processAlgorithm(self, progress):
features = vector.features(layer)
total = 100.0 / len(features)
for current, inFeat in enumerate(features):
geom = QgsGeometry(inFeat.geometry())
geom = inFeat.geometry()
point = geom.asPoint()
x = point.x()
y = point.y()
Expand Down Expand Up @@ -110,7 +110,7 @@ def processAlgorithm(self, progress):
for index in indicies:
request = QgsFeatureRequest().setFilterFid(ptDict[ids[index]])
inFeat = layer.getFeatures(request).next()
geom = QgsGeometry(inFeat.geometry())
geom = inFeat.geometry()
point = QgsPoint(geom.asPoint())
polygon.append(point)
if step <= 3:
Expand Down
2 changes: 1 addition & 1 deletion python/plugins/processing/algs/qgis/DeleteDuplicateGeometries.py
View file
Expand Up @@ -59,7 +59,7 @@ def processAlgorithm(self, progress):
total = 100.0 / len(features)
geoms = dict()
for current, f in enumerate(features):
geoms[f.id()] = QgsGeometry(f.geometry())
geoms[f.id()] = f.geometry()
progress.setPercentage(int(current * total))

cleaned = dict(geoms)
Expand Down
2 changes: 1 addition & 1 deletion python/plugins/processing/algs/qgis/DeleteHoles.py
View file
Expand Up @@ -59,7 +59,7 @@ def processAlgorithm(self, progress):
feat = QgsFeature()
for current, f in enumerate(features):
geometry = f.geometry()
if geometry:
if not geometry.isEmpty():
if geometry.isMultipart():
multi_polygon = geometry.asMultiPolygon()
for polygon in multi_polygon:
Expand Down

0 comments on commit bd7d913

Please sign in to comment.