`GeomFill_CorrectedFrenet` hangs in some cases

[snoyer]

Description

GeomFill_CorrectedFrenet law is not usable on some curves because its initialization never terminates.

Expected Behavior

law.SetCurve(curve) terminates properly and we can proceed to use the object.

Actual Behavior

This loop does not terminate: https://github.com/Open-Cascade-SAS/OCCT/blob/87c04f1833be3cc52fdace2108de489dd301e016/src/GeomFill/GeomFill_CorrectedFrenet.cxx#L538

Sample Code or DRAW Tcl Script

#include <BRepAdaptor_CompCurve.hxx>
#include <BRepBuilderAPI_MakeEdge.hxx>
#include <GCPnts_AbscissaPoint.hxx>
#include <GC_MakeSegment.hxx>
#include <GeomFill_CorrectedFrenet.hxx>
#include <GeomFill_Frenet.hxx>
#include <ShapeExtend_WireData.hxx>
#include <TopoDS_Wire.hxx>
#include <gp_Pnt.hxx>

#include <vector>

int main() {
  const std::vector<gp_Pnt> points = {
      gp_Pnt(-1, -1, 0),
      gp_Pnt(0, -2, 0),
      gp_Pnt(0, -2, -1),
      gp_Pnt(0, -1, -1),
  };

  ShapeExtend_WireData extend;
  for (size_t i = 1; i < points.size(); ++i)
    extend.Add(BRepBuilderAPI_MakeEdge(GC_MakeSegment(points[i - 1], points[i]).Value()).Edge());

  BRepAdaptor_CompCurve adaptor(extend.WireAPIMake());

  GeomFill_Frenet law1;
  law1.SetCurve(adaptor.ShallowCopy()); // ok

  GeomFill_CorrectedFrenet law2;
  law2.SetCurve(adaptor.ShallowCopy()); // infinite loop in GeomFill_CorrectedFrenet::InitInterval

  return 0;
}

Operating System

Linux

Compiler

GCC

Bitness

64-bit

OCCT Version

latest

Additional Files

I do not know if the following change is the correct fix but it does allow the loop to terminate in the provided sample so it may be a starting point for someone who properly understand the algorithm (I don't)

diff --git a/src/GeomFill/GeomFill_CorrectedFrenet.cxx b/src/GeomFill/GeomFill_CorrectedFrenet.cxx
index 6487645dfe..17baef679f 100644
--- a/src/GeomFill/GeomFill_CorrectedFrenet.cxx
+++ b/src/GeomFill/GeomFill_CorrectedFrenet.cxx
@@ -540,7 +540,7 @@ Standard_Boolean GeomFill_CorrectedFrenet::InitInterval(const Standard_Real
     if (currParam > DLast)
     {
       currStep  = DLast - Param;
-      currParam = Last;
+      currParam = Param = Last;
     }
     if (isPlanar)
       currParam = Last;

[snoyer]

@dpasukhi has anyone been able to confirm/reproduce? Is there any chance this is going to get looked into before the 8.0 milestone it's currently assigned to?

I'm following up because it looks to me like:

1. the cause is more of a programming oversight due to the non-trivial stepping in the while loop than a hardcore computational geometry problem (so it hopefully wouldn't be too hard to fix)
2. the consequence (infinite loop) is both unpredictable and unrecoverable making it a pretty bad bug (not just an undesirable exception or a known and avoidable algorithmic limitation)

[dpasukhi]

Dear @snoyer the issue on community' list to do on 8.0. The developer, which I plant to allocate on that is busy with another commercial requests. The issue planned to be done for 8.0 and according internal priority ( commercial - first, critical - second ). In case if you want to receive fix in short time, only commercial request "Bug fixing and improvements" is possible to achieve that. More details: https://occt3d.com/services/bug-corrections-and-improvements/

[snoyer]

@dpasukhi thank you for the explanation, I was not aware of the "commercial first, critical second" policy.

[dpasukhi]

Resolved