Responsibility for managing obstacle geometry is in QgsLabelObstacleS…

…ettings
qgis · Dec 7, 2019 · 7c59700 · 7c59700
1 parent 7f38c41
commit 7c59700
Show file tree

Hide file tree

Showing 10 changed files with 85 additions and 63 deletions.
diff --git a/python/core/auto_generated/qgslabelobstaclesettings.sip.in b/python/core/auto_generated/qgslabelobstaclesettings.sip.in
@@ -96,6 +96,23 @@ Controls how features act as obstacles for labels.
 .. seealso:: :py:func:`isObstacle`
 
 .. seealso:: :py:func:`factor`
+%End
+
+    void setObstacleGeometry( const QgsGeometry &obstacleGeom );
+%Docstring
+Sets the label's obstacle geometry, if different to the feature geometry.
+This can be used to override the shape of the feature for obstacle detection, e.g., to
+buffer around a point geometry to prevent labels being placed too close to the
+point itself. It not set, the feature's geometry is used for obstacle detection.
+
+.. seealso:: :py:func:`obstacleGeometry`
+%End
+
+    QgsGeometry obstacleGeometry() const;
+%Docstring
+Returns the label's obstacle geometry, if different to the feature geometry.
+
+.. seealso:: :py:func:`setObstacleGeometry`
 %End
 
     void updateDataDefinedProperties( const QgsPropertyCollection &properties, QgsExpressionContext &context );

diff --git a/src/core/pal/layer.cpp b/src/core/pal/layer.cpp
@@ -114,7 +114,7 @@ bool Layer::registerFeature( QgsLabelFeature *lf )
 
   GEOSContextHandle_t geosctxt = QgsGeos::getGEOSHandler();
 
-  bool featureGeomIsObstacleGeom = !lf->obstacleGeometry();
+  bool featureGeomIsObstacleGeom = lf->obstacleSettings().obstacleGeometry().isNull();
 
   while ( !simpleGeometries->isEmpty() )
   {
@@ -195,12 +195,15 @@ bool Layer::registerFeature( QgsLabelFeature *lf )
   if ( lf->obstacleSettings().isObstacle() && !featureGeomIsObstacleGeom )
   {
     //do the same for the obstacle geometry
-    simpleGeometries.reset( Util::unmulti( lf->obstacleGeometry() ) );
+    geos::unique_ptr obstacleGeom( QgsGeos::asGeos( lf->obstacleSettings().obstacleGeometry() ) );
+    simpleGeometries.reset( Util::unmulti( obstacleGeom.get() ) );
     if ( !simpleGeometries ) // unmulti() failed?
     {
       throw InternalException::UnknownGeometry();
     }
 
+    mGeosObstacleGeometries.emplace_back( std::move( obstacleGeom ) );
+
     while ( !simpleGeometries->isEmpty() )
     {
       const GEOSGeometry *geom = simpleGeometries->takeFirst();
@@ -448,7 +451,7 @@ void Layer::chopFeaturesAtRepeatDistance()
       // Walk along line
       unsigned int cur = 0;
       double lambda = 0;
-      QVector<Point> part;
+      std::vector<Point> part;
 
       QList<FeaturePart *> repeatParts;
       repeatParts.reserve( possibleSegments );
@@ -463,8 +466,8 @@ void Layer::chopFeaturesAtRepeatDistance()
         if ( cur >= n )
         {
           // Create final part
-          GEOSCoordSequence *cooSeq = GEOSCoordSeq_create_r( geosctxt, part.size(), 2 );
-          for ( int i = 0; i < part.size(); ++i )
+          GEOSCoordSequence *cooSeq = GEOSCoordSeq_create_r( geosctxt, static_cast< unsigned int >( part.size() ), 2 );
+          for ( unsigned int i = 0; i < part.size(); ++i )
           {
 #if GEOS_VERSION_MAJOR>3 || GEOS_VERSION_MINOR>=8
             GEOSCoordSeq_setXY_r( geosctxt, cooSeq, i, part[i].x, part[i].y );
@@ -487,14 +490,14 @@ void Layer::chopFeaturesAtRepeatDistance()
         p.x = points[cur - 1].x + c * ( points[cur].x - points[cur - 1].x );
         p.y = points[cur - 1].y + c * ( points[cur].y - points[cur - 1].y );
         part.push_back( p );
-        GEOSCoordSequence *cooSeq = GEOSCoordSeq_create_r( geosctxt, part.size(), 2 );
-        for ( int i = 0; i < part.size(); ++i )
+        GEOSCoordSequence *cooSeq = GEOSCoordSeq_create_r( geosctxt, static_cast< unsigned int >( part.size() ), 2 );
+        for ( std::size_t i = 0; i < part.size(); ++i )
         {
 #if GEOS_VERSION_MAJOR>3 || GEOS_VERSION_MINOR>=8
           GEOSCoordSeq_setXY_r( geosctxt, cooSeq, i, part[i].x, part[i].y );
 #else
-          GEOSCoordSeq_setX_r( geosctxt, cooSeq, i, part[i].x );
-          GEOSCoordSeq_setY_r( geosctxt, cooSeq, i, part[i].y );
+          GEOSCoordSeq_setX_r( geosctxt, cooSeq, static_cast< unsigned int >( i ), part[i].x );
+          GEOSCoordSeq_setY_r( geosctxt, cooSeq, static_cast< unsigned int >( i ), part[i].y );
 #endif
         }
 

diff --git a/src/core/pal/layer.h b/src/core/pal/layer.h
@@ -36,6 +36,7 @@
 #include "qgis_core.h"
 #include "pal.h" // for LineArrangementFlags enum
 #include "rtree.hpp"
+#include "qgsgeos.h"
 #include <QMutex>
 #include <QLinkedList>
 #include <QHash>
@@ -329,6 +330,8 @@ namespace pal
       //! List of obstacle parts
       QList<FeaturePart *> mObstacleParts;
 
+      std::vector< geos::unique_ptr > mGeosObstacleGeometries;
+
       Pal *pal = nullptr;
 
       double mDefaultPriority;

diff --git a/src/core/qgslabelfeature.cpp b/src/core/qgslabelfeature.cpp
@@ -45,11 +45,6 @@ QgsLabelFeature::~QgsLabelFeature()
   delete mInfo;
 }
 
-void QgsLabelFeature::setObstacleGeometry( geos::unique_ptr obstacleGeom )
-{
-  mObstacleGeometry = std::move( obstacleGeom );
-}
-
 void QgsLabelFeature::setPermissibleZone( const QgsGeometry &geometry )
 {
   mPermissibleZone = geometry;

diff --git a/src/core/qgslabelfeature.h b/src/core/qgslabelfeature.h
@@ -67,24 +67,6 @@ class CORE_EXPORT QgsLabelFeature
     //! Gets access to the associated geometry
     GEOSGeometry *geometry() const { return mGeometry.get(); }
 
-    /**
-     * Sets the label's obstacle geometry, if different to the feature geometry.
-     * This can be used to override the shape of the feature for obstacle detection, e.g., to
-     * buffer around a point geometry to prevent labels being placed too close to the
-     * point itself. It not set, the feature's geometry is used for obstacle detection.
-     * Ownership of obstacle geometry is transferred.
-     * \see obstacleGeometry()
-     * \since QGIS 2.14
-     */
-    void setObstacleGeometry( geos::unique_ptr obstacleGeom );
-
-    /**
-     * Returns the label's obstacle geometry, if different to the feature geometry.
-     * \see setObstacleGeometry()
-     * \since QGIS 2.14
-     */
-    GEOSGeometry *obstacleGeometry() const { return mObstacleGeometry.get(); }
-
     /**
      * Sets the label's permissible zone geometry. If set, the feature's label MUST be fully contained
      * within this zone, and the feature will not be labeled if no candidates can be generated which
@@ -460,8 +442,6 @@ class CORE_EXPORT QgsLabelFeature
     QgsFeatureId mId;
     //! Geometry of the feature to be labelled
     geos::unique_ptr mGeometry;
-    //! Optional geometry to use for label obstacles, if different to mGeometry
-    geos::unique_ptr mObstacleGeometry;
     //! Optional geometry to use for label's permissible zone
     QgsGeometry mPermissibleZone;
     //! Width and height of the label

diff --git a/src/core/qgslabelobstaclesettings.cpp b/src/core/qgslabelobstaclesettings.cpp
@@ -18,6 +18,16 @@
 #include "qgsexpressioncontext.h"
 #include "qgspallabeling.h"
 
+void QgsLabelObstacleSettings::setObstacleGeometry( const QgsGeometry &obstacleGeom )
+{
+  mObstacleGeometry = obstacleGeom;
+}
+
+QgsGeometry QgsLabelObstacleSettings::obstacleGeometry() const
+{
+  return mObstacleGeometry;
+}
+
 void QgsLabelObstacleSettings::updateDataDefinedProperties( const QgsPropertyCollection &properties, QgsExpressionContext &context )
 {
   if ( properties.isActive( QgsPalLayerSettings::ObstacleFactor ) )

diff --git a/src/core/qgslabelobstaclesettings.h b/src/core/qgslabelobstaclesettings.h
@@ -18,6 +18,7 @@
 
 #include "qgis_core.h"
 #include "qgis_sip.h"
+#include "qgsgeos.h"
 
 class QgsPropertyCollection;
 class QgsExpressionContext;
@@ -120,6 +121,22 @@ class CORE_EXPORT QgsLabelObstacleSettings
       mObstacleType = type;
     }
 
+    /**
+     * Sets the label's obstacle geometry, if different to the feature geometry.
+     * This can be used to override the shape of the feature for obstacle detection, e.g., to
+     * buffer around a point geometry to prevent labels being placed too close to the
+     * point itself. It not set, the feature's geometry is used for obstacle detection.
+     *
+     * \see obstacleGeometry()
+     */
+    void setObstacleGeometry( const QgsGeometry &obstacleGeom );
+
+    /**
+     * Returns the label's obstacle geometry, if different to the feature geometry.
+     * \see setObstacleGeometry()
+     */
+    QgsGeometry obstacleGeometry() const;
+
     /**
      * Updates the obstacle settings to respect any data defined properties
      * set within the specified \a properties collection.
@@ -132,6 +149,9 @@ class CORE_EXPORT QgsLabelObstacleSettings
     double mObstacleFactor = 1.0;
     ObstacleType mObstacleType = PolygonBoundary;
 
+    //! Optional geometry to use for label obstacles
+    QgsGeometry mObstacleGeometry;
+
 };
 
 #endif // QGSLABELOBSTACLESETTINGS_H
diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp
@@ -2003,13 +2003,6 @@ void QgsPalLayerSettings::registerFeature( const QgsFeature &f, QgsRenderContext
     }
   }
 
-  geos::unique_ptr geosObstacleGeomClone;
-  if ( isObstacle && !obstacleGeometry.isNull() )
-  {
-    geosObstacleGeomClone = QgsGeos::asGeos( obstacleGeometry );
-  }
-
-
   //data defined position / alignment / rotation?
   bool dataDefinedPosition = false;
   bool layerDefinedRotation = false;
@@ -2359,16 +2352,11 @@ void QgsPalLayerSettings::registerFeature( const QgsFeature &f, QgsRenderContext
   ( *labelFeature )->setOverrunDistance( overrunDistanceEval );
   ( *labelFeature )->setOverrunSmoothDistance( overrunSmoothDist );
   ( *labelFeature )->setLabelAllParts( labelAll );
-  if ( geosObstacleGeomClone )
+  if ( geom.type() == QgsWkbTypes::PointGeometry && isObstacle && !obstacleGeometry.isNull() )
   {
-    ( *labelFeature )->setObstacleGeometry( std::move( geosObstacleGeomClone ) );
-
-    if ( geom.type() == QgsWkbTypes::PointGeometry )
-    {
-      //register symbol size
-      ( *labelFeature )->setSymbolSize( QSizeF( obstacleGeometry.boundingBox().width(),
-                                        obstacleGeometry.boundingBox().height() ) );
-    }
+    //register symbol size
+    ( *labelFeature )->setSymbolSize( QSizeF( obstacleGeometry.boundingBox().width(),
+                                      obstacleGeometry.boundingBox().height() ) );
   }
 
   //set label's visual margin so that top visual margin is the leading, and bottom margin is the font's descent
@@ -2481,6 +2469,7 @@ void QgsPalLayerSettings::registerFeature( const QgsFeature &f, QgsRenderContext
   QgsLabelObstacleSettings os = mObstacleSettings;
   os.setIsObstacle( isObstacle );
   os.updateDataDefinedProperties( mDataDefinedProperties, context.expressionContext() );
+  os.setObstacleGeometry( obstacleGeometry );
   lf->setObstacleSettings( os );
 
   QVector< QgsPalLayerSettings::PredefinedPointPosition > positionOrder = predefinedPositionOrder;

diff --git a/src/core/qgsvectorlayerdiagramprovider.cpp b/src/core/qgsvectorlayerdiagramprovider.cpp
@@ -232,16 +232,14 @@ QgsLabelFeature *QgsVectorLayerDiagramProvider::registerDiagram( QgsFeature &fea
   if ( !geomCopy )
     return nullptr; // invalid geometry
 
-  geos::unique_ptr geosObstacleGeomClone;
-  std::unique_ptr<QgsGeometry> scopedObstacleGeom;
+  QgsGeometry preparedObstacleGeom;
   if ( isObstacle && !obstacleGeometry.isNull() && QgsPalLabeling::geometryRequiresPreparation( obstacleGeometry, context, mSettings.coordinateTransform(), extentGeom ) )
   {
-    QgsGeometry preparedObstacleGeom = QgsPalLabeling::prepareGeometry( obstacleGeometry, context, mSettings.coordinateTransform(), extentGeom );
-    geosObstacleGeomClone = QgsGeos::asGeos( preparedObstacleGeom );
+    preparedObstacleGeom = QgsPalLabeling::prepareGeometry( obstacleGeometry, context, mSettings.coordinateTransform(), extentGeom );
   }
   else if ( mSettings.isObstacle() && !obstacleGeometry.isNull() )
   {
-    geosObstacleGeomClone = QgsGeos::asGeos( obstacleGeometry );
+    preparedObstacleGeom = obstacleGeometry;
   }
 
   double diagramWidth = 0;
@@ -297,11 +295,8 @@ QgsLabelFeature *QgsVectorLayerDiagramProvider::registerDiagram( QgsFeature &fea
   lf->setAlwaysShow( alwaysShow );
   QgsLabelObstacleSettings os;
   os.setIsObstacle( isObstacle );
+  os.setObstacleGeometry( preparedObstacleGeom );
   lf->setObstacleSettings( os );
-  if ( geosObstacleGeomClone )
-  {
-    lf->setObstacleGeometry( std::move( geosObstacleGeomClone ) );
-  }
 
   if ( dr )
   {

diff --git a/tests/src/python/test_qgslabelobstaclesettings.py b/tests/src/python/test_qgslabelobstaclesettings.py
@@ -17,9 +17,12 @@
                        QgsPalLayerSettings,
                        QgsLabelObstacleSettings,
                        QgsExpressionContext,
-                       QgsExpressionContextScope)
+                       QgsExpressionContextScope,
+                       QgsGeometry)
 
-from qgis.testing import unittest
+from qgis.testing import unittest, start_app
+
+start_app()
 
 
 class TestQgsLabelObstacleSettings(unittest.TestCase):
@@ -67,8 +70,15 @@ def testUpdateDataDefinedProps(self):
         context = QgsExpressionContext()
         scope = QgsExpressionContextScope()
         scope.setVariable('factor', 9)
-        props.updateDataDefinedProperties(props, context)
-        self.assertEqual(settings.factor(), 0.1)
+        context.appendScope(scope)
+        settings.updateDataDefinedProperties(props, context)
+        self.assertAlmostEqual(settings.factor(), 1.8, 3)
+
+    def testObstacleGeom(self):
+        settings = QgsLabelObstacleSettings()
+        self.assertTrue(settings.obstacleGeometry().isNull())
+        settings.setObstacleGeometry(QgsGeometry.fromWkt('LineString( 0 0, 1 1)'))
+        self.assertEqual(settings.obstacleGeometry().asWkt(), 'LineString (0 0, 1 1)')
 
 
 if __name__ == '__main__':