diff --git a/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_Face.cxx b/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_Face.cxx index 20879a1db7..8556543a70 100644 --- a/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_Face.cxx +++ b/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_Face.cxx @@ -261,6 +261,9 @@ static bool SplitWire(const TopoDS_Face& face, occ::handle sewd1 = new ShapeExtend_WireData; sewd1->Add(E1); bool IsConnectedEdge = true; + // Set to true when null pcurves prevent the 2D closure verification: + // the partial wire collected so far is discarded for this start edge. + bool aIsAbandonedSplit = false; for (j = 2; j <= sewd->NbEdges() && IsConnectedEdge; j++) { TopoDS_Edge E2; @@ -294,9 +297,9 @@ static bool SplitWire(const TopoDS_Face& face, occ::handle curve2 = BRep_Tool::CurveOnSurface(E2, face, a2, b2); if (curve1.IsNull() || curve2.IsNull()) { - continue; + aIsAbandonedSplit = true; + break; } - gp_Pnt2d v0, v1; if (E1.Orientation() == TopAbs_REVERSED) { @@ -319,6 +322,10 @@ static bool SplitWire(const TopoDS_Face& face, } } } + if (aIsAbandonedSplit) + { + continue; + } if (!IsConnectedEdge) { // create new notclosed wire diff --git a/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_IntersectionTool.cxx b/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_IntersectionTool.cxx index b5bb2dc52d..ab21649985 100644 --- a/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_IntersectionTool.cxx +++ b/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_IntersectionTool.cxx @@ -1842,7 +1842,6 @@ bool ShapeFix_IntersectionTool::FixIntersectingWires(TopoDS_Face& face) const // TopoDS_Shape SF = TopoDS::Face(S); TopoDS_Shape SF = face; TopAbs_Orientation ori = face.Orientation(); - NCollection_Map anAddedWires; NCollection_Sequence SeqWir; NCollection_Sequence SeqNMShapes; for (TopoDS_Iterator iter(SF, false); iter.More(); iter.Next()) @@ -1855,11 +1854,7 @@ bool ShapeFix_IntersectionTool::FixIntersectingWires(TopoDS_Face& face) const continue; } TopoDS_Wire wire = TopoDS::Wire(iter.Value()); - // Prevent duplicate wires from being added to the sequence - if (anAddedWires.Add(wire)) - { - SeqWir.Append(wire); - } + SeqWir.Append(wire); } if (SeqWir.Length() < 2) { diff --git a/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_Wire.cxx b/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_Wire.cxx index 03c7ba07f2..76bf20b640 100644 --- a/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_Wire.cxx +++ b/src/ModelingAlgorithms/TKShHealing/ShapeFix/ShapeFix_Wire.cxx @@ -562,16 +562,31 @@ bool ShapeFix_Wire::FixConnected(const double prec) return false; } - int stop = (myClosedMode ? 0 : 1); - for (int i = NbEdges(); i > stop; i--) + const int aStop = (myClosedMode ? 0 : 1); + occ::handle aWireSBWD = WireData(); + for (int aI = NbEdges(); aI > aStop; aI--) { - // Call without UpdateWire to avoid O(n^2) behavior in the loop - FixConnected(i, prec, false); + FixConnected(aI, prec, false); myStatusConnected |= myLastFixStatus; + // Refresh the edge that the next iteration will analyze as n1. + if (Context().IsNull() || aI - 1 <= aStop) + { + continue; + } + const int aN1Next = (aI - 1 > 1) ? aI - 2 : aWireSBWD->NbEdges(); + if (aN1Next == aI || aN1Next == aI - 1) + { + continue; + } + const TopoDS_Edge aEPrev = aWireSBWD->Edge(aN1Next); + const TopoDS_Shape aRefresh = Context()->Apply(aEPrev); + if (aRefresh.IsNull() || aRefresh.IsSame(aEPrev) || aRefresh.ShapeType() != TopAbs_EDGE) + { + continue; + } + aWireSBWD->Set(TopoDS::Edge(aRefresh), aN1Next); } - // Update wire once after all connections are fixed - // Using Value() in UpdateWire() prevents edge explosion from replacement chains if (!Context().IsNull()) { UpdateWire(); @@ -4280,11 +4295,9 @@ void ShapeFix_Wire::UpdateWire() occ::handle sbwd = WireData(); for (int i = 1; i <= sbwd->NbEdges(); i++) { - TopoDS_Edge E = sbwd->Edge(i); - // ValueLeaf follows the replacement chain without descending into sub-shapes, - // so a previously-split edge is not re-expanded on subsequent Perform() passes. - TopoDS_Shape S = Context()->ValueLeaf(E); - if (!S.IsNull() && S.IsEqual(E)) + TopoDS_Edge E = sbwd->Edge(i); + TopoDS_Shape S = Context()->Apply(E); + if (S == E) { continue; } diff --git a/src/ModelingAlgorithms/TKShHealing/ShapeProcess/ShapeProcess_ShapeContext.cxx b/src/ModelingAlgorithms/TKShHealing/ShapeProcess/ShapeProcess_ShapeContext.cxx index 8f2dcb8fea..4336e57084 100644 --- a/src/ModelingAlgorithms/TKShHealing/ShapeProcess/ShapeProcess_ShapeContext.cxx +++ b/src/ModelingAlgorithms/TKShHealing/ShapeProcess/ShapeProcess_ShapeContext.cxx @@ -314,17 +314,9 @@ static void RecModif(const TopoDS_Shape& { next = S; } - - // Map to prevent infinite loops in case of cyclic replacements. - NCollection_Map aVisitedShapes; do { cur = next; - if (!aVisitedShapes.Add(cur)) - { - break; - } - if (msgmap.IsBound(cur)) { const NCollection_List& msglist = msgmap.Find(cur); diff --git a/src/ModelingData/TKBRep/BRepTools/BRepTools_ReShape.cxx b/src/ModelingData/TKBRep/BRepTools/BRepTools_ReShape.cxx index 6b074d5734..33011f1436 100644 --- a/src/ModelingData/TKBRep/BRepTools/BRepTools_ReShape.cxx +++ b/src/ModelingData/TKBRep/BRepTools/BRepTools_ReShape.cxx @@ -200,48 +200,10 @@ void BRepTools_ReShape::replace(const TopoDS_Shape& ashape, std::cout << "Warning: BRepTools_ReShape::Replace: shape already recorded" << std::endl; #endif - // Reject replacements that would introduce a cycle into the replacement chain, - // e.g. A -> ... -> X -> A. Walk forward from newshape via Value(); if the walk - // ever lands on shape itself, record only an identity (effectively no-op) to - // avoid forming a cycle that would later deadlock Apply()/ValueLeaf(). - if (theKind != TReplacementKind_Remove && !newshape.IsNull() && !newshape.IsPartner(shape)) - { - // Reject replacements that would close a cycle in the map. Walk forward from - // newshape via Value(); if the chain ever lands back on shape's underlying TShape - // (any orientation, any location), abort. Key by the TShape handle to mirror the - // identity the map itself uses once orientation/location are normalized away. - TopoDS_Shape aProbe = newshape; - NCollection_Map> aSeen; - aSeen.Add(shape.TShape()); - aSeen.Add(aProbe.TShape()); - bool aCycle = false; - for (;;) - { - const TopoDS_Shape aNext = Value(aProbe); - if (aNext.IsNull() || aNext.IsSame(aProbe)) - { - break; - } - if (aNext.IsPartner(shape)) - { - aCycle = true; - break; - } - if (!aSeen.Add(aNext.TShape())) - { - break; // existing cycle in data - not ours to introduce, bail - } - aProbe = aNext; - } - if (aCycle) - { -#ifdef OCCT_DEBUG - std::cout << "Warning: BRepTools_ReShape::Replace: cycle rejected" << std::endl; -#endif - return; - } - } - + // Cycle handling: cycles in the replacement map (A -> B -> A or longer) are + // accepted at insertion time. The DFS in-flight guard inside Apply()/applyImpl() + // breaks any cycle at traversal time, returning the immediate Value() instead + // of recursing. myShapeToReplacement.Bind(shape, TReplacement(newshape, theKind)); myNewShapes.Add(newshape); } diff --git a/src/ModelingData/TKBRep/GTests/BRepTools_ReShape_Test.cxx b/src/ModelingData/TKBRep/GTests/BRepTools_ReShape_Test.cxx index cea31ad1a3..7466f97707 100644 --- a/src/ModelingData/TKBRep/GTests/BRepTools_ReShape_Test.cxx +++ b/src/ModelingData/TKBRep/GTests/BRepTools_ReShape_Test.cxx @@ -78,24 +78,30 @@ TEST(BRepTools_ReShapeTest, ValueLeaf_ChainEndingInRemoveReturnsNull) EXPECT_TRUE(aReShape.ValueLeaf(aA).IsNull()); } -// A direct A->B then B->A would close a cycle in the replacement map. -// The second Replace() must be rejected; the first stays intact. -TEST(BRepTools_ReShapeTest, Replace_RejectsDirectCycle) +// A direct A->B then B->A creates a cycle in the replacement map; both bindings +// are recorded, and the DFS in-flight guard inside Apply() handles termination. +TEST(BRepTools_ReShapeTest, Replace_DirectCycleHandledByApply) { const TopoDS_Vertex aA = MakeVertex(0, 0, 0); const TopoDS_Vertex aB = MakeVertex(1, 0, 0); BRepTools_ReShape aReShape; aReShape.Replace(aA, aB); - aReShape.Replace(aB, aA); // would create A -> B -> A + aReShape.Replace(aB, aA); - EXPECT_TRUE(aReShape.Value(aA).IsSame(aB)) << "First replacement must remain in effect"; - EXPECT_TRUE(aReShape.Value(aB).IsSame(aB)) << "Cyclic second replacement must have been rejected"; - EXPECT_TRUE(aReShape.ValueLeaf(aA).IsSame(aB)) << "Chain must terminate, not loop"; + EXPECT_TRUE(aReShape.Value(aA).IsSame(aB)); + EXPECT_TRUE(aReShape.Value(aB).IsSame(aA)); + + // Apply() must terminate (DFS in-flight guard) and return a defined shape. + const TopoDS_Shape aResult = aReShape.Apply(aA); + EXPECT_FALSE(aResult.IsNull()) << "Apply must terminate via the DFS guard"; } -// A longer cycle A -> B -> C -> A must also be rejected at the closing edge. -TEST(BRepTools_ReShapeTest, Replace_RejectsLongerCycle) +// Longer cycles A -> B -> C -> A are accepted at Replace() time and broken at +// traversal time by the DFS in-flight guard inside Apply(). This avoids +// over-rejection in IGES/STEP healing pipelines where shapes legitimately +// alias TShapes across stages. +TEST(BRepTools_ReShapeTest, Replace_LongerCycleHandledByApply) { const TopoDS_Vertex aA = MakeVertex(0, 0, 0); const TopoDS_Vertex aB = MakeVertex(1, 0, 0); @@ -104,10 +110,15 @@ TEST(BRepTools_ReShapeTest, Replace_RejectsLongerCycle) BRepTools_ReShape aReShape; aReShape.Replace(aA, aB); aReShape.Replace(aB, aC); - aReShape.Replace(aC, aA); // would create A -> B -> C -> A + aReShape.Replace(aC, aA); - EXPECT_TRUE(aReShape.ValueLeaf(aA).IsSame(aC)) << "Chain terminates at C; no cycle formed"; - EXPECT_TRUE(aReShape.Value(aC).IsSame(aC)) << "Closing replacement was rejected"; + EXPECT_TRUE(aReShape.Value(aA).IsSame(aB)); + EXPECT_TRUE(aReShape.Value(aB).IsSame(aC)); + EXPECT_TRUE(aReShape.Value(aC).IsSame(aA)); + + // Apply() must terminate (DFS in-flight guard) and return a defined shape. + const TopoDS_Shape aResult = aReShape.Apply(aA); + EXPECT_FALSE(aResult.IsNull()) << "Apply must terminate via the DFS guard"; } // Apply() must not stack-overflow when a shape's replacement is a compound that diff --git a/tests/de/step_2/R7 b/tests/de/step_2/R7 index 61eb1ef4e7..0d62a31269 100644 --- a/tests/de/step_2/R7 +++ b/tests/de/step_2/R7 @@ -7,10 +7,10 @@ set filename trj10_b2-oc-214.stp set ref_data { DATA : Faulties = 0 ( 0 ) Warnings = 0 ( 0 ) Summary = 0 ( 0 ) TPSTAT : Faulties = 0 ( 0 ) Warnings = 5 ( 3 ) Summary = 5 ( 3 ) -CHECKSHAPE : Wires = 1 ( 1 ) Faces = 1 ( 1 ) Shells = 0 ( 0 ) Solids = 0 ( 0 ) -NBSHAPES : Solid = 0 ( 0 ) Shell = 1 ( 1 ) Face = 250 ( 250 ) -STATSHAPE : Solid = 0 ( 0 ) Shell = 1 ( 1 ) Face = 250 ( 250 ) FreeWire = 0 ( 0 ) -TOLERANCE : MaxTol = 0.002562014753 ( 0.00500444492 ) AvgTol = 2.347098051e-005 ( 7.280140961e-005 ) +CHECKSHAPE : Wires = 0 ( 0 ) Faces = 0 ( 0 ) Shells = 0 ( 0 ) Solids = 0 ( 0 ) +NBSHAPES : Solid = 1 ( 1 ) Shell = 1 ( 1 ) Face = 251 ( 251 ) +STATSHAPE : Solid = 1 ( 1 ) Shell = 1 ( 1 ) Face = 251 ( 251 ) FreeWire = 0 ( 0 ) +TOLERANCE : MaxTol = 0.002562014753 ( 0.00500444492 ) AvgTol = 2.347098051e-05 ( 7.280140961e-05 ) LABELS : N0Labels = 1 ( 1 ) N1Labels = 0 ( 0 ) N2Labels = 0 ( 0 ) TotalLabels = 1 ( 1 ) NameLabels = 1 ( 1 ) ColorLabels = 0 ( 0 ) LayerLabels = 0 ( 0 ) PROPS : Centroid = 1 ( 1 ) Volume = 1 ( 1 ) Area = 1 ( 1 ) NCOLORS : NColors = 0 ( 0 )