[FEATURE] Use rendered symbol size as obstacle for point feature labels

Previously, only the point feature itself was treated as an obstacle for label candidates. If a large or offset symbol was used for the point, then labels were allowed to overlap this symbol without incurring the obstacle cost. Sponsored by City of Uster
qgis · Nov 22, 2015 · 7d600bd · 7d600bd · nirvn · Nov 22, 2015
1 parent 6b0e7de
commit 7d600bd
Show file tree

Hide file tree

Showing 7 changed files with 132 additions and 14 deletions.
diff --git a/src/core/qgsvectorlayerlabelprovider.cpp b/src/core/qgsvectorlayerlabelprovider.cpp
@@ -22,6 +22,10 @@
 #include "qgspallabeling.h"
 #include "qgsvectorlayer.h"
 #include "qgsvectorlayerfeatureiterator.h"
+#include "qgsrendererv2.h"
+#include "qgspolygonv2.h"
+#include "qgslinestringv2.h"
+#include "qgsmultipolygonv2.h"
 
 #include "feature.h"
 #include "labelposition.h"
@@ -46,12 +50,13 @@ static void _fixQPictureDPI( QPainter* p )
 
 
 QgsVectorLayerLabelProvider::QgsVectorLayerLabelProvider( QgsVectorLayer* layer, bool withFeatureLoop, const QgsPalLayerSettings* settings, const QString& layerName )
+    : mSettings( settings ? *settings : QgsPalLayerSettings::fromLayer( layer ) )
+    , mLayerId( layer->id() )
+    , mRenderer( layer->rendererV2() )
+    , mFields( layer->fields() )
+    , mCrs( layer->crs() )
 {
-  mSettings = settings ? *settings : QgsPalLayerSettings::fromLayer( layer );
   mName = layerName.isEmpty() ? layer->id() : layerName;
-  mLayerId = layer->id();
-  mFields = layer->fields();
-  mCrs = layer->crs();
 
   if ( withFeatureLoop )
   {
@@ -72,9 +77,10 @@ QgsVectorLayerLabelProvider::QgsVectorLayerLabelProvider( const QgsPalLayerSetti
     const QgsFields& fields,
     const QgsCoordinateReferenceSystem& crs,
     QgsAbstractFeatureSource* source,
-    bool ownsSource )
+    bool ownsSource, QgsFeatureRendererV2* renderer )
     : mSettings( settings )
     , mLayerId( layerId )
+    , mRenderer( renderer )
     , mFields( fields )
     , mCrs( crs )
     , mSource( source )
@@ -263,7 +269,13 @@ QList<QgsLabelFeature*> QgsVectorLayerLabelProvider::labelFeatures( QgsRenderCon
   QgsFeature fet;
   while ( fit.nextFeature( fet ) )
   {
-    registerFeature( fet, ctx );
+    QScopedPointer<QgsGeometry> obstacleGeometry;
+    if ( fet.constGeometry()->type() == QGis::Point )
+    {
+      //point feature, use symbol bounds as obstacle
+      obstacleGeometry.reset( getPointObstacleGeometry( fet, ctx, mRenderer ) );
+    }
+    registerFeature( fet, ctx, obstacleGeometry.data() );
   }
 
   return mLabels;
@@ -277,6 +289,86 @@ void QgsVectorLayerLabelProvider::registerFeature( QgsFeature& feature, QgsRende
     mLabels << label;
 }
 
+QgsGeometry* QgsVectorLayerLabelProvider::getPointObstacleGeometry( QgsFeature& fet, QgsRenderContext& context, QgsFeatureRendererV2* renderer )
+{
+  if ( !fet.constGeometry() || fet.constGeometry()->isEmpty() || fet.constGeometry()->type() != QGis::Point || !renderer )
+    return 0;
+
+  //calculate bounds for symbols for feature
+  QgsSymbolV2List symbols = renderer->originalSymbolsForFeature( fet, context );
+
+  bool isMultiPoint = fet.constGeometry()->geometry()->nCoordinates() > 1;
+  QgsAbstractGeometryV2* obstacleGeom = 0;
+  if ( isMultiPoint )
+    obstacleGeom = new QgsMultiPolygonV2();
+  else
+    obstacleGeom = new QgsPolygonV2();
+
+  // for each point
+  for ( int i = 0; i < fet.constGeometry()->geometry()->nCoordinates(); ++i )
+  {
+    QRectF bounds;
+    QgsPointV2 p =  fet.constGeometry()->geometry()->vertexAt( QgsVertexId( i, 0, 0 ) );
+    double x = p.x();
+    double y = p.y();
+    double z = 0; // dummy variable for coordinate transforms
+
+    //transform point to pixels
+    if ( context.coordinateTransform() )
+    {
+      context.coordinateTransform()->transformInPlace( x, y, z );
+    }
+    context.mapToPixel().transformInPlace( x, y );
+
+    QPointF pt( x, y );
+    Q_FOREACH ( QgsSymbolV2* symbol, symbols )
+    {
+      if ( symbol->type() == QgsSymbolV2::Marker )
+      {
+        if ( bounds.isValid() )
+          bounds = bounds.united( static_cast< QgsMarkerSymbolV2* >( symbol )->bounds( pt, context ) );
+        else
+          bounds = static_cast< QgsMarkerSymbolV2* >( symbol )->bounds( pt, context );
+      }
+    }
+
+    //convert bounds to a geometry
+    QgsLineStringV2* boundLineString = new QgsLineStringV2();
+    boundLineString->addVertex( QgsPointV2( bounds.topLeft() ) );
+    boundLineString->addVertex( QgsPointV2( bounds.topRight() ) );
+    boundLineString->addVertex( QgsPointV2( bounds.bottomRight() ) );
+    boundLineString->addVertex( QgsPointV2( bounds.bottomLeft() ) );
+
+    //then transform back to map units
+    //TODO - remove when labeling is refactored to use screen units
+    for ( int i = 0; i < boundLineString->numPoints(); ++i )
+    {
+      QgsPoint point = context.mapToPixel().toMapCoordinates( boundLineString->xAt( i ), boundLineString->yAt( i ) );
+      boundLineString->setXAt( i, point.x() );
+      boundLineString->setYAt( i, point.y() );
+    }
+    if ( context.coordinateTransform() )
+    {
+      boundLineString->transform( *context.coordinateTransform(), QgsCoordinateTransform::ReverseTransform );
+    }
+    boundLineString->close();
+
+    QgsPolygonV2* obstaclePolygon = new QgsPolygonV2();
+    obstaclePolygon->setExteriorRing( boundLineString );
+
+    if ( isMultiPoint )
+    {
+      static_cast<QgsMultiPolygonV2*>( obstacleGeom )->addGeometry( obstaclePolygon );
+    }
+    else
+    {
+      obstacleGeom = obstaclePolygon;
+    }
+  }
+
+  return new QgsGeometry( obstacleGeom );
+}
+
 void QgsVectorLayerLabelProvider::drawLabel( QgsRenderContext& context, pal::LabelPosition* label ) const
 {
   if ( !mSettings.drawLabels )

diff --git a/src/core/qgsvectorlayerlabelprovider.h b/src/core/qgsvectorlayerlabelprovider.h
@@ -41,7 +41,8 @@ class CORE_EXPORT QgsVectorLayerLabelProvider : public QgsAbstractLabelProvider
                                  const QgsFields& fields,
                                  const QgsCoordinateReferenceSystem& crs,
                                  QgsAbstractFeatureSource* source,
-                                 bool ownsSource );
+                                 bool ownsSource,
+                                 QgsFeatureRendererV2* renderer = 0 );
 
     ~QgsVectorLayerLabelProvider();
 
@@ -72,6 +73,16 @@ class CORE_EXPORT QgsVectorLayerLabelProvider : public QgsAbstractLabelProvider
      */
     virtual void registerFeature( QgsFeature& feature, QgsRenderContext &context, QgsGeometry* obstacleGeometry = 0 );
 
+    /** Returns the geometry for a point feature which should be used as an obstacle for labels. This
+     * obstacle geometry will respect the dimensions and offsets of the symbol used to render the
+     * point, and ensures that labels will not overlap large or offset points.
+     * @param fet point feature
+     * @param context render context
+     * @param renderer renderer used for layer, required to determine symbols rendered for point feature
+     * @note added in QGIS 2.14
+     */
+    static QgsGeometry* getPointObstacleGeometry( QgsFeature& fet, QgsRenderContext& context, QgsFeatureRendererV2* renderer );
+
   protected:
     //! initialization method - called from constructors
     void init();
@@ -84,6 +95,8 @@ class CORE_EXPORT QgsVectorLayerLabelProvider : public QgsAbstractLabelProvider
     //! Layer's ID
     QString mLayerId;
 
+    QgsFeatureRendererV2* mRenderer;
+
     // these are needed only if using own renderer loop
 
     //! Layer's fields

diff --git a/src/core/qgsvectorlayerrenderer.cpp b/src/core/qgsvectorlayerrenderer.cpp
@@ -326,13 +326,18 @@ void QgsVectorLayerRenderer::drawRendererV2( QgsFeatureIterator& fit )
         // new labeling engine
         if ( mContext.labelingEngineV2() )
         {
+          QScopedPointer<QgsGeometry> obstacleGeometry;
+          if ( fet.constGeometry()->type() == QGis::Point )
+          {
+            obstacleGeometry.reset( QgsVectorLayerLabelProvider::getPointObstacleGeometry( fet, mContext, mRendererV2 ) );
+          }
           if ( mLabelProvider )
           {
-            mLabelProvider->registerFeature( fet, mContext );
+            mLabelProvider->registerFeature( fet, mContext, obstacleGeometry.data() );
           }
           if ( mDiagramProvider )
           {
-            mDiagramProvider->registerFeature( fet, mContext );
+            mDiagramProvider->registerFeature( fet, mContext, obstacleGeometry.data() );
           }
         }
       }
@@ -409,13 +414,18 @@ void QgsVectorLayerRenderer::drawRendererV2Levels( QgsFeatureIterator& fit )
     // new labeling engine
     if ( mContext.labelingEngineV2() )
     {
+      QScopedPointer<QgsGeometry> obstacleGeometry;
+      if ( fet.constGeometry()->type() == QGis::Point )
+      {
+        obstacleGeometry.reset( QgsVectorLayerLabelProvider::getPointObstacleGeometry( fet, mContext, mRendererV2 ) );
+      }
       if ( mLabelProvider )
       {
-        mLabelProvider->registerFeature( fet, mContext );
+        mLabelProvider->registerFeature( fet, mContext, obstacleGeometry.data() );
       }
       if ( mDiagramProvider )
       {
-        mDiagramProvider->registerFeature( fet, mContext );
+        mDiagramProvider->registerFeature( fet, mContext, obstacleGeometry.data() );
       }
     }
   }

diff --git a/src/core/symbology-ng/qgscategorizedsymbolrendererv2.cpp b/src/core/symbology-ng/qgscategorizedsymbolrendererv2.cpp
@@ -243,7 +243,9 @@ QgsSymbolV2* QgsCategorizedSymbolRendererV2::originalSymbolForFeature( QgsFeatur
   QVariant value;
   if ( mAttrNum == -1 )
   {
-    Q_ASSERT( mExpression.data() );
+    if ( !mExpression.data() )
+      return 0;
+
     value = mExpression->evaluate( &context.expressionContext() );
   }
   else

diff --git a/tests/src/core/testqgslabelingenginev2.cpp b/tests/src/core/testqgslabelingenginev2.cpp
@@ -134,16 +134,18 @@ void TestQgsLabelingEngineV2::testBasic()
   QVERIFY( imageCheck( "labeling_basic", img, 0 ) );
 
   // now let's test the variant when integrated into rendering loop
+  //note the reference images are slightly different due to use of renderer for this test
 
   job.start();
   job.waitForFinished();
   QImage img2 = job.renderedImage();
 
   vl->setCustomProperty( "labeling/enabled", false );
 
-  QVERIFY( imageCheck( "labeling_basic", img2, 0 ) );
+  QVERIFY( imageCheck( "labeling_basic_loop", img2, 0 ) );
 }
 
+
 void TestQgsLabelingEngineV2::testDiagrams()
 {
   QSize size( 640, 480 );

diff --git a/tests/src/python/test_qgspallabeling_placement.py b/tests/src/python/test_qgspallabeling_placement.py
@@ -133,7 +133,6 @@ def test_point_placement_narrow_polygon_obstacle(self):
         self.removeMapLayer(polyLayer)
         self.layer = None
 
-    @skip("not yet implemented")
     def test_point_placement_around_obstacle_large_symbol(self):
         # Default point label placement with obstacle and large symbols
         self.layer = TestQgsPalLabeling.loadFeatureLayer('point3')

diff --git a/.../labelingenginev2/expected_labeling_basic_loop/expected_labeling_basic_loop.png b/.../labelingenginev2/expected_labeling_basic_loop/expected_labeling_basic_loop.png