Add force_polygon_cw and force_polygon_ccw expression functions

These are recommended for use instead of the existing force_rhr function, due to the variability in definition of the "right hand rule" between different software applications. Using an explicit force_polygon_cw/ccw function removes user confusion when the results vary between different applications.
qgis · Nov 1, 2021 · 11dc9cc · 11dc9cc
1 parent 469d387
commit 11dc9cc
Show file tree

Hide file tree

Showing 13 changed files with 319 additions and 3 deletions.
diff --git a/python/core/auto_generated/geometry/qgscurvepolygon.sip.in b/python/core/auto_generated/geometry/qgscurvepolygon.sip.in
@@ -195,7 +195,38 @@ bounded by the polygon is to the right of the boundary. In particular, the exter
 ring is oriented in a clockwise direction and the interior rings in a counter-clockwise
 direction.
 
+.. warning::
+
+   Due to the conflicting definitions of the right-hand-rule in general use, it is recommended
+   to use the explicit :py:func:`~QgsCurvePolygon.forceClockwise` or :py:func:`~QgsCurvePolygon.forceCounterClockwise` methods instead.
+
+.. seealso:: :py:func:`forceClockwise`
+
+.. seealso:: :py:func:`forceCounterClockwise`
+
 .. versionadded:: 3.6
+%End
+
+    void forceClockwise();
+%Docstring
+Forces the polygon to respect the exterior ring is clockwise, interior rings are counter-clockwise convention.
+
+This convention is used primarily by ESRI software.
+
+.. seealso:: :py:func:`forceCounterClockwise`
+
+.. versionadded:: 3.24
+%End
+
+    void forceCounterClockwise();
+%Docstring
+Forces the polygon to respect the exterior ring is counter-clockwise, interior rings are clockwise convention.
+
+This convention matches the OGC Simple Features specification.
+
+.. seealso:: :py:func:`forceClockwise`
+
+.. versionadded:: 3.24
 %End
 
     virtual QPainterPath asQPainterPath() const;

diff --git a/python/core/auto_generated/geometry/qgsgeometry.sip.in b/python/core/auto_generated/geometry/qgsgeometry.sip.in
@@ -2302,7 +2302,38 @@ Forces geometries to respect the Right-Hand-Rule, in which the area that is boun
 is to the right of the boundary. In particular, the exterior ring is oriented in a clockwise direction
 and the interior rings in a counter-clockwise direction.
 
+.. warning::
+
+   Due to the conflicting definitions of the right-hand-rule in general use, it is recommended
+   to use the explicit :py:func:`~QgsGeometry.forcePolygonClockwise` or :py:func:`~QgsGeometry.forcePolygonCounterClockwise` methods instead.
+
+.. seealso:: :py:func:`forcePolygonClockwise`
+
+.. seealso:: :py:func:`forcePolygonCounterClockwise`
+
 .. versionadded:: 3.6
+%End
+
+    QgsGeometry forcePolygonClockwise() const;
+%Docstring
+Forces geometries to respect the exterior ring is clockwise, interior rings are counter-clockwise convention.
+
+This convention is used primarily by ESRI software.
+
+.. seealso:: :py:func:`forcePolygonCounterClockwise`
+
+.. versionadded:: 3.24
+%End
+
+    QgsGeometry forcePolygonCounterClockwise() const;
+%Docstring
+Forces geometries to respect the exterior ring is counter-clockwise, interior rings are clockwise convention.
+
+This convention matches the OGC Simple Features specification.
+
+.. seealso:: :py:func:`forcePolygonClockwise`
+
+.. versionadded:: 3.24
 %End
 
     class Error

diff --git a/resources/function_help/json/force_polygon_ccw b/resources/function_help/json/force_polygon_ccw
@@ -0,0 +1,9 @@
+{
+  "name": "force_polygon_ccw",
+  "type": "function",
+  "groups": ["GeometryGroup"],
+  "description": "Forces a geometry to respect the convention where exterior rings are counter-clockwise, interior rings are clockwise.",
+  "arguments": [ {"arg":"geometry","description":"a geometry. Any non-polygon geometries are returned unchanged."}],
+  "examples": [ { "expression":"geom_to_wkt(force_polygon_ccw(geometry:=geom_from_wkt('Polygon ((-1 -1, 0 2, 4 2, 4 0, -1 -1)))')))", "returns":"'Polygon ((-1 -1, 4 0, 4 2, 0 2, -1 -1))'"}]
+}
+
diff --git a/resources/function_help/json/force_polygon_cw b/resources/function_help/json/force_polygon_cw
@@ -0,0 +1,9 @@
+{
+  "name": "force_polygon_cw",
+  "type": "function",
+  "groups": ["GeometryGroup"],
+  "description": "Forces a geometry to respect the convention where exterior rings are clockwise, interior rings are counter-clockwise.",
+  "arguments": [ {"arg":"geometry","description":"a geometry. Any non-polygon geometries are returned unchanged."}],
+  "examples": [ { "expression":"geom_to_wkt(force_polygon_cw(geometry:=geom_from_wkt('POLYGON((-1 -1, 4 0, 4 2, 0 2, -1 -1))')))", "returns":"'Polygon ((-1 -1, 0 2, 4 2, 4 0, -1 -1))'"}]
+}
+
diff --git a/resources/function_help/json/force_rhr b/resources/function_help/json/force_rhr
@@ -2,7 +2,7 @@
   "name": "force_rhr",
   "type": "function",
   "groups": ["GeometryGroup"],
-  "description": "Forces a geometry to respect the Right-Hand-Rule, in which the area that is bounded by a polygon is to the right of the boundary. In particular, the exterior ring is oriented in a clockwise direction and the interior rings in a counter-clockwise direction.",
+  "description": "Forces a geometry to respect the Right-Hand-Rule, in which the area that is bounded by a polygon is to the right of the boundary. In particular, the exterior ring is oriented in a clockwise direction and the interior rings in a counter-clockwise direction. Due to the inconsistency in the definition of the Right-Hand-Rule in some contexts it is recommended to use the explicit force_polygon_cw function instead.",
   "arguments": [ {"arg":"geometry","description":"a geometry. Any non-polygon geometries are returned unchanged."}],
   "examples": [ { "expression":"geom_to_wkt(force_rhr(geometry:=geom_from_wkt('POLYGON((-1 -1, 4 0, 4 2, 0 2, -1 -1))')))", "returns":"'Polygon ((-1 -1, 0 2, 4 2, 4 0, -1 -1))'"}]
 }

diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp
@@ -4068,6 +4068,20 @@ static QVariant fcnForceRHR( const QVariantList &values, const QgsExpressionCont
   return !reoriented.isNull() ? QVariant::fromValue( reoriented ) : QVariant();
 }
 
+static QVariant fcnForcePolygonCW( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )
+{
+  const QgsGeometry fGeom = QgsExpressionUtils::getGeometry( values.at( 0 ), parent );
+  const QgsGeometry reoriented = fGeom.forcePolygonClockwise();
+  return !reoriented.isNull() ? QVariant::fromValue( reoriented ) : QVariant();
+}
+
+static QVariant fcnForcePolygonCCW( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )
+{
+  const QgsGeometry fGeom = QgsExpressionUtils::getGeometry( values.at( 0 ), parent );
+  const QgsGeometry reoriented = fGeom.forcePolygonCounterClockwise();
+  return !reoriented.isNull() ? QVariant::fromValue( reoriented ) : QVariant();
+}
+
 static QVariant fcnWedgeBuffer( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )
 {
   QgsGeometry fGeom = QgsExpressionUtils::getGeometry( values.at( 0 ), parent );
@@ -7298,6 +7312,10 @@ const QList<QgsExpressionFunction *> &QgsExpression::Functions()
                                             fcnBuffer, QStringLiteral( "GeometryGroup" ) )
         << new QgsStaticExpressionFunction( QStringLiteral( "force_rhr" ), QgsExpressionFunction::ParameterList() << QgsExpressionFunction::Parameter( QStringLiteral( "geometry" ) ),
                                             fcnForceRHR, QStringLiteral( "GeometryGroup" ) )
+        << new QgsStaticExpressionFunction( QStringLiteral( "force_polygon_cw" ), QgsExpressionFunction::ParameterList() << QgsExpressionFunction::Parameter( QStringLiteral( "geometry" ) ),
+                                            fcnForcePolygonCW, QStringLiteral( "GeometryGroup" ) )
+        << new QgsStaticExpressionFunction( QStringLiteral( "force_polygon_ccw" ), QgsExpressionFunction::ParameterList() << QgsExpressionFunction::Parameter( QStringLiteral( "geometry" ) ),
+                                            fcnForcePolygonCCW, QStringLiteral( "GeometryGroup" ) )
         << new QgsStaticExpressionFunction( QStringLiteral( "wedge_buffer" ), QgsExpressionFunction::ParameterList() << QgsExpressionFunction::Parameter( QStringLiteral( "center" ) )
                                             << QgsExpressionFunction::Parameter( QStringLiteral( "azimuth" ) )
                                             << QgsExpressionFunction::Parameter( QStringLiteral( "width" ) )

diff --git a/src/core/geometry/qgscurvepolygon.cpp b/src/core/geometry/qgscurvepolygon.cpp
@@ -802,6 +802,11 @@ void QgsCurvePolygon::removeInvalidRings()
 }
 
 void QgsCurvePolygon::forceRHR()
+{
+  forceClockwise();
+}
+
+void QgsCurvePolygon::forceClockwise()
 {
   if ( mExteriorRing && mExteriorRing->orientation() != QgsCurve::Clockwise )
   {
@@ -828,6 +833,32 @@ void QgsCurvePolygon::forceRHR()
   mInteriorRings = validRings;
 }
 
+void QgsCurvePolygon::forceCounterClockwise()
+{
+  if ( mExteriorRing && mExteriorRing->orientation() != QgsCurve::CounterClockwise )
+  {
+    // flip exterior ring orientation
+    mExteriorRing.reset( mExteriorRing->reversed() );
+  }
+
+  QVector<QgsCurve *> validRings;
+  for ( QgsCurve *curve : std::as_const( mInteriorRings ) )
+  {
+    if ( curve && curve->orientation() != QgsCurve::Clockwise )
+    {
+      // flip interior ring orientation
+      QgsCurve *flipped = curve->reversed();
+      validRings << flipped;
+      delete curve;
+    }
+    else
+    {
+      validRings << curve;
+    }
+  }
+  mInteriorRings = validRings;
+}
+
 QPainterPath QgsCurvePolygon::asQPainterPath() const
 {
   QPainterPath p;

diff --git a/src/core/geometry/qgscurvepolygon.h b/src/core/geometry/qgscurvepolygon.h
@@ -242,10 +242,35 @@ class CORE_EXPORT QgsCurvePolygon: public QgsSurface
      * ring is oriented in a clockwise direction and the interior rings in a counter-clockwise
      * direction.
      *
+     * \warning Due to the conflicting definitions of the right-hand-rule in general use, it is recommended
+     * to use the explicit forceClockwise() or forceCounterClockwise() methods instead.
+     *
+     * \see forceClockwise()
+     * \see forceCounterClockwise()
      * \since QGIS 3.6
      */
     void forceRHR();
 
+    /**
+     * Forces the polygon to respect the exterior ring is clockwise, interior rings are counter-clockwise convention.
+     *
+     * This convention is used primarily by ESRI software.
+     *
+     * \see forceCounterClockwise()
+     * \since QGIS 3.24
+     */
+    void forceClockwise();
+
+    /**
+     * Forces the polygon to respect the exterior ring is counter-clockwise, interior rings are clockwise convention.
+     *
+     * This convention matches the OGC Simple Features specification.
+     *
+     * \see forceClockwise()
+     * \since QGIS 3.24
+     */
+    void forceCounterClockwise();
+
     QPainterPath asQPainterPath() const override;
     void draw( QPainter &p ) const override;
     void transform( const QgsCoordinateTransform &ct, Qgis::TransformDirection d = Qgis::TransformDirection::Forward, bool transformZ = false ) override SIP_THROW( QgsCsException );

diff --git a/src/core/geometry/qgsgeometry.cpp b/src/core/geometry/qgsgeometry.cpp
@@ -2884,6 +2884,53 @@ QgsGeometry QgsGeometry::makeValid() const
 }
 
 QgsGeometry QgsGeometry::forceRHR() const
+{
+  return forcePolygonClockwise();
+}
+
+QgsGeometry QgsGeometry::forcePolygonClockwise() const
+{
+  if ( !d->geometry )
+    return QgsGeometry();
+
+  if ( isMultipart() )
+  {
+    const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( d->geometry.get() );
+    std::unique_ptr< QgsGeometryCollection > newCollection( collection->createEmptyWithSameType() );
+    newCollection->reserve( collection->numGeometries() );
+    for ( int i = 0; i < collection->numGeometries(); ++i )
+    {
+      const QgsAbstractGeometry *g = collection->geometryN( i );
+      if ( const QgsCurvePolygon *cp = qgsgeometry_cast< const QgsCurvePolygon * >( g ) )
+      {
+        std::unique_ptr< QgsCurvePolygon > corrected( cp->clone() );
+        corrected->forceClockwise();
+        newCollection->addGeometry( corrected.release() );
+      }
+      else
+      {
+        newCollection->addGeometry( g->clone() );
+      }
+    }
+    return QgsGeometry( std::move( newCollection ) );
+  }
+  else
+  {
+    if ( const QgsCurvePolygon *cp = qgsgeometry_cast< const QgsCurvePolygon * >( d->geometry.get() ) )
+    {
+      std::unique_ptr< QgsCurvePolygon > corrected( cp->clone() );
+      corrected->forceClockwise();
+      return QgsGeometry( std::move( corrected ) );
+    }
+    else
+    {
+      // not a curve polygon, so return unchanged
+      return *this;
+    }
+  }
+}
+
+QgsGeometry QgsGeometry::forcePolygonCounterClockwise() const
 {
   if ( !d->geometry )
     return QgsGeometry();
@@ -2899,7 +2946,7 @@ QgsGeometry QgsGeometry::forceRHR() const
       if ( const QgsCurvePolygon *cp = qgsgeometry_cast< const QgsCurvePolygon * >( g ) )
       {
         std::unique_ptr< QgsCurvePolygon > corrected( cp->clone() );
-        corrected->forceRHR();
+        corrected->forceCounterClockwise();
         newCollection->addGeometry( corrected.release() );
       }
       else
@@ -2914,7 +2961,7 @@ QgsGeometry QgsGeometry::forceRHR() const
     if ( const QgsCurvePolygon *cp = qgsgeometry_cast< const QgsCurvePolygon * >( d->geometry.get() ) )
     {
       std::unique_ptr< QgsCurvePolygon > corrected( cp->clone() );
-      corrected->forceRHR();
+      corrected->forceCounterClockwise();
       return QgsGeometry( std::move( corrected ) );
     }
     else

diff --git a/src/core/geometry/qgsgeometry.h b/src/core/geometry/qgsgeometry.h
@@ -2362,10 +2362,35 @@ class CORE_EXPORT QgsGeometry
      * is to the right of the boundary. In particular, the exterior ring is oriented in a clockwise direction
      * and the interior rings in a counter-clockwise direction.
      *
+     * \warning Due to the conflicting definitions of the right-hand-rule in general use, it is recommended
+     * to use the explicit forcePolygonClockwise() or forcePolygonCounterClockwise() methods instead.
+     *
+     * \see forcePolygonClockwise()
+     * \see forcePolygonCounterClockwise()
      * \since QGIS 3.6
      */
     QgsGeometry forceRHR() const;
 
+    /**
+     * Forces geometries to respect the exterior ring is clockwise, interior rings are counter-clockwise convention.
+     *
+     * This convention is used primarily by ESRI software.
+     *
+     * \see forcePolygonCounterClockwise()
+     * \since QGIS 3.24
+     */
+    QgsGeometry forcePolygonClockwise() const;
+
+    /**
+     * Forces geometries to respect the exterior ring is counter-clockwise, interior rings are clockwise convention.
+     *
+     * This convention matches the OGC Simple Features specification.
+     *
+     * \see forcePolygonClockwise()
+     * \since QGIS 3.24
+     */
+    QgsGeometry forcePolygonCounterClockwise() const;
+
     /**
      * \ingroup core
      * \brief A geometry error.

diff --git a/tests/src/core/geometry/testqgspolygon.cpp b/tests/src/core/geometry/testqgspolygon.cpp
@@ -1898,6 +1898,44 @@ void TestQgsPolygon::polygon()
   rhr.forceRHR();
   QCOMPARE( rhr.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4),(10 10 1 2, 20 10 6 8, 10 20 3 4, 10 10 1 2))" ) );
 
+  // force cw (same as force RHR)
+  rhr = QgsPolygon();
+  rhr.forceClockwise(); // no crash
+  rhr.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4))" ) );
+  rhr.forceClockwise();
+  QCOMPARE( rhr.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4))" ) );
+  rhr.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 100 0 13 14, 100 100 23 24, 0 100 23 24, 0 0 3 4))" ) );
+  rhr.forceClockwise();
+  QCOMPARE( rhr.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 23 24, 100 100 23 24, 100 0 13 14, 0 0 3 4))" ) );
+  rhr.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4),(10 10 1 2, 20 10 3 4, 20 20 4, 5, 10 20 6 8, 10 10 1 2))" ) );
+  rhr.forceClockwise();
+  QCOMPARE( rhr.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4),(10 10 1 2, 20 10 3 4, 10 20 6 8, 10 10 1 2))" ) );
+  rhr.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 100 0 13 14, 100 100 13 14, 0 100 23 24, 0 0 3 4),(10 10 1 2, 20 10 3 4, 20 20 4, 5, 10 20 6 8, 10 10 1 2))" ) );
+  rhr.forceClockwise();
+  QCOMPARE( rhr.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 23 24, 100 100 13 14, 100 0 13 14, 0 0 3 4),(10 10 1 2, 20 10 3 4, 10 20 6 8, 10 10 1 2))" ) );
+  rhr.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4),(10 10 1 2, 10 20 3 4, 20 10 6 8, 10 10 1 2))" ) );
+  rhr.forceClockwise();
+  QCOMPARE( rhr.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4),(10 10 1 2, 20 10 6 8, 10 20 3 4, 10 10 1 2))" ) );
+
+  // force ccw
+  QgsPolygon ccw;
+  ccw.forceCounterClockwise(); // no crash
+  ccw.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4))" ) );
+  ccw.forceCounterClockwise();
+  QCOMPARE( ccw.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 100 0 23 24, 100 100 13 14, 0 100 13 14, 0 0 3 4))" ) );
+  ccw.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 100 0 13 14, 100 100 23 24, 0 100 23 24, 0 0 3 4))" ) );
+  ccw.forceCounterClockwise();
+  QCOMPARE( ccw.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 100 0 13 14, 100 100 23 24, 0 100 23 24, 0 0 3 4))" ) );
+  ccw.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4),(10 10 1 2, 20 10 3 4, 20 20 4, 5, 10 20 6 8, 10 10 1 2))" ) );
+  ccw.forceCounterClockwise();
+  QCOMPARE( ccw.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 100 0 23 24, 100 100 13 14, 0 100 13 14, 0 0 3 4),(10 10 1 2, 10 20 6 8, 20 10 3 4, 10 10 1 2))" ) );
+  ccw.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 100 0 13 14, 100 100 13 14, 0 100 23 24, 0 0 3 4),(10 10 1 2, 20 10 3 4, 20 20 4, 5, 10 20 6 8, 10 10 1 2))" ) );
+  ccw.forceCounterClockwise();
+  QCOMPARE( ccw.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 100 0 13 14, 100 100 13 14, 0 100 23 24, 0 0 3 4),(10 10 1 2, 10 20 6 8, 20 10 3 4, 10 10 1 2))" ) );
+  ccw.fromWkt( QStringLiteral( "PolygonZM ((0 0 3 4, 0 100 13 14, 100 100 13 14, 100 0 23 24, 0 0 3 4),(10 10 1 2, 10 20 3 4, 20 10 6 8, 10 10 1 2))" ) );
+  ccw.forceCounterClockwise();
+  QCOMPARE( ccw.asWkt( 2 ), QStringLiteral( "PolygonZM ((0 0 3 4, 100 0 23 24, 100 100 13 14, 0 100 13 14, 0 0 3 4),(10 10 1 2, 10 20 3 4, 20 10 6 8, 10 10 1 2))" ) );
+
   // test bounding box intersects
   QgsPolygon bb;
   QVERIFY( !bb.boundingBoxIntersects( QgsRectangle( 1, 3, 6, 9 ) ) );