Ensure that QgsGeometry::offsetCurve does not reverse curve orientation

nyalldawson · nyalldawson · commit e5946c50e737 · 2018-11-09T19:06:48.000+10:00
diff --git a/src/core/geometry/qgsgeometry.cpp b/src/core/geometry/qgsgeometry.cpp
@@ -1746,14 +1746,26 @@ QgsGeometry QgsGeometry::offsetCurve( double distance, int segments, JoinStyle j
   {
     QgsGeos geos( d->geometry.get() );
     mLastError.clear();
-    QgsAbstractGeometry *offsetGeom = geos.offsetCurve( distance, segments, joinStyle, miterLimit, &mLastError );
+
+    // GEOS can flip the curve orientation in some circumstances. So record previous orientation and correct if required
+    const QgsCurve::Orientation prevOrientation = qgsgeometry_cast< const QgsCurve * >( d->geometry.get() )->orientation();
+
+    std::unique_ptr< QgsAbstractGeometry > offsetGeom( geos.offsetCurve( distance, segments, joinStyle, miterLimit, &mLastError ) );
     if ( !offsetGeom )
     {
       QgsGeometry result;
       result.mLastError = mLastError;
       return result;
     }
-    return QgsGeometry( offsetGeom );
+
+    const QgsCurve::Orientation newOrientation = qgsgeometry_cast< const QgsCurve * >( offsetGeom.get() )->orientation();
+    if ( newOrientation != prevOrientation )
+    {
+      // GEOS has flipped line orientation, flip it back
+      std::unique_ptr< QgsAbstractGeometry > flipped( qgsgeometry_cast< const QgsCurve * >( offsetGeom.get() )->reversed() );
+      offsetGeom = std::move( flipped );
+    }
+    return QgsGeometry( std::move( offsetGeom ) );
   }
 }
 
diff --git a/tests/src/python/test_qgsgeometry.py b/tests/src/python/test_qgsgeometry.py
@@ -4483,6 +4483,22 @@ def testBoundingBoxIntersectsRectangle(self):
             res = g1.boundingBoxIntersects(t[1])
             self.assertEqual(res, t[2], "mismatch for {} to {}, expected:\n{}\nGot:\n{}\n".format(g1.asWkt(), t[1].toString(), t[2], res))
 
+    def testOffsetCurve(self):
+        tests = [
+            ["LINESTRING (0 0, 0 100, 100 100)", 1, "LineString (-1 0, -1 101, 100 101)"],
+            ["LINESTRING (0 0, 0 100, 100 100)", -1, "LineString (1 0, 1 99, 100 99)"],
+            ["LINESTRING (100 100, 0 100, 0 0)", 1, "LineString (100 99, 1 99, 1 0)"],
+            ["LINESTRING (100 100, 0 100, 0 0)", -1, "LineString (100 101, -1 101, -1 0)"],
+            ["MULTILINESTRING ((0 0, 0 100, 100 100),(100 100, 0 100, 0 0))", 1, "MultiLineString ((-1 0, -1 101, 100 101),(100 99, 1 99, 1 0))"]
+        ]
+        for t in tests:
+            g1 = QgsGeometry.fromWkt(t[0])
+            res = g1.offsetCurve(t[1], 2, QgsGeometry.JoinStyleMiter, 5)
+
+            self.assertEqual(res.asWkt(1), t[2],
+                             "mismatch for {} to {}, expected:\n{}\nGot:\n{}\n".format(t[0], t[1],
+                                                                                       t[2], 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))

Original file line number	Diff line number	Diff line change
`@@ -1746,14 +1746,26 @@ QgsGeometry QgsGeometry::offsetCurve( double distance, int segments, JoinStyle j`
`1746`	`1746`	`{`
`1747`	`1747`	`QgsGeos geos( d->geometry.get() );`
`1748`	`1748`	`mLastError.clear();`
`1749`		`- QgsAbstractGeometry *offsetGeom = geos.offsetCurve( distance, segments, joinStyle, miterLimit, &mLastError );`
	`1749`	`+`
	`1750`	`+ // GEOS can flip the curve orientation in some circumstances. So record previous orientation and correct if required`
	`1751`	`+ const QgsCurve::Orientation prevOrientation = qgsgeometry_cast< const QgsCurve * >( d->geometry.get() )->orientation();`
	`1752`	`+`
	`1753`	`+ std::unique_ptr< QgsAbstractGeometry > offsetGeom( geos.offsetCurve( distance, segments, joinStyle, miterLimit, &mLastError ) );`
`1750`	`1754`	`if ( !offsetGeom )`
`1751`	`1755`	`{`
`1752`	`1756`	`QgsGeometry result;`
`1753`	`1757`	`result.mLastError = mLastError;`
`1754`	`1758`	`return result;`
`1755`	`1759`	`}`
`1756`		`- return QgsGeometry( offsetGeom );`
	`1760`	`+`
	`1761`	`+ const QgsCurve::Orientation newOrientation = qgsgeometry_cast< const QgsCurve * >( offsetGeom.get() )->orientation();`
	`1762`	`+ if ( newOrientation != prevOrientation )`
	`1763`	`+ {`
	`1764`	`+ // GEOS has flipped line orientation, flip it back`
	`1765`	`+ std::unique_ptr< QgsAbstractGeometry > flipped( qgsgeometry_cast< const QgsCurve * >( offsetGeom.get() )->reversed() );`
	`1766`	`+ offsetGeom = std::move( flipped );`
	`1767`	`+ }`
	`1768`	`+ return QgsGeometry( std::move( offsetGeom ) );`
`1757`	`1769`	`}`
`1758`	`1770`	`}`
`1759`	`1771`