Merge pull request OSGeo#9384 from rouault/fix_9382

OGRGeometry::getCurveGeometry(): avoid failures when building some compound curves with infered circular strings (fixes OSGeo#9382)
BRL-CAD · Mar 6, 2024 · 70e50ac · 70e50ac
2 parents 67882ef + 79cea9c
commit 70e50ac
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 17 deletions.
diff --git a/autotest/ogr/ogr_geom.py b/autotest/ogr/ogr_geom.py
@@ -2874,14 +2874,14 @@ def test_ogr_geom_getcurvegeometry():
 
     # Add repeated point at end of line
     g2 = g1.GetLinearGeometry()
-    g2.AddPoint_2D(2 - 1e-9, 0)
+    g2.AddPoint_2D(2 - 1e-14, 0)
     g3 = g2.GetCurveGeometry()
     assert g3.Equals(g1)
 
     # Add repeated point at end of line
     g2 = g1.GetLinearGeometry()
     g2_new = ogr.Geometry(ogr.wkbLineString)
-    g2_new.AddPoint_2D(0, 1e-9)
+    g2_new.AddPoint_2D(0, 1e-14)
     for i in range(g2.GetPointCount()):
         g2_new.AddPoint_2D(g2.GetX(i), g2.GetY(i))
     g3 = g2_new.GetCurveGeometry()
@@ -3019,6 +3019,20 @@ def test_ogr_geom_getcurvegeometry_issue_8332():
     ogrtest.check_feature_geometry(g3, expected_g, 1e-10)
 
 
+###############################################################################
+
+
+def test_ogr_geom_getcurvegeometry_issue9382():
+
+    wkt = "POLYGON ((299517.68 6695650.45,299519.85 6695648.29,299522.85 6695645.24,299523.253879511 6695644.9271687,299523.639375687 6695644.59194529,299524.00524874 6695644.23540783,299524.350321919 6695643.8587029,299524.673485444 6695643.46304214,299524.973699995 6695643.04969797,299525.25 6695642.62,299525.27 6695642.6,299525.28 6695642.57,299525.581819649 6695642.02405493,299525.844959794 6695641.4584502,299526.068147436 6695640.87592268,299526.250302851 6695640.27928972,299526.390544832 6695639.67143822,299526.488194942 6695639.05530858,299526.542780772 6695638.43388152,299526.554038241 6695637.81016326,299526.521912903 6695637.18717098,299526.446560174 6695636.56791901,299526.328344583 6695635.95540261,299526.167838007 6695635.35258508,299525.965816945 6695634.76238274,299525.723258704 6695634.18765068,299525.441336721 6695633.63116956,299525.121414825 6695633.09563136,299524.765040724 6695632.58362663,299524.373938437 6695632.09763257,299523.95 6695631.64,299522.14 6695633.39,299522.32 6695633.59,299522.48 6695633.77,299522.63 6695633.96,299522.77 6695634.15,299522.9 6695634.35,299523.03 6695634.55,299523.15 6695634.76,299523.26 6695634.98,299523.37 6695635.19,299523.46 6695635.41,299523.55 6695635.64,299523.63 6695635.86,299523.7 6695636.09,299523.76 6695636.32,299523.81 6695636.56,299523.86 6695636.79,299523.9 6695637.03,299523.92 6695637.27,299523.94 6695637.51,299523.95 6695637.71,299523.95 6695637.97,299523.94 6695638.23,299523.93 6695638.47,299523.9 6695638.71,299523.87 6695638.95,299523.82 6695639.18,299523.77 6695639.42,299523.71 6695639.65,299523.64 6695639.88,299523.56 6695640.11,299523.47 6695640.33,299523.38 6695640.55,299523.28 6695640.77,299523.17 6695640.98,299523.05 6695641.19,299522.92 6695641.4,299522.79 6695641.6,299522.65 6695641.79,299522.5 6695641.98,299522.34 6695642.16,299522.21 6695642.31,299522.08 6695642.45,299515.85 6695648.75,299515.41 6695649.19,299514.69 6695649.95,299513.99 6695650.71,299513.29 6695651.49,299512.902658228 6695651.94,299516.254162679 6695651.94,299517.68 6695650.45))"
+    g = ogr.CreateGeometryFromWkt(wkt)
+    g = g.GetCurveGeometry()
+
+    expected_wkt = "CURVEPOLYGON (COMPOUNDCURVE ((299517.68 6695650.45,299519.85 6695648.29,299522.85 6695645.24),CIRCULARSTRING (299522.85 6695645.24,299524.00524874 6695644.23540783,299525.25 6695642.62),(299525.25 6695642.62,299525.27 6695642.6,299525.28 6695642.57),CIRCULARSTRING (299525.28 6695642.57,299526.521912903 6695637.18717098,299523.95 6695631.64),(299523.95 6695631.64,299522.14 6695633.39,299522.32 6695633.59,299522.48 6695633.77,299522.63 6695633.96,299522.77 6695634.15,299522.9 6695634.35,299523.03 6695634.55,299523.15 6695634.76,299523.26 6695634.98,299523.37 6695635.19,299523.46 6695635.41,299523.55 6695635.64,299523.63 6695635.86,299523.7 6695636.09,299523.76 6695636.32,299523.81 6695636.56,299523.86 6695636.79,299523.9 6695637.03,299523.92 6695637.27,299523.94 6695637.51,299523.95 6695637.71,299523.95 6695637.97,299523.94 6695638.23,299523.93 6695638.47,299523.9 6695638.71,299523.87 6695638.95,299523.82 6695639.18,299523.77 6695639.42,299523.71 6695639.65,299523.64 6695639.88,299523.56 6695640.11,299523.47 6695640.33,299523.38 6695640.55,299523.28 6695640.77,299523.17 6695640.98,299523.05 6695641.19,299522.92 6695641.4,299522.79 6695641.6,299522.65 6695641.79,299522.5 6695641.98,299522.34 6695642.16,299522.21 6695642.31,299522.08 6695642.45,299515.85 6695648.75,299515.41 6695649.19,299514.69 6695649.95,299513.99 6695650.71,299513.29 6695651.49,299512.902658228 6695651.94,299516.254162679 6695651.94,299517.68 6695650.45)))"
+    expected_g = ogr.CreateGeometryFromWkt(expected_wkt)
+    ogrtest.check_feature_geometry(g, expected_g, 1e-10)
+
+
 ###############################################################################
 # Test OGR_GT_ functions
 

diff --git a/ogr/ogr_geometry.h b/ogr/ogr_geometry.h
@@ -2193,9 +2193,17 @@ class CPL_DLL OGRCompoundCurve : public OGRCurve
     virtual void
     assignSpatialReference(const OGRSpatialReference *poSR) override;
 
-    OGRErr addCurve(const OGRCurve *, double dfToleranceEps = 1e-14);
-    OGRErr addCurveDirectly(OGRCurve *, double dfToleranceEps = 1e-14);
-    OGRErr addCurve(std::unique_ptr<OGRCurve>, double dfToleranceEps = 1e-14);
+    /** Default relative tolerance to assume that the end of the previous curve
+     * is equal to the start of the next one.
+     */
+    static constexpr double DEFAULT_TOLERANCE_EPSILON = 1e-14;
+
+    OGRErr addCurve(const OGRCurve *,
+                    double dfToleranceEps = DEFAULT_TOLERANCE_EPSILON);
+    OGRErr addCurveDirectly(OGRCurve *,
+                            double dfToleranceEps = DEFAULT_TOLERANCE_EPSILON);
+    OGRErr addCurve(std::unique_ptr<OGRCurve>,
+                    double dfToleranceEps = DEFAULT_TOLERANCE_EPSILON);
     OGRCurve *stealCurve(int);
     virtual OGRPointIterator *getPointIterator() const override;
 

diff --git a/ogr/ogrcompoundcurve.cpp b/ogr/ogrcompoundcurve.cpp
@@ -146,7 +146,8 @@ OGRErr OGRCompoundCurve::addCurveDirectlyFromWkb(OGRGeometry *poSelf,
                                                  OGRCurve *poCurve)
 {
     OGRCompoundCurve *poCC = poSelf->toCompoundCurve();
-    return poCC->addCurveDirectlyInternal(poCurve, 1e-14, FALSE);
+    return poCC->addCurveDirectlyInternal(poCurve, DEFAULT_TOLERANCE_EPSILON,
+                                          FALSE);
 }
 
 /************************************************************************/
@@ -500,7 +501,7 @@ OGRCurve *OGRCompoundCurve::stealCurve(int iCurve)
  * @param poCurve geometry to add to the container.
  * @param dfToleranceEps relative tolerance when checking that the first point
  * of a segment matches then end point of the previous one. Default value:
- * 1e-14.
+ * OGRCompoundCurve::DEFAULT_TOLERANCE_EPSILON.
  *
  * @return OGRERR_NONE if successful, or OGRERR_FAILURE in case of error
  * (for example if curves are not contiguous)
@@ -534,7 +535,7 @@ OGRErr OGRCompoundCurve::addCurve(const OGRCurve *poCurve,
  * @param poCurve geometry to add to the container.
  * @param dfToleranceEps relative tolerance when checking that the first point
  * of a segment matches then end point of the previous one. Default value:
- * 1e-14.
+ * OGRCompoundCurve::DEFAULT_TOLERANCE_EPSILON.
  *
  * @return OGRERR_NONE if successful, or OGRERR_FAILURE in case of error
  * (for example if curves are not contiguous)
@@ -626,7 +627,7 @@ OGRErr OGRCompoundCurve::addCurveDirectlyInternal(OGRCurve *poCurve,
  * @param poCurve geometry to add to the container.
  * @param dfToleranceEps relative tolerance when checking that the first point
  * of a segment matches then end point of the previous one. Default value:
- * 1e-14.
+ * OGRCompoundCurve::DEFAULT_TOLERANCE_EPSILON.
  *
  * @return OGRERR_NONE if successful, or OGRERR_FAILURE in case of error
  * (for example if curves are not contiguous)

diff --git a/ogr/ogrgeometryfactory.cpp b/ogr/ogrgeometryfactory.cpp
@@ -5821,12 +5821,12 @@ static int OGRGF_DetectArc(const OGRLineString *poLS, int i,
         dfScale2 = std::max(dfScale2, fabs(p0.getY()));
         // Not strictly necessary, but helps having 'clean' lines without
         // duplicated points.
-        if (fabs(poLSNew->getX(poLSNew->getNumPoints() - 1) - p0.getX()) /
-                    dfScale2 >
-                1.0e-8 ||
-            fabs(poLSNew->getY(poLSNew->getNumPoints() - 1) - p0.getY()) /
-                    dfScale2 >
-                1.0e-8)
+        constexpr double dfToleranceEps =
+            OGRCompoundCurve::DEFAULT_TOLERANCE_EPSILON;
+        if (fabs(poLSNew->getX(poLSNew->getNumPoints() - 1) - p0.getX()) >
+                dfToleranceEps * dfScale2 ||
+            fabs(poLSNew->getY(poLSNew->getNumPoints() - 1) - p0.getY()) >
+                dfToleranceEps * dfScale2)
             poLSNew->addPoint(&p0);
         if (poLSNew->getNumPoints() >= 2)
         {
@@ -6137,10 +6137,12 @@ OGRCurve *OGRGeometryFactory::curveFromLineString(
             dfScale = std::max(dfScale, fabs(p.getY()));
             if (bIsClosed && i == nLSNumPoints - 1)
                 dfScale = 0;
+            constexpr double dfToleranceEps =
+                OGRCompoundCurve::DEFAULT_TOLERANCE_EPSILON;
             if (fabs(poLSNew->getX(poLSNew->getNumPoints() - 1) - p.getX()) >
-                    1e-8 * dfScale ||
+                    dfToleranceEps * dfScale ||
                 fabs(poLSNew->getY(poLSNew->getNumPoints() - 1) - p.getY()) >
-                    1e-8 * dfScale)
+                    dfToleranceEps * dfScale)
             {
                 poLSNew->addPoint(&p);
             }