Move forceRHR to QgsGeometry, avoid duplicate code

qgis · Nov 9, 2018 · ba17b13 · ba17b13
1 parent 63c9bcb
commit ba17b13
Show file tree

Hide file tree

Showing 9 changed files with 81 additions and 69 deletions.
diff --git a/python/core/auto_generated/geometry/qgsgeometry.sip.in b/python/core/auto_generated/geometry/qgsgeometry.sip.in
@@ -1468,6 +1468,15 @@ by calling `error()` on the returned geometry.
 
 
 .. versionadded:: 3.0
+%End
+
+    QgsGeometry forceRHR() const;
+%Docstring
+Forces geometries 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.
+
+.. versionadded:: 3.6
 %End
 
     class Error

diff --git a/python/plugins/processing/tests/testdata/custom/circular_strings.gpkg b/python/plugins/processing/tests/testdata/custom/circular_strings.gpkg
diff --git a/python/plugins/processing/tests/testdata/custom/pol.gpkg b/python/plugins/processing/tests/testdata/custom/pol.gpkg
diff --git a/python/plugins/processing/tests/testdata/expected/merged_pol.gpkg b/python/plugins/processing/tests/testdata/expected/merged_pol.gpkg
diff --git a/src/analysis/processing/qgsalgorithmforcerhr.cpp b/src/analysis/processing/qgsalgorithmforcerhr.cpp
@@ -84,40 +84,8 @@ QgsFeatureList QgsForceRHRAlgorithm::processFeature( const QgsFeature &feature,
   if ( !feature.hasGeometry() )
     return QgsFeatureList() << feature;
 
-  const QgsGeometry inputGeom = feature.geometry();
-  QgsGeometry outputGeometry = inputGeom;
-  if ( inputGeom.isMultipart() )
-  {
-    const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( inputGeom.constGet() );
-    std::unique_ptr< QgsGeometryCollection > newCollection( collection->createEmptyWithSameType() );
-    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->forceRHR();
-        newCollection->addGeometry( corrected.release() );
-      }
-      else
-      {
-        newCollection->addGeometry( g->clone() );
-      }
-    }
-    outputGeometry = QgsGeometry( std::move( newCollection ) );
-  }
-  else
-  {
-    if ( const QgsCurvePolygon *cp = qgsgeometry_cast< const QgsCurvePolygon * >( inputGeom.constGet() ) )
-    {
-      std::unique_ptr< QgsCurvePolygon > corrected( cp->clone() );
-      corrected->forceRHR();
-      outputGeometry = QgsGeometry( std::move( corrected ) );
-    }
-  }
-
   QgsFeature outputFeature = feature;
-  outputFeature.setGeometry( outputGeometry );
+  outputFeature.setGeometry( feature.geometry().forceRHR() );
 
   return QgsFeatureList() << outputFeature;
 }

diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp
@@ -2694,42 +2694,9 @@ static QVariant fcnBuffer( const QVariantList &values, const QgsExpressionContex
 
 static QVariant fcnForceRHR( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )
 {
-  QgsGeometry fGeom = QgsExpressionUtils::getGeometry( values.at( 0 ), parent );
-  if ( fGeom.isMultipart() )
-  {
-    const QgsGeometryCollection *collection = qgsgeometry_cast< const QgsGeometryCollection * >( fGeom.constGet() );
-    std::unique_ptr< QgsGeometryCollection > newCollection( collection->createEmptyWithSameType() );
-    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->forceRHR();
-        newCollection->addGeometry( corrected.release() );
-      }
-      else
-      {
-        newCollection->addGeometry( g->clone() );
-      }
-    }
-    QgsGeometry geom( std::move( newCollection ) );
-    return !geom.isNull() ? QVariant::fromValue( geom ) : QVariant();
-  }
-  else
-  {
-    if ( const QgsCurvePolygon *cp = qgsgeometry_cast< const QgsCurvePolygon * >( fGeom.constGet() ) )
-    {
-      std::unique_ptr< QgsCurvePolygon > corrected( cp->clone() );
-      corrected->forceRHR();
-      QgsGeometry geom( std::move( corrected ) );
-      return !geom.isNull() ? QVariant::fromValue( geom ) : QVariant();
-    }
-    else
-    {
-      return fGeom;
-    }
-  }
+  const QgsGeometry fGeom = QgsExpressionUtils::getGeometry( values.at( 0 ), parent );
+  const QgsGeometry reoriented = fGeom.forceRHR();
+  return !reoriented.isNull() ? QVariant::fromValue( reoriented ) : QVariant();
 }
 
 static QVariant fcnWedgeBuffer( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * )

diff --git a/src/core/geometry/qgsgeometry.cpp b/src/core/geometry/qgsgeometry.cpp
@@ -2365,6 +2365,47 @@ QgsGeometry QgsGeometry::makeValid() const
   return result;
 }
 
+QgsGeometry QgsGeometry::forceRHR() 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() );
+    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->forceRHR();
+        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->forceRHR();
+      return QgsGeometry( std::move( corrected ) );
+    }
+    else
+    {
+      // not a curve polygon, so return unchanged
+      return *this;
+    }
+  }
+}
+
 
 void QgsGeometry::validateGeometry( QVector<QgsGeometry::Error> &errors, ValidationMethod method ) const
 {

diff --git a/src/core/geometry/qgsgeometry.h b/src/core/geometry/qgsgeometry.h
@@ -1415,6 +1415,15 @@ class CORE_EXPORT QgsGeometry
      */
     QgsGeometry makeValid() const;
 
+    /**
+     * Forces geometries 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.
+     *
+     * \since QGIS 3.6
+     */
+    QgsGeometry forceRHR() const;
+
     /**
      * \ingroup core
      */

diff --git a/tests/src/python/test_qgsgeometry.py b/tests/src/python/test_qgsgeometry.py
@@ -4499,6 +4499,24 @@ def testOffsetCurve(self):
                              "mismatch for {} to {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[1],
                                                                                        t[2], res.asWkt(1)))
 
+    def testForceRHR(self):
+        tests = [
+            ["", ""],
+            ["Point (100 100)", "Point (100 100)"],
+            ["LINESTRING (0 0, 0 100, 100 100)", "LineString (0 0, 0 100, 100 100)"],
+            ["LINESTRING (100 100, 0 100, 0 0)", "LineString (100 100, 0 100, 0 0)"],
+            ["POLYGON((-1 -1, 4 0, 4 2, 0 2, -1 -1))", "Polygon ((-1 -1, 0 2, 4 2, 4 0, -1 -1))"],
+            ["MULTIPOLYGON(Polygon((-1 -1, 4 0, 4 2, 0 2, -1 -1)),Polygon((100 100, 200 100, 200 200, 100 200, 100 100)))", "MultiPolygon (((-1 -1, 0 2, 4 2, 4 0, -1 -1)),((100 100, 100 200, 200 200, 200 100, 100 100)))"],
+            [
+                "GeometryCollection(Polygon((-1 -1, 4 0, 4 2, 0 2, -1 -1)),Polygon((100 100, 200 100, 200 200, 100 200, 100 100)))",
+                "GeometryCollection (Polygon ((-1 -1, 0 2, 4 2, 4 0, -1 -1)),Polygon ((100 100, 100 200, 200 200, 200 100, 100 100)))"]
+        ]
+        for t in tests:
+            g1 = QgsGeometry.fromWkt(t[0])
+            res = g1.forceRHR()
+            self.assertEqual(res.asWkt(1), t[1],
+                             "mismatch for {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[1], res.asWkt(1)))
+
     def renderGeometry(self, geom, use_pen, as_polygon=False, as_painter_path=False):
         image = QImage(200, 200, QImage.Format_RGB32)
         image.fill(QColor(0, 0, 0))