Modeling - Infinite loop when Simplifying Fuse operation, CPU to 100% #557

Minor refactoring of RelocatePCurvesToNewUorigin().
RelocatePCurvesToNewUorigin() can no longer stuck in infinite loop if it found the edge that is not present in theVEmap.
Test bug_gh544 is added to check the fix.

diff --git a/src/ModelingAlgorithms/TKShHealing/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.cxx b/src/ModelingAlgorithms/TKShHealing/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.cxx
index c208deb9d08abd2c161f72829c43efc9b2d7649f..a4c9760d3bfba51dc21d356f4687b17f1e2293ce 100644 (file)
--- a/src/ModelingAlgorithms/TKShHealing/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.cxx
+++ b/src/ModelingAlgorithms/TKShHealing/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.cxx
@@ -282,26 +282,40 @@ static Standard_Boolean TryMakeLine(const Handle(Geom2d_Curve)& thePCurve,
   return Standard_True;
 }
 
-static void RemoveEdgeFromMap(const TopoDS_Edge&                         theEdge,
-                              TopTools_IndexedDataMapOfShapeListOfShape& theVEmap)
+// Removes the specified edge from the vertex to edge map.
+// @param theEdge The edge to remove.
+// @param theVertexToEdges The map of vertices to edges.
+// @return True if the edge was removed, false otherwise.
+static bool RemoveEdgeFromMap(const TopoDS_Edge&                         theEdge,
+                              TopTools_IndexedDataMapOfShapeListOfShape& theVertexToEdges)
 {
-  TopoDS_Vertex VV[2];
-  TopExp::Vertices(theEdge, VV[0], VV[1]);
-  for (Standard_Integer i = 0; i < 2; i++)
+  bool          anIsRemoved = false;
+  TopoDS_Vertex aFirstVertex;
+  TopoDS_Vertex aLastVertex;
+  TopExp::Vertices(theEdge, aFirstVertex, aLastVertex);
+  for (const auto& aVertex : {aFirstVertex, aLastVertex})
   {
-    if (!theVEmap.Contains(VV[i]))
+    if (!theVertexToEdges.Contains(aVertex))
+    {
       continue;
-    TopTools_ListOfShape&              Elist = theVEmap.ChangeFromKey(VV[i]);
-    TopTools_ListIteratorOfListOfShape itl(Elist);
-    while (itl.More())
+    }
+    TopTools_ListOfShape&              aVertexEdges = theVertexToEdges.ChangeFromKey(aVertex);
+    TopTools_ListIteratorOfListOfShape anEdgesIter(aVertexEdges);
+    while (anEdgesIter.More())
     {
-      const TopoDS_Shape& anEdge = itl.Value();
+      const TopoDS_Shape& anEdge = anEdgesIter.Value();
       if (anEdge.IsSame(theEdge))
-        Elist.Remove(itl);
+      {
+        anIsRemoved = true;
+        aVertexEdges.Remove(anEdgesIter);
+      }
       else
-        itl.Next();
+      {
+        anEdgesIter.Next();
+      }
     }
   }
+  return anIsRemoved;
 }
 
 static Standard_Real ComputeMinEdgeSize(const TopTools_SequenceOfShape& theEdges,
@@ -451,6 +465,32 @@ static Standard_Boolean FindCoordBounds(const TopTools_SequenceOfShape&
   return Standard_True;
 }
 
+//==================================================================================================
+
+// Returns the start and end points of the edge in parametric space of the face.
+// The orientation of the edge is taken into account, so the start and end points
+// will be swapped if the edge has a reversed orientation.
+// @param theEdge The edge to get the points from.
+// @param theRefFace The reference face to get the parametric points.
+// @return A pair of points representing the start and end points of the edge in parametric space.
+static std::pair<gp_Pnt2d, gp_Pnt2d> getCurveParams(const TopoDS_Edge& theEdge,
+                                                    const TopoDS_Face& theRefFace)
+{
+  BRepAdaptor_Curve2d aCurveAdaptor(theEdge, theRefFace);
+  Standard_Real       aFirstParam = aCurveAdaptor.FirstParameter();
+  Standard_Real       aLastParam  = aCurveAdaptor.LastParameter();
+  if (theEdge.Orientation() != TopAbs_FORWARD)
+  {
+    std::swap(aFirstParam, aLastParam);
+  }
+
+  const gp_Pnt2d aFirstPoint = aCurveAdaptor.Value(aFirstParam);
+  const gp_Pnt2d aLastPoint  = aCurveAdaptor.Value(aLastParam);
+  return {aFirstPoint, aLastPoint};
+}
+
+//==================================================================================================
+
 static void RelocatePCurvesToNewUorigin(
   const TopTools_SequenceOfShape&                          theEdges,
   const TopoDS_Shape&                                      theFirstFace,
@@ -462,141 +502,129 @@ static void RelocatePCurvesToNewUorigin(
   NCollection_DataMap<TopoDS_Shape, Handle(Geom2d_Curve)>& theEdgeNewPCurve,
   TopTools_MapOfShape&                                     theUsedEdges)
 {
-  TopTools_MapOfShape EdgesOfFirstFace;
-  TopExp::MapShapes(theFirstFace, EdgesOfFirstFace);
+  TopTools_MapOfShape anEdgesOfFirstFace;
+  TopExp::MapShapes(theFirstFace, anEdgesOfFirstFace);
 
   for (;;) // walk by contours
   {
     // try to find the start edge that:
     // 1. belongs to outer edges of first face;
     // 2. has minimum U-coord of its start point
-    TopoDS_Edge        StartEdge;
-    TopAbs_Orientation anOr      = TopAbs_FORWARD;
-    Standard_Real      aCoordMin = RealLast();
-    for (Standard_Integer ii = 1; ii <= theEdges.Length(); ii++)
+    TopoDS_Edge        aStartEdge;
+    TopAbs_Orientation anOrientation = TopAbs_FORWARD;
+    Standard_Real      aCoordMin     = RealLast();
+    for (Standard_Integer anEdgeIndex = 1; anEdgeIndex <= theEdges.Length(); ++anEdgeIndex)
     {
-      const TopoDS_Edge& anEdge = TopoDS::Edge(theEdges(ii));
+      const TopoDS_Edge& anEdge = TopoDS::Edge(theEdges(anEdgeIndex));
       if (theUsedEdges.Contains(anEdge))
+      {
+        continue;
+      }
+
+      if (!anEdgesOfFirstFace.Contains(anEdge))
+      {
         continue;
-      if (EdgesOfFirstFace.Contains(anEdge))
+      }
+
+      if (aStartEdge.IsNull())
       {
-        if (StartEdge.IsNull())
+        aStartEdge                             = anEdge;
+        const auto&& [aFirstPoint, aLastPoint] = getCurveParams(anEdge, theRefFace);
+        if (aFirstPoint.Coord(theIndCoord) < aLastPoint.Coord(theIndCoord))
         {
-          StartEdge = anEdge;
-          BRepAdaptor_Curve2d StartBAcurve(StartEdge, theRefFace);
-          Standard_Real       aFirstParam, aLastParam;
-          if (StartEdge.Orientation() == TopAbs_FORWARD)
-          {
-            aFirstParam = StartBAcurve.FirstParameter();
-            aLastParam  = StartBAcurve.LastParameter();
-          }
-          else
-          {
-            aFirstParam = StartBAcurve.LastParameter();
-            aLastParam  = StartBAcurve.FirstParameter();
-          }
-          gp_Pnt2d aFirstPoint = StartBAcurve.Value(aFirstParam);
-          gp_Pnt2d aLastPoint  = StartBAcurve.Value(aLastParam);
-          if (aFirstPoint.Coord(theIndCoord) < aLastPoint.Coord(theIndCoord))
-          {
-            aCoordMin = aFirstPoint.Coord(theIndCoord);
-            anOr      = TopAbs_FORWARD;
-          }
-          else
-          {
-            aCoordMin = aLastPoint.Coord(theIndCoord);
-            anOr      = TopAbs_REVERSED;
-          }
+          aCoordMin     = aFirstPoint.Coord(theIndCoord);
+          anOrientation = TopAbs_FORWARD;
         }
         else
         {
-          BRepAdaptor_Curve2d aBAcurve(anEdge, theRefFace);
-          Standard_Real       aFirstParam, aLastParam;
-          if (anEdge.Orientation() == TopAbs_FORWARD)
-          {
-            aFirstParam = aBAcurve.FirstParameter();
-            aLastParam  = aBAcurve.LastParameter();
-          }
-          else
-          {
-            aFirstParam = aBAcurve.LastParameter();
-            aLastParam  = aBAcurve.FirstParameter();
-          }
-          gp_Pnt2d aFirstPoint = aBAcurve.Value(aFirstParam);
-          gp_Pnt2d aLastPoint  = aBAcurve.Value(aLastParam);
-          if (aFirstPoint.Coord(theIndCoord) < aCoordMin)
-          {
-            StartEdge = anEdge;
-            aCoordMin = aFirstPoint.Coord(theIndCoord);
-            anOr      = TopAbs_FORWARD;
-          }
-          if (aLastPoint.Coord(theIndCoord) < aCoordMin)
-          {
-            StartEdge = anEdge;
-            aCoordMin = aLastPoint.Coord(theIndCoord);
-            anOr      = TopAbs_REVERSED;
-          }
+          aCoordMin     = aLastPoint.Coord(theIndCoord);
+          anOrientation = TopAbs_REVERSED;
         }
-      } // if (EdgesOfFirstFace.Contains(anEdge))
-    } // for (Standard_Integer ii = 1; ii <= edges.Length(); ii++)
+      }
+      else
+      {
+        const auto&& [aFirstPoint, aLastPoint] = getCurveParams(anEdge, theRefFace);
+        if (aFirstPoint.Coord(theIndCoord) < aCoordMin)
+        {
+          aStartEdge    = anEdge;
+          aCoordMin     = aFirstPoint.Coord(theIndCoord);
+          anOrientation = TopAbs_FORWARD;
+        }
+        if (aLastPoint.Coord(theIndCoord) < aCoordMin)
+        {
+          aStartEdge    = anEdge;
+          aCoordMin     = aLastPoint.Coord(theIndCoord);
+          anOrientation = TopAbs_REVERSED;
+        }
+      }
+    }
 
-    if (StartEdge.IsNull()) // all contours are passed
+    if (aStartEdge.IsNull()) // all contours are passed
+    {
       break;
+    }
 
-    TopoDS_Vertex        StartVertex = (anOr == TopAbs_FORWARD)
-                                         ? TopExp::FirstVertex(StartEdge, Standard_True)
-                                         : TopExp::LastVertex(StartEdge, Standard_True);
-    TopoDS_Edge          CurEdge     = StartEdge;
-    Standard_Real        fpar, lpar;
-    Handle(Geom2d_Curve) CurPCurve = BRep_Tool::CurveOnSurface(CurEdge, theRefFace, fpar, lpar);
-    CurPCurve                      = new Geom2d_TrimmedCurve(CurPCurve, fpar, lpar);
-    theEdgeNewPCurve.Bind(CurEdge, CurPCurve);
-    if (CurEdge.Orientation() == TopAbs_REVERSED)
+    TopoDS_Edge          aCurrentEdge = aStartEdge;
+    Standard_Real        anEdgeStartParam, anEdgeEndParam;
+    Handle(Geom2d_Curve) CurPCurve =
+      BRep_Tool::CurveOnSurface(aCurrentEdge, theRefFace, anEdgeStartParam, anEdgeEndParam);
+    CurPCurve = new Geom2d_TrimmedCurve(CurPCurve, anEdgeStartParam, anEdgeEndParam);
+    theEdgeNewPCurve.Bind(aCurrentEdge, CurPCurve);
+    if (aCurrentEdge.Orientation() == TopAbs_REVERSED)
     {
-      Standard_Real tmp = fpar;
-      fpar              = lpar;
-      lpar              = tmp;
+      std::swap(anEdgeStartParam, anEdgeEndParam);
     }
-    Standard_Real CurParam = (anOr == TopAbs_FORWARD) ? lpar : fpar;
+    Standard_Real CurParam = (anOrientation == TopAbs_FORWARD) ? anEdgeEndParam : anEdgeStartParam;
     gp_Pnt2d      CurPoint = CurPCurve->Value(CurParam);
 
     for (;;) // collect pcurves of a contour
     {
-      RemoveEdgeFromMap(CurEdge, theVEmap);
-      theUsedEdges.Add(CurEdge);
-      TopoDS_Vertex CurVertex = (anOr == TopAbs_FORWARD)
-                                  ? TopExp::LastVertex(CurEdge, Standard_True)
-                                  : TopExp::FirstVertex(CurEdge, Standard_True);
+      if (!RemoveEdgeFromMap(aCurrentEdge, theVEmap))
+      {
+        break; // end of contour in 2d
+      }
+      theUsedEdges.Add(aCurrentEdge);
+      TopoDS_Vertex CurVertex = (anOrientation == TopAbs_FORWARD)
+                                  ? TopExp::LastVertex(aCurrentEdge, Standard_True)
+                                  : TopExp::FirstVertex(aCurrentEdge, Standard_True);
 
       const TopTools_ListOfShape& Elist = theVEmap.FindFromKey(CurVertex);
       if (Elist.IsEmpty())
+      {
         break; // end of contour in 3d
+      }
 
-      TopTools_ListIteratorOfListOfShape itl(Elist);
-      for (; itl.More(); itl.Next())
+      for (TopTools_ListIteratorOfListOfShape itl(Elist); itl.More(); itl.Next())
       {
         const TopoDS_Edge& anEdge = TopoDS::Edge(itl.Value());
-        if (anEdge.IsSame(CurEdge))
+        if (anEdge.IsSame(aCurrentEdge))
+        {
           continue;
-        TopoDS_Vertex aFirstVertex = (anOr == TopAbs_FORWARD)
+        }
+
+        TopoDS_Vertex aFirstVertex = (anOrientation == TopAbs_FORWARD)
                                        ? TopExp::FirstVertex(anEdge, Standard_True)
                                        : TopExp::LastVertex(anEdge, Standard_True);
         if (!aFirstVertex.IsSame(CurVertex)) // may be if CurVertex is deg.vertex
+        {
           continue;
+        }
 
-        Handle(Geom2d_Curve) aPCurve = BRep_Tool::CurveOnSurface(anEdge, theRefFace, fpar, lpar);
-        aPCurve                      = new Geom2d_TrimmedCurve(aPCurve, fpar, lpar);
+        Handle(Geom2d_Curve) aPCurve =
+          BRep_Tool::CurveOnSurface(anEdge, theRefFace, anEdgeStartParam, anEdgeEndParam);
+        aPCurve = new Geom2d_TrimmedCurve(aPCurve, anEdgeStartParam, anEdgeEndParam);
         if (anEdge.Orientation() == TopAbs_REVERSED)
         {
-          Standard_Real tmp = fpar;
-          fpar              = lpar;
-          lpar              = tmp;
+          std::swap(anEdgeStartParam, anEdgeEndParam);
         }
-        Standard_Real aParam   = (anOr == TopAbs_FORWARD) ? fpar : lpar;
+        Standard_Real aParam =
+          (anOrientation == TopAbs_FORWARD) ? anEdgeStartParam : anEdgeEndParam;
         gp_Pnt2d      aPoint   = aPCurve->Value(aParam);
         Standard_Real anOffset = CurPoint.Coord(theIndCoord) - aPoint.Coord(theIndCoord);
         if (!(Abs(anOffset) < theCoordTol || Abs(Abs(anOffset) - thePeriod) < theCoordTol))
+        {
           continue; // may be if CurVertex is deg.vertex
+        }
 
         if (Abs(anOffset) > thePeriod / 2)
         {
@@ -611,8 +639,8 @@ static void RelocatePCurvesToNewUorigin(
           aPCurve = aNewPCurve;
         }
         theEdgeNewPCurve.Bind(anEdge, aPCurve);
-        CurEdge                  = anEdge;
-        TopAbs_Orientation CurOr = TopAbs::Compose(anOr, CurEdge.Orientation());
+        aCurrentEdge             = anEdge;
+        TopAbs_Orientation CurOr = TopAbs::Compose(anOrientation, aCurrentEdge.Orientation());
         CurParam = (CurOr == TopAbs_FORWARD) ? aPCurve->LastParameter() : aPCurve->FirstParameter();
         CurPoint = aPCurve->Value(CurParam);
         break;

diff --git a/tests/bugs/modalg_8/bug_gh544 b/tests/bugs/modalg_8/bug_gh544
new file mode 100644 (file)
index 0000000..06280a3
--- /dev/null
+++ b/tests/bugs/modalg_8/bug_gh544
@@ -0,0 +1,29 @@
+# This test is to check the fix for issue #544
+# We have to make sure that boolean operation doesn't stuck in infinite loop.
+
+# Load the base solid
+restore [locate_data_file bug_gh544.brep] body
+# restore solid.brep body
+
+# get all the wires
+explode body W
+
+# make it a face
+mkplane f body_18
+
+# prism it, z = 12
+prism p f 0 0 12
+
+# settings to fuse
+bclearobjects
+bcleartools
+baddobjects p
+baddtools body
+bfillds
+
+# set simplify to true
+bsimplify -f 1
+
+# If this doesn't stuck in infinite loop, behavior is correct.
+# If it does, it will be killed by the timeout eventually.
+bapibop res_simple 1