[symbology] Fix broken centroid fill's force point inside polygon

qgis · Jun 26, 2019 · 870b037 · 870b037
1 parent 0a098a9
commit 870b037
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 17 deletions.
diff --git a/python/core/auto_generated/symbology/qgssymbollayerutils.sip.in b/python/core/auto_generated/symbology/qgssymbollayerutils.sip.in
@@ -661,9 +661,9 @@ Determines an SVG symbol's name from its ``path``.
 Calculate the centroid point of a QPolygonF
 %End
 
-    static QPointF polygonPointOnSurface( const QPolygonF &points );
+    static QPointF polygonPointOnSurface( const QPolygonF &points, QList<QPolygonF> *rings = 0 );
 %Docstring
-Calculate a point within of a QPolygonF
+Calculate a point on the surface of a QPolygonF
 %End
 
     static bool pointInPolygon( const QPolygonF &points, QPointF point );

diff --git a/src/core/symbology/qgsfillsymbollayer.cpp b/src/core/symbology/qgsfillsymbollayer.cpp
@@ -3440,8 +3440,6 @@ void QgsCentroidFillSymbolLayer::stopRender( QgsSymbolRenderContext &context )
 
 void QgsCentroidFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
 {
-  Q_UNUSED( rings );
-
   if ( !mPointOnAllParts )
   {
     const QgsFeature *feature = context.feature();
@@ -3475,7 +3473,7 @@ void QgsCentroidFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<Q
 
   if ( mPointOnAllParts || ( context.geometryPartNum() == mBiggestPartIndex ) )
   {
-    QPointF centroid = mPointOnSurface ? QgsSymbolLayerUtils::polygonPointOnSurface( points ) : QgsSymbolLayerUtils::polygonCentroid( points );
+    QPointF centroid = mPointOnSurface ? QgsSymbolLayerUtils::polygonPointOnSurface( points, rings ) : QgsSymbolLayerUtils::polygonCentroid( points );
     mMarker->renderPoint( centroid, context.feature(), context.renderContext(), -1, context.selected() );
   }
 }

diff --git a/src/core/symbology/qgssymbollayerutils.cpp b/src/core/symbology/qgssymbollayerutils.cpp
@@ -3871,32 +3871,41 @@ QPointF QgsSymbolLayerUtils::polygonCentroid( const QPolygonF &points )
   return QPointF( cx, cy );
 }
 
-QPointF QgsSymbolLayerUtils::polygonPointOnSurface( const QPolygonF &points )
+QPointF QgsSymbolLayerUtils::polygonPointOnSurface( const QPolygonF &points, QList<QPolygonF> *rings )
 {
   QPointF centroid = QgsSymbolLayerUtils::polygonCentroid( points );
 
-  // check if centroid inside in polygon
-  if ( !QgsSymbolLayerUtils::pointInPolygon( points, centroid ) )
+  if ( ( rings && rings->count() > 0 ) || !pointInPolygon( points, centroid ) )
   {
     unsigned int i, pointCount = points.count();
-
     QgsPolylineXY polyline( pointCount );
     for ( i = 0; i < pointCount; ++i ) polyline[i] = QgsPointXY( points[i].x(), points[i].y() );
-
     QgsGeometry geom = QgsGeometry::fromPolygonXY( QgsPolygonXY() << polyline );
     if ( !geom.isNull() )
     {
-      QgsGeometry pointOnSurfaceGeom = geom.pointOnSurface();
+      if ( rings )
+      {
+        QList<QPolygonF>::const_iterator ringIt = rings->constBegin();
+        for ( ; ringIt != rings->constEnd(); ++ringIt )
+        {
+          pointCount = ( *ringIt ).count();
+          QgsPolylineXY polyline( pointCount );
+          for ( i = 0; i < pointCount; ++i ) polyline[i] = QgsPointXY( ( *ringIt )[i].x(), ( *ringIt )[i].y() );
+          geom.addRing( polyline );
+        }
+      }
 
+      QgsGeometry pointOnSurfaceGeom = geom.pointOnSurface();
       if ( !pointOnSurfaceGeom.isNull() )
       {
         QgsPointXY point = pointOnSurfaceGeom.asPoint();
-
-        return QPointF( point.x(), point.y() );
+        centroid.setX( point.x() );
+        centroid.setY( point.y() );
       }
     }
   }
-  return centroid;
+
+  return QPointF( centroid.x(), centroid.y() );
 }
 
 bool QgsSymbolLayerUtils::pointInPolygon( const QPolygonF &points, QPointF point )

diff --git a/src/core/symbology/qgssymbollayerutils.h b/src/core/symbology/qgssymbollayerutils.h
@@ -605,8 +605,8 @@ class CORE_EXPORT QgsSymbolLayerUtils
     //! Calculate the centroid point of a QPolygonF
     static QPointF polygonCentroid( const QPolygonF &points );
 
-    //! Calculate a point within of a QPolygonF
-    static QPointF polygonPointOnSurface( const QPolygonF &points );
+    //! Calculate a point on the surface of a QPolygonF
+    static QPointF polygonPointOnSurface( const QPolygonF &points, QList<QPolygonF> *rings = nullptr );
 
     //! Calculate whether a point is within of a QPolygonF
     static bool pointInPolygon( const QPolygonF &points, QPointF point );

diff --git a/tests/src/core/testqgscentroidfillsymbol.cpp b/tests/src/core/testqgscentroidfillsymbol.cpp
@@ -53,6 +53,7 @@ class TestQgsCentroidFillSymbol : public QObject
     void cleanup() {} // will be called after every testfunction.
 
     void centroidFillSymbol();
+    void centroidFillSymbolPointOnSurface();
     void centroidFillSymbolPartBiggest();
 
   private:
@@ -132,11 +133,18 @@ void TestQgsCentroidFillSymbol::centroidFillSymbol()
   QVERIFY( imageCheck( "symbol_centroidfill" ) );
 }
 
+void TestQgsCentroidFillSymbol::centroidFillSymbolPointOnSurface()
+{
+  mCentroidFill->setPointOnSurface( true );
+  QVERIFY( imageCheck( "symbol_centroidfill_point_on_surface" ) );
+  mCentroidFill->setPointOnSurface( false );
+}
+
 void TestQgsCentroidFillSymbol::centroidFillSymbolPartBiggest()
 {
   mCentroidFill->setPointOnAllParts( false );
-
   QVERIFY( imageCheck( "symbol_centroidfill_part_biggest" ) );
+  mCentroidFill->setPointOnAllParts( true );
 }
 
 //

diff --git a/...centroidfill_point_on_surface/expected_symbol_centroidfill_point_on_surface.png b/...centroidfill_point_on_surface/expected_symbol_centroidfill_point_on_surface.png
diff --git a/...oidfill_point_on_surface/expected_symbol_centroidfill_point_on_surface_mask.png b/...oidfill_point_on_surface/expected_symbol_centroidfill_point_on_surface_mask.png