From dd458d72fb454e9a9922fee6aa441853fe1da2cd Mon Sep 17 00:00:00 2001 From: jfa Date: Thu, 16 Feb 2017 12:47:37 +0300 Subject: [PATCH] 0023411: [CEA 2023] Bug MakeFillet1D SALOME master --- src/GEOMImpl/GEOMImpl_Fillet1d.cxx | 10 +++- src/GEOMImpl/GEOMImpl_Fillet1dDriver.cxx | 73 ++++++++++++++++++------ 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/src/GEOMImpl/GEOMImpl_Fillet1d.cxx b/src/GEOMImpl/GEOMImpl_Fillet1d.cxx index f25690b5a..7b6cd60e1 100644 --- a/src/GEOMImpl/GEOMImpl_Fillet1d.cxx +++ b/src/GEOMImpl/GEOMImpl_Fillet1d.cxx @@ -65,7 +65,7 @@ static Standard_Boolean IsDivideEdge(const TopoDS_Edge &theEdge, gp_Pnt aPStart = aCurve->Value(theStart); gp_Pnt aPEnd = aCurve->Value(theEnd); TopoDS_Vertex aVFirst = TopExp::FirstVertex(theEdge); - TopoDS_Vertex aVLast = TopExp::FirstVertex(theEdge); + TopoDS_Vertex aVLast = TopExp::LastVertex(theEdge); Standard_Real aTolFirst = BRep_Tool::Tolerance(aVFirst); Standard_Real aTolLast = BRep_Tool::Tolerance(aVLast); Standard_Real aTolConf = Precision::Confusion(); @@ -81,7 +81,11 @@ static Standard_Boolean IsDivideEdge(const TopoDS_Edge &theEdge, aDistSL <= aTolLast + aTolConf) { if (aDistEF <= aTolFirst + aTolConf || aDistEL <= aTolLast + aTolConf) { - isSplit = Standard_False; + + isSplit = Standard_False; + // in this case the original edge is thrown, and distance (gap) from new arc end + // to a vertex of original wire can reach (aVertexTolerance + Precision::Confusion()). + // Resulting wire is fixed (Mantis issue 0023411) in GEOMImpl_Fillet1dDriver::MakeFillet() } } @@ -367,7 +371,7 @@ Standard_Boolean GEOMImpl_Fillet1d::Perform(const Standard_Real theRadius) aParam2 > myStart1 + aTol && aParam2 < myEnd1 - aTol) { // Add the point in the list in increasing order. const Standard_Real aParam = 0.5*(aParam1 + aParam2); - + for(anIter.Initialize(aParams); anIter.More(); anIter.Next()) { if (anIter.Value() > aParam) { aParams.InsertBefore(aParam, anIter); diff --git a/src/GEOMImpl/GEOMImpl_Fillet1dDriver.cxx b/src/GEOMImpl/GEOMImpl_Fillet1dDriver.cxx index 34efa40cd..bd2ece120 100644 --- a/src/GEOMImpl/GEOMImpl_Fillet1dDriver.cxx +++ b/src/GEOMImpl/GEOMImpl_Fillet1dDriver.cxx @@ -48,6 +48,7 @@ #include #include +#include #include #include #include @@ -270,11 +271,11 @@ Standard_Integer GEOMImpl_Fillet1dDriver::Execute(LOGBOOK& log) const //function : MakeFillet //purpose : //======================================================================= -bool GEOMImpl_Fillet1dDriver::MakeFillet(const TopoDS_Wire& aWire, - const TopTools_ListOfShape& aVertexList, - const Standard_Real rad, +bool GEOMImpl_Fillet1dDriver::MakeFillet(const TopoDS_Wire& theWire, + const TopTools_ListOfShape& theVertexList, + const Standard_Real theRadius, bool isFinalPass, - TopoDS_Wire& aResult) const + TopoDS_Wire& theResult) const { // this variable is needed to break execution // in case of fillet failure and try to fuse edges @@ -290,8 +291,8 @@ bool GEOMImpl_Fillet1dDriver::MakeFillet(const TopoDS_Wire& aWire, TopTools_ListOfShape aListOfNewEdge; // remember relation between initial and modified map TopTools_IndexedDataMapOfShapeListOfShape aMapVToEdges; - TopExp::MapShapesAndAncestors( aWire, TopAbs_VERTEX, TopAbs_EDGE, aMapVToEdges ); - TopTools_ListIteratorOfListOfShape anIt( aVertexList ); + TopExp::MapShapesAndAncestors( theWire, TopAbs_VERTEX, TopAbs_EDGE, aMapVToEdges ); + TopTools_ListIteratorOfListOfShape anIt( theVertexList ); for ( ; anIt.More(); anIt.Next() ) { TopoDS_Vertex aV = TopoDS::Vertex( anIt.Value() ); if ( aV.IsNull() || !aMapVToEdges.Contains( aV ) ) @@ -313,7 +314,7 @@ bool GEOMImpl_Fillet1dDriver::MakeFillet(const TopoDS_Wire& aWire, continue; // seems edges does not belong to same plane or parallel (fillet can not be build) GEOMImpl_Fillet1d aFilletAlgo (anEdge1, anEdge2, aPlane); - if (!aFilletAlgo.Perform(rad)) { + if (!aFilletAlgo.Perform(theRadius)) { if (isFinalPass) continue; // can not create fillet with given radius else { @@ -356,23 +357,59 @@ bool GEOMImpl_Fillet1dDriver::MakeFillet(const TopoDS_Wire& aWire, return false; // create new wire instead of original - for (TopExp_Explorer anExp (aWire, TopAbs_EDGE); anExp.More(); anExp.Next()) { + Standard_Real aTol; + Standard_Real aVertMaxTol = -RealLast(); + for (TopExp_Explorer anExp (theWire, TopAbs_EDGE); anExp.More(); anExp.Next()) { TopoDS_Shape anEdge = anExp.Current(); if (!anEdgeToEdgeMap.IsBound(anEdge)) aListOfNewEdge.Append(anEdge); else if (!anEdgeToEdgeMap.Find(anEdge).IsNull()) aListOfNewEdge.Append(anEdgeToEdgeMap.Find(anEdge)); - } - - GEOMUtils::SortShapes(aListOfNewEdge); - BRepBuilderAPI_MakeWire aWireTool; - aWireTool.Add(aListOfNewEdge); - aWireTool.Build(); - if (!aWireTool.IsDone()) - return 0; + // calculate maximum vertex tolerance of the initial wire + // to be used for resulting wire fixing (some gaps are possible) + for (TopExp_Explorer anExV (anEdge, TopAbs_VERTEX); anExV.More(); anExV.Next()) { + TopoDS_Vertex aVert = TopoDS::Vertex(anExV.Current()); + aTol = BRep_Tool::Tolerance(aVert); + if (aTol > aVertMaxTol) + aVertMaxTol = aTol; + } + } - aResult = aWireTool.Wire(); + // Fix for Mantis issue 0023411: BEGIN + BRep_Builder B; + TopoDS_Wire aResWire; + B.MakeWire(aResWire); + TopTools_ListIteratorOfListOfShape anItNewEdge (aListOfNewEdge); + for (; anItNewEdge.More(); anItNewEdge.Next()) { + B.Add(aResWire, TopoDS::Edge(anItNewEdge.Value())); + } + Handle(ShapeFix_Wire) aFW = new ShapeFix_Wire; + aFW->Load(aResWire); + aFW->FixReorder(); + aFW->ClosedWireMode() = theWire.Closed(); + // Fix for Mantis issue 0023411 + // We forced to do this because of fillet 1d algorithm feature + // (see function DivideEdge in file GEOMImpl_Fillet1d.cxx): + // a distance (gap) from new arc end to a vertex of original wire + // can reach (aVertexTolerance + Precision::Confusion()), + // so a distance between two adjacent arcs will be covered by value below + aFW->FixConnected(aVertMaxTol*2.0 + Precision::Confusion()*3.0); + theResult = aFW->WireAPIMake(); + GEOMUtils::FixShapeTolerance(theResult, TopAbs_VERTEX, Precision::Confusion()); + + // In OCCT 7.0.0 and earlier this gap was successfully covered by + // implementation of BRepBuilderAPI_MakeWire::Add(TopTools_ListOfShape) + // (in the case described in Mantis issue 0023411) + + //GEOMUtils::SortShapes(aListOfNewEdge); + //BRepBuilderAPI_MakeWire aWireTool; + //aWireTool.Add(aListOfNewEdge); + //aWireTool.Build(); + //if (!aWireTool.IsDone()) + // return 0; + //theResult = aWireTool.Wire(); + // Fix for Mantis issue 0023411: END return isAllStepsOk; } @@ -408,7 +445,7 @@ GetCreationInformation(std::string& theOperationName, default: return false; } - + return true; } -- 2.30.2