Merge pull request #5352 from nyalldawson/geom

Geometry cleanups and modernization
qgis · Oct 13, 2017 · b91b854 · b91b854
2 parents b250729 + 947b0cc
commit b91b854
Show file tree

Hide file tree

Showing 60 changed files with 656 additions and 609 deletions.
diff --git a/doc/api_break.dox b/doc/api_break.dox
@@ -1288,6 +1288,8 @@ QgsGeometryEngine        {#qgis_api_break_3_0_QgsGeometryEngine}
 - `QgsAbstractGeometry&` parameters have been changed to `QgsAbstractGeometry*` (Affects C++ only)
 - `centroid()` returns the point instead of working on a parameter. The return value is a `nullptr` when `false` has been returned in the past.
 - `pointOnSurface()` returns the point instead of working on a parameter. The return value is a `nullptr` when `false` has been returned in the past.
+- splitGeometry() now returns new geometries as QgsGeometry, instead of QgsAbstractGeometry
+- combine() now requires a list of QgsGeometry, instead of QgsAbstractGeometry
 
 
 QgsGeometrySimplifier        {#qgis_api_break_3_0_QgsGeometrySimplifier}

diff --git a/python/core/geometry/qgsgeometry.sip b/python/core/geometry/qgsgeometry.sip
@@ -1170,7 +1170,7 @@ Returns an extruded version of this geometry.
  :rtype: int
 %End
 
-    QgsGeometry makeValid();
+    QgsGeometry makeValid() const;
 %Docstring
  Attempts to make an invalid geometry valid without losing vertices.
 
@@ -1225,7 +1225,7 @@ Returns an extruded version of this geometry.
       ValidatorGeos,
     };
 
-    void validateGeometry( QList<QgsGeometry::Error> &errors /Out/, ValidationMethod method = ValidatorQgisInternal );
+    void validateGeometry( QList<QgsGeometry::Error> &errors /Out/, ValidationMethod method = ValidatorQgisInternal ) const;
 %Docstring
  Validates geometry and produces a list of geometry errors.
  The ``method`` argument dictates which validator to utilize.

diff --git a/python/core/geometry/qgsgeometryengine.sip b/python/core/geometry/qgsgeometryengine.sip
@@ -37,7 +37,21 @@ class QgsGeometryEngine
     virtual ~QgsGeometryEngine();
 
     virtual void geometryChanged() = 0;
+%Docstring
+ Should be called whenever the geometry associated with the engine
+ has been modified and the engine must be updated to suit.
+%End
+
     virtual void prepareGeometry() = 0;
+%Docstring
+ Prepares the geometry, so that subsequent calls to spatial relation methods
+ are much faster.
+
+ This should be called for any geometry which is used for multiple relation
+ tests against other geometries.
+
+.. seealso:: geometryChanged()
+%End
 
     virtual QgsAbstractGeometry *intersection( const QgsAbstractGeometry *geom, QString *errorMsg = 0 ) const = 0 /Factory/;
 %Docstring
@@ -63,7 +77,7 @@ class QgsGeometryEngine
  :rtype: QgsAbstractGeometry
 %End
 
-    virtual QgsAbstractGeometry *combine( const QList< QgsAbstractGeometry * > &geometries, QString *errorMsg = 0 ) const = 0 /Factory/;
+    virtual QgsAbstractGeometry *combine( const QList< QgsGeometry > &geometries, QString *errorMsg = 0 ) const = 0 /Factory/;
 %Docstring
  Calculate the combination of this and ``geometries``.
 
@@ -244,10 +258,17 @@ class QgsGeometryEngine
 %End
 
     virtual QgsGeometryEngine::EngineOperationResult splitGeometry( const QgsLineString &splitLine,
-        QList<QgsAbstractGeometry *> &newGeometries,
+        QList<QgsGeometry > &newGeometries /Out/,
         bool topological,
         QgsPointSequence &topologyTestPoints, QString *errorMsg = 0 ) const;
 %Docstring
+ Splits this geometry according to a given line.
+ \param splitLine the line that splits the geometry
+ \param[out] newGeometries list of new geometries that have been created with the split
+ \param topological true if topological editing is enabled
+ \param[out] topologyTestPoints points that need to be tested for topological completeness in the dataset
+ \param[out] errorMsg error messages emitted, if any
+ :return: 0 in case of success, 1 if geometry has not been split, error else
  :rtype: QgsGeometryEngine.EngineOperationResult
 %End
 

diff --git a/src/app/layout/qgslayoutappmenuprovider.cpp b/src/app/layout/qgslayoutappmenuprovider.cpp
@@ -55,7 +55,7 @@ QMenu *QgsLayoutAppMenuProvider::createContextMenu( QWidget *parent, QgsLayout *
     bool foundSelectedGroup = false;
     QList< QgsLayoutItemGroup * > groups;
     layout->layoutItems( groups );
-    for ( QgsLayoutItemGroup *group : qgsAsConst( groups ) )
+    for ( QgsLayoutItemGroup *group : qgis::as_const( groups ) )
     {
       if ( group->isSelected() )
       {

diff --git a/src/app/main.cpp b/src/app/main.cpp
@@ -1166,7 +1166,7 @@ int main( int argc, char *argv[] )
   /////////////////////////////////////////////////////////////////////
   // autoload any file names that were passed in on the command line
   /////////////////////////////////////////////////////////////////////
-  for ( const QString &layerName : qgsAsConst( sFileList ) )
+  for ( const QString &layerName : qgis::as_const( sFileList ) )
   {
     QgsDebugMsg( QString( "Trying to load file : %1" ).arg( layerName ) );
     // don't load anything with a .qgs extension - these are project files

diff --git a/src/app/nodetool/qgsnodetool.cpp b/src/app/nodetool/qgsnodetool.cpp
@@ -332,11 +332,11 @@ void QgsNodeTool::clearDragBands()
   mDragPointMarkers.clear();
   mDragPointMarkersOffset.clear();
 
-  for ( const StraightBand &b : qgsAsConst( mDragStraightBands ) )
+  for ( const StraightBand &b : qgis::as_const( mDragStraightBands ) )
     delete b.band;
   mDragStraightBands.clear();
 
-  for ( const CircularBand &b : qgsAsConst( mDragCircularBands ) )
+  for ( const CircularBand &b : qgis::as_const( mDragCircularBands ) )
     delete b.band;
   mDragCircularBands.clear();
 }
@@ -352,7 +352,7 @@ void QgsNodeTool::cadCanvasPressEvent( QgsMapMouseEvent *e )
     QgsPointLocator::Match m = snapToEditableLayer( e );
     if ( m.hasVertex() )
     {
-      for ( const Vertex &selectedNode : qgsAsConst( mSelectedNodes ) )
+      for ( const Vertex &selectedNode : qgis::as_const( mSelectedNodes ) )
       {
         if ( selectedNode.layer == m.layer() && selectedNode.fid == m.featureId() && selectedNode.vertexId == m.vertexIndex() )
         {
@@ -783,7 +783,7 @@ void QgsNodeTool::mouseMoveNotDragging( QgsMapMouseEvent *e )
       QgsPointSequence points;
       QgsGeometryUtils::segmentizeArc( QgsPoint( pX ), QgsPoint( p0 ), QgsPoint( p1 ), points );
       mEdgeBand->reset();
-      for ( const QgsPoint &pt : qgsAsConst( points ) )
+      for ( const QgsPoint &pt : qgis::as_const( points ) )
         mEdgeBand->addPoint( pt );
     }
     else if ( isCircularVertex( geom, m.vertexIndex() + 1 ) )
@@ -794,7 +794,7 @@ void QgsNodeTool::mouseMoveNotDragging( QgsMapMouseEvent *e )
       QgsPointSequence points;
       QgsGeometryUtils::segmentizeArc( QgsPoint( p0 ), QgsPoint( p1 ), QgsPoint( pX ), points );
       mEdgeBand->reset();
-      for ( const QgsPoint &pt : qgsAsConst( points ) )
+      for ( const QgsPoint &pt : qgis::as_const( points ) )
         mEdgeBand->addPoint( pt );
     }
     else
@@ -974,7 +974,7 @@ void QgsNodeTool::deleteNodeEditorSelection()
   QgsVectorLayer *layer = mSelectedFeature->vlayer();
   QgsFeatureId fid = mSelectedFeature->featureId();
   QgsGeometry geometry = cachedGeometry( layer, fid );
-  for ( QgsVertexEntry *vertex : qgsAsConst( selFeatureVertices ) )
+  for ( QgsVertexEntry *vertex : qgis::as_const( selFeatureVertices ) )
   {
     if ( vertex->isSelected() )
     {
@@ -1053,7 +1053,7 @@ void QgsNodeTool::startDraggingMoveVertex( const QgsPointLocator::Match &m )
   QgsPointXY origDraggingVertexPoint = geom.vertexAt( mDraggingVertex->vertexId );
 
   // if there are other highlighted nodes, they should be dragged as well with their offset
-  for ( const Vertex &v : qgsAsConst( mSelectedNodes ) )
+  for ( const Vertex &v : qgis::as_const( mSelectedNodes ) )
   {
     if ( v != *mDraggingVertex )
     {
@@ -1100,7 +1100,7 @@ void QgsNodeTool::startDraggingMoveVertex( const QgsPointLocator::Match &m )
 
   QSet<Vertex> movingVertices;
   movingVertices << *mDraggingVertex;
-  for ( const Vertex &v : qgsAsConst( mDraggingExtraVertices ) )
+  for ( const Vertex &v : qgis::as_const( mDraggingExtraVertices ) )
     movingVertices << v;
 
   QgsPointXY dragVertexMapPoint = m.point();
@@ -1116,7 +1116,7 @@ void QgsNodeTool::buildDragBandsForVertices( const QSet<Vertex> &movingVertices,
   // i.e. every circular band is defined by its middle circular vertex
   QSet<Vertex> verticesInCircularBands;
 
-  for ( const Vertex &v : qgsAsConst( movingVertices ) )
+  for ( const Vertex &v : qgis::as_const( movingVertices ) )
   {
     int v0idx, v1idx;
     QgsGeometry geom = cachedGeometry( v.layer, v.fid );
@@ -1328,7 +1328,7 @@ void QgsNodeTool::startDraggingEdge( const QgsPointLocator::Match &m, const QgsP
 
   QgsPointXY layerPoint = toLayerCoordinates( m.layer(), mapPoint );
 
-  for ( const Vertex &v : qgsAsConst( movingVertices ) )
+  for ( const Vertex &v : qgis::as_const( movingVertices ) )
   {
     mDraggingExtraVertices << v;
     mDraggingExtraVerticesOffset << ( geom.vertexAt( v.vertexId ) - QgsPoint( layerPoint ) );
@@ -1535,7 +1535,7 @@ void QgsNodeTool::deleteVertex()
   {
     // if topo editing is enabled, delete all the vertices that are on the same location
     QSet<Vertex> topoVerticesToDelete;
-    for ( const Vertex &vertexToDelete : qgsAsConst( toDelete ) )
+    for ( const Vertex &vertexToDelete : qgis::as_const( toDelete ) )
     {
       QgsPointXY layerPt = cachedGeometryForVertex( vertexToDelete ).vertexAt( vertexToDelete.vertexId );
       QgsPointXY mapPt = toMapCoordinates( vertexToDelete.layer, layerPt );
@@ -1555,7 +1555,7 @@ void QgsNodeTool::deleteVertex()
 
   // switch from a plain list to dictionary { layer: { fid: [vertexNr1, vertexNr2, ...] } }
   QHash<QgsVectorLayer *, QHash<QgsFeatureId, QList<int> > > toDeleteGrouped;
-  for ( const Vertex &vertex : qgsAsConst( toDelete ) )
+  for ( const Vertex &vertex : qgis::as_const( toDelete ) )
   {
     toDeleteGrouped[vertex.layer][vertex.fid].append( vertex.vertexId );
   }
@@ -1591,7 +1591,7 @@ void QgsNodeTool::deleteVertex()
           }
         }
         // now delete the duplicities
-        for ( int duplicateVertexIndex : qgsAsConst( duplicateVertexIndices ) )
+        for ( int duplicateVertexIndex : qgis::as_const( duplicateVertexIndices ) )
           vertexIds.removeOne( duplicateVertexIndex );
       }
     }
@@ -1670,7 +1670,7 @@ void QgsNodeTool::setHighlightedNodes( QList<Vertex> listNodes )
   mSelectedNodesMarkers.clear();
   mSelectedNodes.clear();
 
-  for ( const Vertex &node : qgsAsConst( listNodes ) )
+  for ( const Vertex &node : qgis::as_const( listNodes ) )
   {
     QgsGeometry geom = cachedGeometryForVertex( node );
     QgsVertexId vid;
@@ -1689,7 +1689,7 @@ void QgsNodeTool::setHighlightedNodes( QList<Vertex> listNodes )
 
 void QgsNodeTool::setHighlightedNodesVisible( bool visible )
 {
-  for ( QgsVertexMarker *marker : qgsAsConst( mSelectedNodesMarkers ) )
+  for ( QgsVertexMarker *marker : qgis::as_const( mSelectedNodesMarkers ) )
     marker->setVisible( visible );
 }
 
@@ -1784,7 +1784,7 @@ void QgsNodeTool::CircularBand::updateRubberBand( const QgsPointXY &mapPoint )
   QgsGeometryUtils::segmentizeArc( QgsPoint( v0 ), QgsPoint( v1 ), QgsPoint( v2 ), points );
   // it would be useful to have QgsRubberBand::setPoints() call
   band->reset();
-  for ( const QgsPoint &p : qgsAsConst( points ) )
+  for ( const QgsPoint &p : qgis::as_const( points ) )
     band->addPoint( p );
 }
 

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -1423,7 +1423,7 @@ void QgisApp::dropEvent( QDropEvent *event )
   {
     freezeCanvases();
 
-    for ( const QString &file : qgsAsConst( files ) )
+    for ( const QString &file : qgis::as_const( files ) )
     {
       bool handled = false;
 
@@ -7926,7 +7926,7 @@ void QgisApp::editPaste( QgsMapLayer *destinationLayer )
 
   pasteVectorLayer->addFeatures( newFeatures );
   QgsFeatureIds newIds;
-  for ( const QgsFeature &f : qgsAsConst( newFeatures ) )
+  for ( const QgsFeature &f : qgis::as_const( newFeatures ) )
   {
     newIds << f.id();
   }

diff --git a/src/app/qgsmaptoollabel.cpp b/src/app/qgsmaptoollabel.cpp
@@ -732,7 +732,7 @@ bool QgsMapToolLabel::createAuxiliaryFields( LabelDetails &details, QgsPalIndexe
   if ( !vlayer->auxiliaryLayer() )
     return false;
 
-  for ( const QgsPalLayerSettings::Property &p : qgsAsConst( mPalProperties ) )
+  for ( const QgsPalLayerSettings::Property &p : qgis::as_const( mPalProperties ) )
   {
     int index = -1;
 
@@ -779,7 +779,7 @@ bool QgsMapToolLabel::createAuxiliaryFields( LabelDetails &details, QgsDiagramIn
   if ( !vlayer->auxiliaryLayer() )
     return false;
 
-  for ( const QgsDiagramLayerSettings::Property &p : qgsAsConst( mDiagramProperties ) )
+  for ( const QgsDiagramLayerSettings::Property &p : qgis::as_const( mDiagramProperties ) )
   {
     int index = -1;
 

diff --git a/src/app/qgsmaptoolshowhidelabels.cpp b/src/app/qgsmaptoolshowhidelabels.cpp
@@ -142,7 +142,7 @@ void QgsMapToolShowHideLabels::showHideLabels( QMouseEvent *e )
     QList<QgsLabelPosition> positions;
     if ( selectedLabelFeatures( vlayer, positions ) )
     {
-      for ( const QgsLabelPosition &pos : qgsAsConst( positions ) )
+      for ( const QgsLabelPosition &pos : qgis::as_const( positions ) )
       {
         if ( showHide( pos, false ) )
           labelChanged = true;
@@ -154,7 +154,7 @@ void QgsMapToolShowHideLabels::showHideLabels( QMouseEvent *e )
     QgsFeatureIds fids;
     if ( selectedFeatures( vlayer, fids ) )
     {
-      for ( const QgsFeatureId &fid : qgsAsConst( fids ) )
+      for ( const QgsFeatureId &fid : qgis::as_const( fids ) )
       {
         QgsLabelPosition pos;
         pos.featureId = fid;

diff --git a/src/app/qgsoptions.cpp b/src/app/qgsoptions.cpp
@@ -2416,7 +2416,7 @@ void QgsOptions::showHelp()
   QString link;
 
   // give first priority to created pages which have specified a help key
-  for ( const QgsOptionsPageWidget *widget : qgsAsConst( mAdditionalOptionWidgets ) )
+  for ( const QgsOptionsPageWidget *widget : qgis::as_const( mAdditionalOptionWidgets ) )
   {
     if ( widget == activeTab )
     {

diff --git a/src/core/annotations/qgsannotationmanager.cpp b/src/core/annotations/qgsannotationmanager.cpp
@@ -62,7 +62,7 @@ bool QgsAnnotationManager::removeAnnotation( QgsAnnotation *annotation )
 
 void QgsAnnotationManager::clear()
 {
-  for ( auto *a : qgsAsConst( mAnnotations ) )
+  for ( auto *a : qgis::as_const( mAnnotations ) )
   {
     removeAnnotation( a );
   }
@@ -76,7 +76,7 @@ QList<QgsAnnotation *> QgsAnnotationManager::annotations() const
 QList<QgsAnnotation *> QgsAnnotationManager::cloneAnnotations() const
 {
   QList<QgsAnnotation *> results;
-  for ( const auto *a : qgsAsConst( mAnnotations ) )
+  for ( const auto *a : qgis::as_const( mAnnotations ) )
   {
     results << a->clone();
   }

diff --git a/src/core/auth/qgsauthmanager.cpp b/src/core/auth/qgsauthmanager.cpp
@@ -2437,7 +2437,7 @@ const QList<QSslCertificate> QgsAuthManager::getExtraFileCAs()
     filecerts = QgsAuthCertUtils::certsFromFile( cafile );
   }
   // only CAs or certs capable of signing other certs are allowed
-  for ( const auto &cert : qgsAsConst( filecerts ) )
+  for ( const auto &cert : qgis::as_const( filecerts ) )
   {
     if ( !allowinvalid.toBool() && !cert.isValid() )
     {
@@ -3403,7 +3403,7 @@ bool QgsAuthManager::reencryptAllAuthenticationSettings( const QString &prevpass
   QStringList encryptedsettings;
   encryptedsettings << "";
 
-  for ( const auto & sett, qgsAsConst( encryptedsettings ) )
+  for ( const auto & sett, qgis::as_const( encryptedsettings ) )
   {
     if ( sett.isEmpty() || !existsAuthSetting( sett ) )
       continue;

diff --git a/src/core/diagram/qgshistogramdiagram.cpp b/src/core/diagram/qgshistogramdiagram.cpp
@@ -49,7 +49,7 @@ QSizeF QgsHistogramDiagram::diagramSize( const QgsFeature &feature, const QgsRen
   if ( !feature.fields().isEmpty() )
     expressionContext.setFields( feature.fields() );
 
-  for ( const QString &cat : qgsAsConst( s.categoryAttributes ) )
+  for ( const QString &cat : qgis::as_const( s.categoryAttributes ) )
   {
     QgsExpression *expression = getExpression( cat, expressionContext );
     maxValue = std::max( expression->evaluate( &expressionContext ).toDouble(), maxValue );
@@ -151,7 +151,7 @@ void QgsHistogramDiagram::renderDiagram( const QgsFeature &feature, QgsRenderCon
   if ( !feature.fields().isEmpty() )
     expressionContext.setFields( feature.fields() );
 
-  for ( const QString &cat : qgsAsConst( s.categoryAttributes ) )
+  for ( const QString &cat : qgis::as_const( s.categoryAttributes ) )
   {
     QgsExpression *expression = getExpression( cat, expressionContext );
     double currentVal = expression->evaluate( &expressionContext ).toDouble();

diff --git a/src/core/dxf/qgsdxfexport.cpp b/src/core/dxf/qgsdxfexport.cpp
@@ -802,7 +802,7 @@ void QgsDxfExport::writeTables()
   writeGroup( 6, QStringLiteral( "CONTINUOUS" ) );
   writeHandle( 390, DXF_HANDPLOTSTYLE );
 
-  for ( const QString &layerName : qgsAsConst( layerNames ) )
+  for ( const QString &layerName : qgis::as_const( layerNames ) )
   {
     writeGroup( 0, QStringLiteral( "LAYER" ) );
     writeHandle();

diff --git a/src/core/expression/qgsexpression.cpp b/src/core/expression/qgsexpression.cpp
@@ -514,7 +514,7 @@ QString QgsExpression::helpText( QString name )
                         .arg( tr( "%1 %2" ).arg( f.mType, name ),
                               f.mDescription ) );
 
-  for ( const HelpVariant &v : qgsAsConst( f.mVariants ) )
+  for ( const HelpVariant &v : qgis::as_const( f.mVariants ) )
   {
     if ( f.mVariants.size() > 1 )
     {
@@ -546,7 +546,7 @@ QString QgsExpression::helpText( QString name )
         helpContents += '(';
 
         QString delim;
-        for ( const HelpArg &a : qgsAsConst( v.mArguments ) )
+        for ( const HelpArg &a : qgis::as_const( v.mArguments ) )
         {
           helpContents += delim;
           delim = QStringLiteral( ", " );
@@ -572,7 +572,7 @@ QString QgsExpression::helpText( QString name )
     {
       helpContents += QStringLiteral( "<h4>%1</h4>\n<div class=\"arguments\">\n<table>" ).arg( tr( "Arguments" ) );
 
-      for ( const HelpArg &a : qgsAsConst( v.mArguments ) )
+      for ( const HelpArg &a : qgis::as_const( v.mArguments ) )
       {
         if ( a.mSyntaxOnly )
           continue;
@@ -587,7 +587,7 @@ QString QgsExpression::helpText( QString name )
     {
       helpContents += QStringLiteral( "<h4>%1</h4>\n<div class=\"examples\">\n<ul>\n" ).arg( tr( "Examples" ) );
 
-      for ( const HelpExample &e : qgsAsConst( v.mExamples ) )
+      for ( const HelpExample &e : qgis::as_const( v.mExamples ) )
       {
         helpContents += "<li><code>" + e.mExpression + "</code> &rarr; <code>" + e.mReturns + "</code>";