Merge pull request #1238 from ahuarte47/Issue_9480

dakcarto · dakcarto · commit fe42b0048fba · 2014-05-29T14:45:22.000-06:00
Fix bug #9480: Labels for polygon centroids should always originate inside a polygon
diff --git a/python/core/qgsgeometry.sip b/python/core/qgsgeometry.sip
@@ -322,6 +322,9 @@ class QgsGeometry
     * and for point based geometries, the point itself is returned */
     QgsGeometry* centroid() /Factory/;
 
+    /** Returns a point within a geometry */
+    QgsGeometry* pointOnSurface() /Factory/;
+
     /** Returns the smallest convex polygon that contains all the points in the geometry. */
     QgsGeometry* convexHull() /Factory/;
 
diff --git a/python/core/qgspallabeling.sip b/python/core/qgspallabeling.sip
@@ -394,6 +394,7 @@ class QgsPalLayerSettings
     unsigned int placementFlags;
 
     bool centroidWhole; // whether centroid calculated from whole or visible polygon
+    bool centroidInside; // whether centroid-point calculated must be inside polygon
     double dist; // distance from the feature (in mm)
     bool distInMapUnits; //true if distance is in map units (otherwise in mm)
     QgsMapUnitScale distMapUnitScale;
diff --git a/src/app/qgslabelinggui.cpp b/src/app/qgslabelinggui.cpp
@@ -259,6 +259,7 @@ void QgsLabelingGui::init()
 
   // populate placement options
   mCentroidRadioWhole->setChecked( lyr.centroidWhole );
+  mCentroidInsideCheckBox->setChecked( lyr.centroidInside );
   switch ( lyr.placement )
   {
     case QgsPalLayerSettings::AroundPoint:
@@ -545,6 +546,7 @@ QgsPalLayerSettings QgsLabelingGui::layerSettings()
 
   QWidget* curPlacementWdgt = stackedPlacement->currentWidget();
   lyr.centroidWhole = mCentroidRadioWhole->isChecked();
+  lyr.centroidInside = mCentroidInsideCheckBox->isChecked();
   if (( curPlacementWdgt == pagePoint && radAroundPoint->isChecked() )
       || ( curPlacementWdgt == pagePolygon && radAroundCentroid->isChecked() ) )
   {
diff --git a/src/core/pal/feature.cpp b/src/core/pal/feature.cpp
@@ -1344,7 +1344,7 @@ namespace pal
             case P_POINT:
             case P_POINT_OVER:
               double cx, cy;
-              mapShape->getCentroid( cx, cy );
+              mapShape->getCentroid( cx, cy, f->layer->getCentroidInside() );
               if ( f->layer->getArrangement() == P_POINT_OVER )
                 nbp = setPositionOverPoint( cx, cy, scale, lPos, delta, angle );
               else
diff --git a/src/core/pal/layer.cpp b/src/core/pal/layer.cpp
@@ -61,7 +61,7 @@ namespace pal
 
   Layer::Layer( const char *lyrName, double min_scale, double max_scale, Arrangement arrangement, Units label_unit, double defaultPriority, bool obstacle, bool active, bool toLabel, Pal *pal, bool displayAll )
       :  pal( pal ), obstacle( obstacle ), active( active ),
-      toLabel( toLabel ), displayAll( displayAll ), label_unit( label_unit ),
+      toLabel( toLabel ), displayAll( displayAll ), centroidInside( false ), label_unit( label_unit ),
       min_scale( min_scale ), max_scale( max_scale ),
       arrangement( arrangement ), arrangementFlags( 0 ), mode( LabelPerFeature ), mergeLines( false )
   {
diff --git a/src/core/pal/layer.h b/src/core/pal/layer.h
@@ -101,6 +101,7 @@ namespace pal
       bool active;
       bool toLabel;
       bool displayAll;
+      bool centroidInside;
 
       Units label_unit;
 
@@ -291,6 +292,9 @@ namespace pal
       void setUpsidedownLabels( UpsideDownLabels ud ) { upsidedownLabels = ud; }
       UpsideDownLabels getUpsidedownLabels() const { return upsidedownLabels; }
 
+      void setCentroidInside( bool forceInside ) { centroidInside = forceInside; }
+      bool getCentroidInside() const { return centroidInside; }
+
       /**
        * \brief register a feature in the layer
        *
diff --git a/src/core/pal/pointset.cpp b/src/core/pal/pointset.cpp
@@ -956,7 +956,7 @@ namespace pal
 
 
 
-  void PointSet::getCentroid( double &px, double &py )
+  void PointSet::getCentroid( double &px, double &py, bool forceInside )
   {
     // for explanation see this page:
     // http://local.wasp.uwa.edu.au/~pbourke/geometry/polyarea/
@@ -984,6 +984,35 @@ namespace pal
       px = cx / ( 3 * A ) + x[0];
       py = cy / ( 3 * A ) + y[0];
     }
+
+    // check if centroid inside in polygon
+    if ( forceInside && !isPointInPolygon( nbPoints, x, y, px, py ) )
+    {
+      GEOSCoordSequence *coord = GEOSCoordSeq_create( nbPoints, 2 );
+
+      for ( int i = 0; i < nbPoints; ++i )
+      {
+        GEOSCoordSeq_setX( coord, i, x[i] );
+        GEOSCoordSeq_setY( coord, i, y[i] );
+      }
+
+      GEOSGeometry *geom = GEOSGeom_createPolygon( GEOSGeom_createLinearRing( coord ), 0, 0 );
+
+      if ( geom )
+      {
+        GEOSGeometry *pointGeom = GEOSPointOnSurface( geom );
+
+        if ( pointGeom )
+        {
+          const GEOSCoordSequence *coordSeq = GEOSGeom_getCoordSeq( pointGeom );
+          GEOSCoordSeq_getX( coordSeq, 0, &px );
+          GEOSCoordSeq_getY( coordSeq, 0, &py );
+
+          GEOSGeom_destroy( pointGeom );
+        }
+        GEOSGeom_destroy( geom );
+      }
+    }
   }
 
 } // end namespace
diff --git a/src/core/pal/pointset.h b/src/core/pal/pointset.h
@@ -160,7 +160,7 @@ namespace pal
 
       //double getDistInside(double px, double py);
 
-      void getCentroid( double &px, double &py );
+      void getCentroid( double &px, double &py, bool forceInside = false );
 
 
       int getGeosType() const { return type; }
diff --git a/src/core/qgsgeometry.cpp b/src/core/qgsgeometry.cpp
@@ -145,6 +145,7 @@ static GEOSInit geosinit;
 #define GEOSArea(g, a) GEOSArea( (GEOSGeometry*) g, a )
 #define GEOSTopologyPreserveSimplify(g, t) GEOSTopologyPreserveSimplify( (GEOSGeometry*) g, t )
 #define GEOSGetCentroid(g) GEOSGetCentroid( (GEOSGeometry*) g )
+#define GEOSPointOnSurface(g) GEOSPointOnSurface( (GEOSGeometry*) g )
 
 #define GEOSCoordSeq_getSize(cs,n) GEOSCoordSeq_getSize( (GEOSCoordSequence *) cs, n )
 #define GEOSCoordSeq_getX(cs,i,x) GEOSCoordSeq_getX( (GEOSCoordSequence *)cs, i, x )
@@ -5623,6 +5624,21 @@ QgsGeometry* QgsGeometry::centroid()
   CATCH_GEOS( 0 )
 }
 
+QgsGeometry* QgsGeometry::pointOnSurface()
+{
+  if ( mDirtyGeos )
+    exportWkbToGeos();
+
+  if ( !mGeos )
+    return 0;
+
+  try
+  {
+    return fromGeosGeom( GEOSPointOnSurface( mGeos ) );
+  }
+  CATCH_GEOS( 0 )
+}
+
 QgsGeometry* QgsGeometry::convexHull()
 {
   if ( mDirtyGeos )
diff --git a/src/core/qgsgeometry.h b/src/core/qgsgeometry.h
@@ -363,6 +363,9 @@ class CORE_EXPORT QgsGeometry
     * and for point based geometries, the point itself is returned */
     QgsGeometry* centroid();
 
+    /** Returns a point within a geometry */
+    QgsGeometry* pointOnSurface();
+
     /** Returns the smallest convex polygon that contains all the points in the geometry. */
     QgsGeometry* convexHull();
 
diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp
@@ -311,6 +311,7 @@ QgsPalLayerSettings::QgsPalLayerSettings()
   placement = AroundPoint;
   placementFlags = 0;
   centroidWhole = false;
+  centroidInside = false;
   quadOffset = QuadrantOver;
   xOffset = 0;
   yOffset = 0;
@@ -502,6 +503,7 @@ QgsPalLayerSettings::QgsPalLayerSettings( const QgsPalLayerSettings& s )
   placement = s.placement;
   placementFlags = s.placementFlags;
   centroidWhole = s.centroidWhole;
+  centroidInside = s.centroidInside;
   quadOffset = s.quadOffset;
   xOffset = s.xOffset;
   yOffset = s.yOffset;
@@ -997,6 +999,7 @@ void QgsPalLayerSettings::readFromLayer( QgsVectorLayer* layer )
   placement = ( Placement )layer->customProperty( "labeling/placement" ).toInt();
   placementFlags = layer->customProperty( "labeling/placementFlags" ).toUInt();
   centroidWhole = layer->customProperty( "labeling/centroidWhole", QVariant( false ) ).toBool();
+  centroidInside = layer->customProperty( "labeling/centroidInside", QVariant( false ) ).toBool();
   dist = layer->customProperty( "labeling/dist" ).toDouble();
   distInMapUnits = layer->customProperty( "labeling/distInMapUnits" ).toBool();
   distMapUnitScale.minScale = layer->customProperty( "labeling/distMapUnitMinScale", 0.0 ).toDouble();
@@ -1167,6 +1170,7 @@ void QgsPalLayerSettings::writeToLayer( QgsVectorLayer* layer )
   layer->setCustomProperty( "labeling/placement", placement );
   layer->setCustomProperty( "labeling/placementFlags", ( unsigned int )placementFlags );
   layer->setCustomProperty( "labeling/centroidWhole", centroidWhole );
+  layer->setCustomProperty( "labeling/centroidInside", centroidInside );
   layer->setCustomProperty( "labeling/dist", dist );
   layer->setCustomProperty( "labeling/distInMapUnits", distInMapUnits );
   layer->setCustomProperty( "labeling/distMapUnitMinScale", distMapUnitScale.minScale );
@@ -3389,6 +3393,9 @@ int QgsPalLabeling::prepareLayer( QgsVectorLayer* layer, QStringList& attrNames,
   // set whether adjacent lines should be merged
   l->setMergeConnectedLines( lyr.mergeLines );
 
+  // set whether location of centroid must be inside of polygons
+  l->setCentroidInside( lyr.centroidInside );
+
   // set how to show upside-down labels
   Layer::UpsideDownLabels upsdnlabels;
   switch ( lyr.upsidedownLabels )
diff --git a/src/core/qgspallabeling.h b/src/core/qgspallabeling.h
@@ -374,6 +374,7 @@ class CORE_EXPORT QgsPalLayerSettings
     unsigned int placementFlags;
 
     bool centroidWhole; // whether centroid calculated from whole or visible polygon
+    bool centroidInside; // whether centroid-point calculated must be inside polygon 
     double dist; // distance from the feature (in mm)
     bool distInMapUnits; //true if distance is in map units (otherwise in mm)
     QgsMapUnitScale distMapUnitScale;
diff --git a/src/core/symbology-ng/qgsfillsymbollayerv2.cpp b/src/core/symbology-ng/qgsfillsymbollayerv2.cpp
@@ -3166,7 +3166,7 @@ QSet<QString> QgsPointPatternFillSymbolLayer::usedAttributes() const
 //////////////
 
 
-QgsCentroidFillSymbolLayerV2::QgsCentroidFillSymbolLayerV2(): mMarker( NULL )
+QgsCentroidFillSymbolLayerV2::QgsCentroidFillSymbolLayerV2(): mMarker( NULL ), mPointOnSurface( false )
 {
   setSubSymbol( new QgsMarkerSymbolV2() );
 }
@@ -3178,8 +3178,12 @@ QgsCentroidFillSymbolLayerV2::~QgsCentroidFillSymbolLayerV2()
 
 QgsSymbolLayerV2* QgsCentroidFillSymbolLayerV2::create( const QgsStringMap& properties )
 {
-  Q_UNUSED( properties );
-  return new QgsCentroidFillSymbolLayerV2();
+  QgsCentroidFillSymbolLayerV2* sl = new QgsCentroidFillSymbolLayerV2();
+
+  if ( properties.contains( "point_on_surface" ) )
+    sl->setPointOnSurface( properties["point_on_surface"].toInt() != 0 );
+
+  return sl;
 }
 
 QString QgsCentroidFillSymbolLayerV2::layerType() const
@@ -3208,13 +3212,15 @@ void QgsCentroidFillSymbolLayerV2::renderPolygon( const QPolygonF& points, QList
 {
   Q_UNUSED( rings );
 
-  QPointF centroid = QgsSymbolLayerV2Utils::polygonCentroid( points );
+  QPointF centroid = mPointOnSurface ? QgsSymbolLayerV2Utils::polygonPointOnSurface( points ) : QgsSymbolLayerV2Utils::polygonCentroid( points );
   mMarker->renderPoint( centroid, context.feature(), context.renderContext(), -1, context.selected() );
 }
 
 QgsStringMap QgsCentroidFillSymbolLayerV2::properties() const
 {
-  return QgsStringMap();
+  QgsStringMap map;
+  map["point_on_surface"] = QString::number( mPointOnSurface );
+  return map;
 }
 
 QgsSymbolLayerV2* QgsCentroidFillSymbolLayerV2::clone() const
@@ -3223,6 +3229,7 @@ QgsSymbolLayerV2* QgsCentroidFillSymbolLayerV2::clone() const
   x->mAngle = mAngle;
   x->mColor = mColor;
   x->setSubSymbol( mMarker->clone() );
+  x->setPointOnSurface( mPointOnSurface );
   return x;
 }
 
@@ -3246,9 +3253,9 @@ QgsSymbolLayerV2* QgsCentroidFillSymbolLayerV2::createFromSld( QDomElement &elem
   layers.append( l );
   QgsMarkerSymbolV2 *marker = new QgsMarkerSymbolV2( layers );
 
-  QgsCentroidFillSymbolLayerV2* x = new QgsCentroidFillSymbolLayerV2();
-  x->setSubSymbol( marker );
-  return x;
+  QgsCentroidFillSymbolLayerV2* sl = new QgsCentroidFillSymbolLayerV2();
+  sl->setSubSymbol( marker );
+  return sl;
 }
 
 
diff --git a/src/core/symbology-ng/qgsfillsymbollayerv2.h b/src/core/symbology-ng/qgsfillsymbollayerv2.h
@@ -904,8 +904,12 @@ class CORE_EXPORT QgsCentroidFillSymbolLayerV2 : public QgsFillSymbolLayerV2
 
     virtual QSet<QString> usedAttributes() const;
 
+    void setPointOnSurface( bool pointOnSurface ) { mPointOnSurface = pointOnSurface; }
+    bool pointOnSurface() const { return mPointOnSurface; }
+
   protected:
     QgsMarkerSymbolV2* mMarker;
+    bool mPointOnSurface;
 };
 
 #endif
diff --git a/src/core/symbology-ng/qgssymbollayerv2utils.cpp b/src/core/symbology-ng/qgssymbollayerv2utils.cpp
@@ -3179,6 +3179,62 @@ QPointF QgsSymbolLayerV2Utils::polygonCentroid( const QPolygonF& points )
   return QPointF( cx, cy );
 }
 
+QPointF QgsSymbolLayerV2Utils::polygonPointOnSurface( const QPolygonF& points )
+{
+  QPointF centroid = QgsSymbolLayerV2Utils::polygonCentroid( points );
+
+  // check if centroid inside in polygon
+  if ( !QgsSymbolLayerV2Utils::pointInPolygon( points, centroid ) )
+  {
+    unsigned int i, pointCount = points.count();
+
+    QgsPolyline polyline( pointCount );
+    for ( i = 0; i < pointCount; ++i ) polyline[i] = QgsPoint( points[i].x(), points[i].y() );
+
+    QgsGeometry* geom = QgsGeometry::fromPolygon( QgsPolygon() << polyline );
+    if ( geom )
+    {
+      QgsGeometry* pointOnSurfaceGeom = geom->pointOnSurface();
+
+      if ( pointOnSurfaceGeom )
+      {
+        QgsPoint point = pointOnSurfaceGeom->asPoint();
+        delete pointOnSurfaceGeom;
+        delete geom;
+
+        return QPointF( point.x(), point.y() );
+      }
+      delete geom;
+    }
+  }
+  return centroid;
+}
+
+bool QgsSymbolLayerV2Utils::pointInPolygon( const QPolygonF &points, const QPointF &point )
+{
+  bool inside = false;
+
+  double x = point.x();
+  double y = point.y();
+
+  for ( int i = 0, j = points.count() - 1; i < points.count(); i++ )
+  {
+    const QPointF& p1 = points[i];
+    const QPointF& p2 = points[j];
+
+    if ( p1.x() == x && p1.y() == y )
+      return true;
+
+    if ( ( p1.y() < y && p2.y() >= y ) || ( p2.y() < y && p1.y() >= y ) )
+    {
+      if ( p1.x() + ( y - p1.y() ) / ( p2.y() - p1.y() )*( p2.x() - p1.x() ) <= x )
+        inside = !inside;
+    }
+
+    j = i;
+  }
+  return inside;
+}
 
 QgsExpression* QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( const QString& fieldOrExpression )
 {
diff --git a/src/core/symbology-ng/qgssymbollayerv2utils.h b/src/core/symbology-ng/qgssymbollayerv2utils.h
@@ -287,6 +287,12 @@ class CORE_EXPORT QgsSymbolLayerV2Utils
     //! 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 whether a point is within of a QPolygonF
+    static bool pointInPolygon( const QPolygonF &points, const QPointF &point );
+
     /** Return a new valid expression instance for given field or expression string.
      * If the input is not a valid expression, it is assumed that it is a field name and gets properly quoted.
      * If the string is empty, returns null pointer.
diff --git a/src/gui/symbology-ng/qgssymbollayerv2widget.cpp b/src/gui/symbology-ng/qgssymbollayerv2widget.cpp
@@ -2794,12 +2794,20 @@ void QgsCentroidFillSymbolLayerV2Widget::setSymbolLayer( QgsSymbolLayerV2* layer
 
   // layer type is correct, we can do the cast
   mLayer = static_cast<QgsCentroidFillSymbolLayerV2*>( layer );
+
+  // set values
+  mDrawInsideCheckBox->blockSignals( true );
+  mDrawInsideCheckBox->setChecked( mLayer->pointOnSurface() );
+  mDrawInsideCheckBox->blockSignals( false );
 }
 
 QgsSymbolLayerV2* QgsCentroidFillSymbolLayerV2Widget::symbolLayer()
 {
   return mLayer;
 }
 
-
-
+void QgsCentroidFillSymbolLayerV2Widget::on_mDrawInsideCheckBox_stateChanged( int state )
+{
+  mLayer->setPointOnSurface( state == Qt::Checked );
+  emit changed();
+}
diff --git a/src/gui/symbology-ng/qgssymbollayerv2widget.h b/src/gui/symbology-ng/qgssymbollayerv2widget.h
@@ -482,6 +482,9 @@ class GUI_EXPORT QgsCentroidFillSymbolLayerV2Widget : public QgsSymbolLayerV2Wid
     virtual void setSymbolLayer( QgsSymbolLayerV2* layer );
     virtual QgsSymbolLayerV2* symbolLayer();
 
+  public slots:
+    void on_mDrawInsideCheckBox_stateChanged( int state );
+
   protected:
     QgsCentroidFillSymbolLayerV2* mLayer;
 };
diff --git a/src/ui/qgslabelingguibase.ui b/src/ui/qgslabelingguibase.ui
diff --git a/src/ui/symbollayer/widget_centroidfill.ui b/src/ui/symbollayer/widget_centroidfill.ui

Original file line number	Diff line number	Diff line change
`@@ -259,6 +259,7 @@ void QgsLabelingGui::init()`
`259`	`259`
`260`	`260`	`// populate placement options`
`261`	`261`	`mCentroidRadioWhole->setChecked( lyr.centroidWhole );`
	`262`	`+ mCentroidInsideCheckBox->setChecked( lyr.centroidInside );`
`262`	`263`	`switch ( lyr.placement )`
`263`	`264`	`{`
`264`	`265`	`case QgsPalLayerSettings::AroundPoint:`
`@@ -545,6 +546,7 @@ QgsPalLayerSettings QgsLabelingGui::layerSettings()`
`545`	`546`
`546`	`547`	`QWidget* curPlacementWdgt = stackedPlacement->currentWidget();`
`547`	`548`	`lyr.centroidWhole = mCentroidRadioWhole->isChecked();`
	`549`	`+ lyr.centroidInside = mCentroidInsideCheckBox->isChecked();`
`548`	`550`	`if (( curPlacementWdgt == pagePoint && radAroundPoint->isChecked() )`
`549`	`551`	`\|\| ( curPlacementWdgt == pagePolygon && radAroundCentroid->isChecked() ) )`
`550`	`552`	`{`
Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ namespace pal`
`61`	`61`
`62`	`62`	`Layer::Layer( const char lyrName, double min_scale, double max_scale, Arrangement arrangement, Units label_unit, double defaultPriority, bool obstacle, bool active, bool toLabel, Pal pal, bool displayAll )`
`63`	`63`	`: pal( pal ), obstacle( obstacle ), active( active ),`
`64`		`- toLabel( toLabel ), displayAll( displayAll ), label_unit( label_unit ),`
	`64`	`+ toLabel( toLabel ), displayAll( displayAll ), centroidInside( false ), label_unit( label_unit ),`
`65`	`65`	`min_scale( min_scale ), max_scale( max_scale ),`
`66`	`66`	`arrangement( arrangement ), arrangementFlags( 0 ), mode( LabelPerFeature ), mergeLines( false )`
`67`	`67`	`{`