From 7ed396b0ebb33a12cc15714b2c83ae7e29825950 Mon Sep 17 00:00:00 2001 From: Dmitrii Kulikov <164657232+AtheneNoctuaPt@users.noreply.github.com> Date: Thu, 22 May 2025 11:29:54 +0100 Subject: [PATCH] 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. --- .../ShapeUpgrade_UnifySameDomain.cxx | 250 ++++++++++-------- tests/bugs/modalg_8/bug_gh544 | 29 ++ 2 files changed, 168 insertions(+), 111 deletions(-) create mode 100644 tests/bugs/modalg_8/bug_gh544 diff --git a/src/ModelingAlgorithms/TKShHealing/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.cxx b/src/ModelingAlgorithms/TKShHealing/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.cxx index c208deb9d0..a4c9760d3b 100644 --- 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])) - continue; - TopTools_ListOfShape& Elist = theVEmap.ChangeFromKey(VV[i]); - TopTools_ListIteratorOfListOfShape itl(Elist); - while (itl.More()) + if (!theVertexToEdges.Contains(aVertex)) { - const TopoDS_Shape& anEdge = itl.Value(); + continue; + } + TopTools_ListOfShape& aVertexEdges = theVertexToEdges.ChangeFromKey(aVertex); + TopTools_ListIteratorOfListOfShape anEdgesIter(aVertexEdges); + while (anEdgesIter.More()) + { + 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 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& 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 (EdgesOfFirstFace.Contains(anEdge)) { - if (StartEdge.IsNull()) + continue; + } + + if (!anEdgesOfFirstFace.Contains(anEdge)) + { + continue; + } + + if (aStartEdge.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++) - - if (StartEdge.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) - { - Standard_Real tmp = fpar; - fpar = lpar; - lpar = tmp; + } + 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; + } + } } - Standard_Real CurParam = (anOr == TopAbs_FORWARD) ? lpar : fpar; + + if (aStartEdge.IsNull()) // all contours are passed + { + break; + } + + 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) + { + std::swap(anEdgeStartParam, anEdgeEndParam); + } + 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 index 0000000000..06280a33ed --- /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