From 0d4b04deb332244b8eadf4bb7aed2d17eab0ad9e Mon Sep 17 00:00:00 2001
From: wandererfan <wandererfan@gmail.com>
Date: Wed, 21 Aug 2024 21:47:38 -0400
Subject: [PATCH] [TD]fix false positives in isCircle (fix #15979)

---
 src/Mod/TechDraw/App/Geometry.cpp | 59 ++++++++++++++++++++++---------
 src/Mod/TechDraw/App/Geometry.h   |  3 ++
 2 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/src/Mod/TechDraw/App/Geometry.cpp b/src/Mod/TechDraw/App/Geometry.cpp
index 43510f4da3..585b904166 100644
--- a/src/Mod/TechDraw/App/Geometry.cpp
+++ b/src/Mod/TechDraw/App/Geometry.cpp
@@ -41,6 +41,7 @@
 # include <BRepLib.hxx>
 # include <BRepLProp_CLProps.hxx>
 # include <BRepTools.hxx>
+#include <BRepLProp_CurveTool.hxx>
 # include <GC_MakeArcOfCircle.hxx>
 # include <GC_MakeEllipse.hxx>
 #include <GC_MakeCircle.hxx>
@@ -1556,9 +1557,12 @@ bool GeometryUtils::isCircle(TopoDS_Edge occEdge)
     return GeometryUtils::getCircleParms(occEdge, radius, center, isArc);
 }
 
-//tries to interpret a BSpline edge as a circle. Used by DVDim for approximate dimensions.
+//! tries to interpret a BSpline edge as a circle. Used by DVDim for approximate dimensions.
+//! calculates the curvature of the spline at a number of places and measures the deviation from the average
+//! a true circle has constant curvature and would have no deviation from the average.
 bool GeometryUtils::getCircleParms(TopoDS_Edge occEdge, double& radius, Base::Vector3d& center, bool& isArc)
 {
+    int testCount = 5;
     double curveLimit = EWTOLERANCE;
     BRepAdaptor_Curve c(occEdge);
     Handle(Geom_BSplineCurve) spline = c.BSpline();
@@ -1566,7 +1570,6 @@ bool GeometryUtils::getCircleParms(TopoDS_Edge occEdge, double& radius, Base::Ve
     f = c.FirstParameter();
     l = c.LastParameter();
     double parmRange = fabs(l - f);
-    int testCount = 6;
     double parmStep = parmRange/testCount;
     std::vector<double> curvatures;
     std::vector<gp_Pnt> centers;
@@ -1575,12 +1578,8 @@ bool GeometryUtils::getCircleParms(TopoDS_Edge occEdge, double& radius, Base::Ve
     Base::Vector3d sumCenter, valueAt;
     try {
         GeomLProp_CLProps prop(spline, f, 3, Precision::Confusion());
-        curvatures.push_back(prop.Curvature());
-        sumCurvature += prop.Curvature();
-        prop.CentreOfCurvature(curveCenter);
-        centers.push_back(curveCenter);
-        sumCenter += DrawUtil::toVector3d(curveCenter);
 
+        // check only the interior points of the edge
         for (int i = 1; i < (testCount - 1); i++) {
             prop.SetParameter(parmStep * i);
             curvatures.push_back(prop.Curvature());
@@ -1589,32 +1588,42 @@ bool GeometryUtils::getCircleParms(TopoDS_Edge occEdge, double& radius, Base::Ve
             centers.push_back(curveCenter);
             sumCenter += DrawUtil::toVector3d(curveCenter);
         }
-        prop.SetParameter(l);
-        curvatures.push_back(prop.Curvature());
-        sumCurvature += prop.Curvature();
-        prop.CentreOfCurvature(curveCenter);
-        centers.push_back(curveCenter);
-        sumCenter += DrawUtil::toVector3d(curveCenter);
+
     }
     catch (Standard_Failure&) {
         return false;
     }
-    Base::Vector3d avgCenter = sumCenter/testCount;
+    Base::Vector3d avgCenter = sumCenter/ centers.size();
 
-    double avgCurve = sumCurvature/testCount;
+    double avgCurve = sumCurvature/ centers.size();
     double errorCurve  = 0;
+    // sum the errors in curvature
     for (auto& cv: curvatures) {
         errorCurve += avgCurve - cv;
     }
-    errorCurve  = errorCurve/testCount;
+
+    double errorCenter{0};
+    for (auto& observe : centers) {
+        auto error = (DU::toVector3d(observe)- avgCenter).Length();
+        errorCenter += error;
+    }
+
+    // calculate average error in curvature.  we are only interested in the magnitude of the error
+    errorCurve  = fabs(errorCurve / curvatures.size());
+    // calculate the average error in center of curvature
+    errorCenter = errorCenter / curvatures.size();
+    auto edgeLong = edgeLength(occEdge);
+    double centerLimit = edgeLong * 0.01;
 
     isArc = !c.IsClosed();
     bool isCircle(false);
-    if ( errorCurve < curveLimit ) {
+    if ( errorCurve <= curveLimit  &&
+         errorCenter <= centerLimit) {
         isCircle = true;
         radius = 1.0/avgCurve;
         center = avgCenter;
     }
+
     return isCircle;
 }
 
@@ -1708,3 +1717,19 @@ TopoDS_Edge GeometryUtils::asLine(TopoDS_Edge occEdge)
     return result;
 }
 
+
+double GeometryUtils::edgeLength(TopoDS_Edge occEdge)
+{
+    BRepAdaptor_Curve adapt(occEdge);
+    const Handle(Geom_Curve) curve = adapt.Curve().Curve();
+    double first = BRepLProp_CurveTool::FirstParameter(adapt);
+    double last = BRepLProp_CurveTool::LastParameter(adapt);
+    try {
+        GeomAdaptor_Curve adaptor(curve);
+        return GCPnts_AbscissaPoint::Length(adaptor,first,last,Precision::Confusion());
+    }
+    catch (Standard_Failure& exc) {
+        THROWM(Base::CADKernelError, exc.GetMessageString())
+    }
+}
+
diff --git a/src/Mod/TechDraw/App/Geometry.h b/src/Mod/TechDraw/App/Geometry.h
index e87949e6dc..a417a41b65 100644
--- a/src/Mod/TechDraw/App/Geometry.h
+++ b/src/Mod/TechDraw/App/Geometry.h
@@ -450,6 +450,9 @@ class TechDrawExport GeometryUtils
         static bool isLine(TopoDS_Edge occEdge);
         static TopoDS_Edge asLine(TopoDS_Edge occEdge);
 
+        static double edgeLength(TopoDS_Edge occEdge);
+
+
 };
 
 } //end namespace TechDraw