From a8232603a065cf91e448d3e70dab2ff056769a59 Mon Sep 17 00:00:00 2001 From: emv Date: Fri, 23 Dec 2016 16:04:25 +0300 Subject: [PATCH] 0028273: Inefficient implementation of the BRepOffset_MakeOffset::BuildSplitsOfFace function The method for building splits of the face BRepOffset_MakeOffset::BuildSplitsOfFace has been redesigned to use the BOPAlgo_BuilderFace algorithm directly. The methods FindInvalidFaces and TrimNewIntersectionEdges have been corrected to avoid possible instabilities. The filtering of invalid edges by existing bounding edges has been corrected to avoid removal of valid splits. The method BRepOffset_Analyse::Correct2dPoint has been modified to avoid unnecessary initialization of BRepAdaptor_Surface. Adjustment of the test cases. Test case bugs modalg_6 bug26513 has been modified to unify faces of the input shape before operation. --- src/BRepOffset/BRepOffset_Analyse.cxx | 80 +++++++------ src/BRepOffset/BRepOffset_MakeOffset_1.cxx | 131 +++++++++++++-------- tests/bugs/modalg_6/bug26513 | 21 ++-- tests/offset/shape_type_i_c/XD3 | 11 +- tests/offset/shape_type_i_c/ZD9 | 5 +- 5 files changed, 140 insertions(+), 108 deletions(-) diff --git a/src/BRepOffset/BRepOffset_Analyse.cxx b/src/BRepOffset/BRepOffset_Analyse.cxx index 825abd54f3..ca2281ae08 100644 --- a/src/BRepOffset/BRepOffset_Analyse.cxx +++ b/src/BRepOffset/BRepOffset_Analyse.cxx @@ -46,7 +46,7 @@ #include // -static void Correct2dPoint(const Adaptor3d_Surface& theS, gp_Pnt2d& theP2d); +static void Correct2dPoint(const TopoDS_Face& theF, gp_Pnt2d& theP2d); // static BRepOffset_Type DefineConnectType(const TopoDS_Edge& E, const TopoDS_Face& F1, @@ -484,40 +484,44 @@ void BRepOffset_Analyse::AddFaces (const TopoDS_Face& Face, //function : Correct2dPoint //purpose : //======================================================================= -void Correct2dPoint(const Adaptor3d_Surface& theS, gp_Pnt2d& theP2d) +void Correct2dPoint(const TopoDS_Face& theF, gp_Pnt2d& theP2d) { + BRepAdaptor_Surface aBAS(theF, Standard_False); + if (aBAS.GetType() < GeomAbs_BezierSurface) { + return; + } + // const Standard_Real coeff = 0.01; Standard_Real eps; Standard_Real u1, u2, v1, v2; - if(theS.GetType() >= GeomAbs_BezierSurface) + // + aBAS.Initialize(theF, Standard_True); + u1 = aBAS.FirstUParameter(); + u2 = aBAS.LastUParameter(); + v1 = aBAS.FirstVParameter(); + v2 = aBAS.LastVParameter(); + if (!(Precision::IsInfinite(u1) || Precision::IsInfinite(u2))) { - u1 = theS.FirstUParameter(); - u2 = theS.LastUParameter(); - v1 = theS.FirstVParameter(); - v2 = theS.LastVParameter(); - if(!(Precision::IsInfinite(u1) || Precision::IsInfinite(u2))) + eps = Max(coeff*(u2 - u1), Precision::PConfusion()); + if (Abs(theP2d.X() - u1) < eps) { - eps = Max(coeff*(u2-u1), Precision::PConfusion()); - if(Abs(theP2d.X()-u1) < eps) - { - theP2d.SetX(u1 + eps); - } - if(Abs(theP2d.X()-u2) < eps) - { - theP2d.SetX(u2 - eps); - } + theP2d.SetX(u1 + eps); } - if(!(Precision::IsInfinite(v1) || Precision::IsInfinite(v2))) + if (Abs(theP2d.X() - u2) < eps) { - eps = Max(coeff*(v2-v1), Precision::PConfusion()); - if(Abs(theP2d.Y()-v1) < eps) - { - theP2d.SetY(v1 + eps); - } - if(Abs(theP2d.Y()-v2) < eps) - { - theP2d.SetY(v2 - eps); - } + theP2d.SetX(u2 - eps); + } + } + if (!(Precision::IsInfinite(v1) || Precision::IsInfinite(v2))) + { + eps = Max(coeff*(v2 - v1), Precision::PConfusion()); + if (Abs(theP2d.Y() - v1) < eps) + { + theP2d.SetY(v1 + eps); + } + if (Abs(theP2d.Y() - v2) < eps) + { + theP2d.SetY(v2 - eps); } } } @@ -526,16 +530,18 @@ void Correct2dPoint(const Adaptor3d_Surface& theS, gp_Pnt2d& theP2d) //function : DefineConnectType //purpose : //======================================================================= -BRepOffset_Type DefineConnectType(const TopoDS_Edge& E, - const TopoDS_Face& F1, - const TopoDS_Face& F2, - const Standard_Real SinTol, - const Standard_Boolean CorrectPoint) +BRepOffset_Type DefineConnectType(const TopoDS_Edge& E, + const TopoDS_Face& F1, + const TopoDS_Face& F2, + const Standard_Real SinTol, + const Standard_Boolean CorrectPoint) { TopLoc_Location L; Standard_Real f,l; - BRepAdaptor_Surface S1(F1), S2(F2); + const Handle(Geom_Surface)& S1 = BRep_Tool::Surface(F1); + const Handle(Geom_Surface)& S2 = BRep_Tool::Surface(F2); + // Handle (Geom2d_Curve) C1 = BRep_Tool::CurveOnSurface(E,F1,f,l); Handle (Geom2d_Curve) C2 = BRep_Tool::CurveOnSurface(E,F2,f,l); @@ -559,16 +565,16 @@ BRepOffset_Type DefineConnectType(const TopoDS_Edge& E, gp_Vec D1U,D1V; if(CorrectPoint) - Correct2dPoint(S1, P); + Correct2dPoint(F1, P); // - S1.D1(P.X(),P.Y(),P3,D1U,D1V); + S1->D1(P.X(),P.Y(),P3,D1U,D1V); gp_Vec DN1(D1U^D1V); if (F1.Orientation() == TopAbs_REVERSED) DN1.Reverse(); P = C2->Value(ParOnC); if(CorrectPoint) - Correct2dPoint(S2, P); - S2.D1(P.X(),P.Y(),P3,D1U,D1V); + Correct2dPoint(F2, P); + S2->D1(P.X(),P.Y(),P3,D1U,D1V); gp_Vec DN2(D1U^D1V); if (F2.Orientation() == TopAbs_REVERSED) DN2.Reverse(); diff --git a/src/BRepOffset/BRepOffset_MakeOffset_1.cxx b/src/BRepOffset/BRepOffset_MakeOffset_1.cxx index 0a4b37d7de..ba00c3f72e 100644 --- a/src/BRepOffset/BRepOffset_MakeOffset_1.cxx +++ b/src/BRepOffset/BRepOffset_MakeOffset_1.cxx @@ -49,6 +49,7 @@ #include #include #include +#include #include #include @@ -57,6 +58,7 @@ #include #include #include +#include #include #include @@ -83,10 +85,14 @@ static TopoDS_Shape& theEdges, TopTools_IndexedMapOfShape& theInv); +static + void BuildSplitsOfTrimmedFace(const TopoDS_Face& theFace, + const TopoDS_Shape& theEdges, + TopTools_ListOfShape& theLFImages); + static void BuildSplitsOfFace(const TopoDS_Face& theFace, const TopoDS_Shape& theEdges, - Standard_Boolean bTrimmed, TopTools_DataMapOfShapeShape& theOrigins, TopTools_ListOfShape& theLFImages); @@ -400,6 +406,7 @@ static const TopoDS_Shape& theSolids, const TopTools_IndexedMapOfShape& theInvEdges, const TopTools_MapOfShape& theInvertedEdges, + const TopTools_MapOfShape& theMEInvOnArt, TopTools_MapOfShape& theMECheckExt, TopTools_IndexedMapOfShape& theEdgesToAvoid, TopTools_DataMapOfShapeListOfShape& theEdgesOrigins, @@ -466,6 +473,7 @@ static const TopTools_DataMapOfShapeListOfShape& theMELF, const TopTools_DataMapOfShapeListOfShape& theEImages, const TopTools_MapOfShape& theMECheckExt, + const TopTools_MapOfShape& theMEInvOnArt, TopTools_MapOfShape& theVertsToAvoid, TopTools_MapOfShape& theMEInv); @@ -564,7 +572,7 @@ void BRepOffset_MakeOffset::BuildSplitsOfTrimmedFaces(const TopTools_ListOfShape continue; } // - BuildSplitsOfFace(aF, aCE, Standard_True, anEmptyDMSS, aLFImages); + BuildSplitsOfTrimmedFace(aF, aCE, aLFImages); aDMFFIm.Add(aF, aLFImages); } // Fill history for faces and edges @@ -764,7 +772,7 @@ void BuildSplitsOfFaces(const TopTools_ListOfShape& theLF, // // build splits TopTools_ListOfShape aLFImages; - BuildSplitsOfFace(aF, aCE, Standard_False, theFacesOrigins, aLFImages); + BuildSplitsOfFace(aF, aCE, theFacesOrigins, aLFImages); // if (aMapEInv.Extent()) { // check if all possible faces are built @@ -781,7 +789,7 @@ void BuildSplitsOfFaces(const TopTools_ListOfShape& theLF, } // TopTools_ListOfShape aLFImages1; - BuildSplitsOfFace(aF, aCE1, Standard_False, theFacesOrigins, aLFImages1); + BuildSplitsOfFace(aF, aCE1, theFacesOrigins, aLFImages1); // // check if the rebuilding has added some new faces to the splits for (TopTools_ListIteratorOfListOfShape aItLFIm(aLFImages1); aItLFIm.More();) @@ -1229,9 +1237,53 @@ Standard_Boolean GetEdges(const TopoDS_Face& theFace, //======================================================================= void BuildSplitsOfFace(const TopoDS_Face& theFace, const TopoDS_Shape& theEdges, - Standard_Boolean bTrimmed, TopTools_DataMapOfShapeShape& theFacesOrigins, TopTools_ListOfShape& theLFImages) +{ + theLFImages.Clear(); + // + // take edges to split the face + BOPCol_ListOfShape aLE; + TopExp_Explorer aExp(theEdges, TopAbs_EDGE); + for (; aExp.More(); aExp.Next()) { + TopoDS_Edge aE = TopoDS::Edge(aExp.Current()); + aE.Orientation(TopAbs_FORWARD); + aLE.Append(aE); + aE.Orientation(TopAbs_REVERSED); + aLE.Append(aE); + } + // + TopoDS_Face aFF = theFace; + TopAbs_Orientation anOr = theFace.Orientation(); + aFF.Orientation(TopAbs_FORWARD); + // + // build pcurves for edges on the face + BOPTools_AlgoTools2D::BuildPCurveForEdgesOnPlane(aLE, aFF); + // + // build splits of faces + BOPAlgo_BuilderFace aBF; + aBF.SetFace(aFF); + aBF.SetShapes(aLE); + aBF.Perform(); + // + const BOPCol_ListOfShape& aLFSp = aBF.Areas(); + BOPCol_ListIteratorOfListOfShape aItLF(aLFSp); + for (; aItLF.More(); aItLF.Next()) { + TopoDS_Shape& aFSp = aItLF.ChangeValue(); + aFSp.Orientation(anOr); + theLFImages.Append(aFSp); + // + theFacesOrigins.Bind(aFSp, theFace); + } +} + +//======================================================================= +//function : BuildSplitsOfFace +//purpose : Building the splits of offset face +//======================================================================= +void BuildSplitsOfTrimmedFace(const TopoDS_Face& theFace, + const TopoDS_Shape& theEdges, + TopTools_ListOfShape& theLFImages) { BOPAlgo_Builder aGF; // @@ -1244,44 +1296,6 @@ void BuildSplitsOfFace(const TopoDS_Face& theFace, // // splits of the offset shape theLFImages = aGF.Modified(theFace); - if (theLFImages.IsEmpty()) { - return; - } - // - if (!bTrimmed) { - // get the real splits of faces by removing the faces - // containing boundaries of the extended faces - - // get edges to map - TopTools_IndexedMapOfShape aME; - TopExp::MapShapes(theEdges, TopAbs_EDGE, aME); - // - TopTools_ListIteratorOfListOfShape aItLE(theLFImages); - for (; aItLE.More();) { - const TopoDS_Face& aFIm = *(TopoDS_Face*)&aItLE.Value(); - // - TopExp_Explorer aExp(aFIm, TopAbs_EDGE); - for (; aExp.More(); aExp.Next()) { - const TopoDS_Edge& aE = *(TopoDS_Edge*)&aExp.Current(); - if (!aME.Contains(aE)) { - break; - } - } - // - if (!aExp.More()) { - aItLE.Next(); - } - else { - theLFImages.Remove(aItLE); - } - } - } - // update origins - TopTools_ListIteratorOfListOfShape aIt(theLFImages); - for (; aIt.More(); aIt.Next()) { - const TopoDS_Shape& aFIm = aIt.Value(); - theFacesOrigins.Bind(aFIm, theFace); - } } //======================================================================= @@ -1786,6 +1800,8 @@ void FindInvalidFaces(TopTools_ListOfShape& theLFImages, !(bHasValid || bAllValid) && !(bAllInvNeutral && (aNbChecked == 1)))) { theInvFaces.Append(aFIm); + aItLF.Next(); + continue; } // if (!bAllInvNeutral) { @@ -3344,7 +3360,7 @@ void IntersectFaces(const TopTools_IndexedDataMapOfShapeListOfShape& theFToRebui BOPTools_AlgoTools::MakeConnexityBlocks(aCFArt, TopAbs_VERTEX, TopAbs_FACE, aLCBArt); // // alone edges - TopTools_MapOfShape aMEAlone; + TopTools_MapOfShape aMEAlone, aMEInvOnArt; // TopTools_ListIteratorOfListOfShape aItLCBArt(aLCBArt); for (; aItLCBArt.More(); aItLCBArt.Next()) { @@ -3371,6 +3387,7 @@ void IntersectFaces(const TopTools_IndexedDataMapOfShapeListOfShape& theFToRebui for (; aExpE.More(); aExpE.Next()) { const TopoDS_Shape& aE = aExpE.Current(); if (theInvEdges.Contains(aE)) { + aMEInvOnArt.Add(aE); for (TopoDS_Iterator aItV(aE); aItV.More(); aItV.Next()) { aMVEInv.Add(aItV.Value()); } @@ -3650,7 +3667,7 @@ void IntersectFaces(const TopTools_IndexedDataMapOfShapeListOfShape& theFToRebui } // // filter the obtained edges - UpdateValidEdges(theFImages, aFLE, aMVBounds, theSolids, theInvEdges, theInvertedEdges, + UpdateValidEdges(theFImages, aFLE, aMVBounds, theSolids, theInvEdges, theInvertedEdges, aMEInvOnArt, aMECheckExt, theEdgesToAvoid, theEdgesOrigins, theOEImages, theOEOrigins, theVertsToAvoid, theETrimEInf, aEImages, aDMEETrim, theModifiedEdges, theAsDes); } @@ -4556,6 +4573,7 @@ void UpdateValidEdges(const TopTools_IndexedDataMapOfShapeListOfShape& theFImage const TopoDS_Shape& theSolids, const TopTools_IndexedMapOfShape& theInvEdges, const TopTools_MapOfShape& theInvertedEdges, + const TopTools_MapOfShape& theMEInvOnArt, TopTools_MapOfShape& theMECheckExt, TopTools_IndexedMapOfShape& theEdgesToAvoid, TopTools_DataMapOfShapeListOfShape& theEdgesOrigins, @@ -4643,7 +4661,7 @@ void UpdateValidEdges(const TopTools_IndexedDataMapOfShapeListOfShape& theFImage TopTools_MapOfShape aMEInv; GetInvalidEdgesByBounds(aSplits1, aFilterBounds, theFImages, theSolids, theInvEdges, aMVOld, aMENew, aDMEOr, aMELF, theEImages, - theMECheckExt, theVertsToAvoid, aMEInv); + theMECheckExt, theMEInvOnArt, theVertsToAvoid, aMEInv); // // get valid edges only TopoDS_Shape aSplits; @@ -4825,7 +4843,7 @@ void TrimNewIntersectionEdges(const TopTools_ListOfShape& theLE, TopoDS_Shape aCEIm; TopTools_MapOfShape aMEVBounds; // - if (aLEIm.Extent() > 2) { + if (aLEIm.Extent() > 1) { TopTools_IndexedMapOfShape aMV; // fuse these parts BOPAlgo_Builder aGFE; @@ -5147,9 +5165,14 @@ void GetInvalidEdgesByBounds(const TopoDS_Shape& theSplits, const TopTools_DataMapOfShapeListOfShape& theMELF, const TopTools_DataMapOfShapeListOfShape& theEImages, const TopTools_MapOfShape& theMECheckExt, + const TopTools_MapOfShape& theMEInvOnArt, TopTools_MapOfShape& theVertsToAvoid, TopTools_MapOfShape& theMEInv) { + // map splits to check the vertices of edges + TopTools_IndexedDataMapOfShapeListOfShape aDMVE; + TopExp::MapShapesAndAncestors(theSplits, TopAbs_VERTEX, TopAbs_EDGE, aDMVE); + // BOPAlgo_Section aSec; aSec.AddArgument(theSplits); aSec.AddArgument(theBounds); @@ -5183,6 +5206,16 @@ void GetInvalidEdgesByBounds(const TopoDS_Shape& theSplits, theMEInv.Add(aE1); } // + if (theMEInvOnArt.Contains(aE2)) { + // avoid checking of the vertices of the split edge intersected by + // the invalid edge from artificial face + TopoDS_Vertex aV1, aV2; + TopExp::Vertices(TopoDS::Edge(aE2), aV1, aV2); + if (aDMVE.Contains(aV1) && aDMVE.Contains(aV2)) { + continue; + } + } + // // add vertices of all images of the edge from splits for checking const TopTools_ListOfShape& aLEOr = theDMEOr.Find(aE1); TopTools_ListIteratorOfListOfShape aItLEOr(aLEOr); @@ -5206,10 +5239,6 @@ void GetInvalidEdgesByBounds(const TopoDS_Shape& theSplits, } } // - // map to check the vertices of edges - TopTools_IndexedDataMapOfShapeListOfShape aDMVE; - TopExp::MapShapesAndAncestors(theSplits, TopAbs_VERTEX, TopAbs_EDGE, aDMVE); - // // to avoid unnecessary filling of parts due to extra trim of the edges // process Edge/Edge interferences of type EDGE, i.e. common blocks and check // not the bounding vertices of the edges, but check the edge itself diff --git a/tests/bugs/modalg_6/bug26513 b/tests/bugs/modalg_6/bug26513 index d60a5996d0..c2f2bfbf76 100644 --- a/tests/bugs/modalg_6/bug26513 +++ b/tests/bugs/modalg_6/bug26513 @@ -12,21 +12,16 @@ smallview restore [locate_data_file bug26513-offset_input.brep] a +unifysamedom s a -a 1.e-8 offsetparameter 1.e-7 c i -offsetload a 5 - -dchrono cr reset -dchrono cr start +offsetload s 5 offsetperform result -dchrono cr stop -set chrono_info [dchrono cr show] -regexp {CPU user time: ([-0-9.+eE]+) seconds} $chrono_info full CPU_time -if { $CPU_time > ${max_time} } { - puts "ERROR: OCC26513 is reproduced. Offset operation is too long (> $max_time sec)" -} +checkshape result -donly result -fit +checkprops result -s 3.28125e+008 -v 3.07334e+010 -checkview -screenshot -2d -path ${imagedir}/${test_image}.png +unifysamedom result_unif result +checknbshapes result_unif -face 114 -shell 1 + +checkview -display result_unif -2d -path ${imagedir}/${test_image}.png diff --git a/tests/offset/shape_type_i_c/XD3 b/tests/offset/shape_type_i_c/XD3 index 05655de975..910ba6654c 100755 --- a/tests/offset/shape_type_i_c/XD3 +++ b/tests/offset/shape_type_i_c/XD3 @@ -1,14 +1,15 @@ -puts "TODO OCC27414 ALL: Error: The command cannot be built" -puts "TODO OCC27414 ALL: Tcl Exception" -puts "TODO OCC27414 ALL: TEST INCOMPLETE" +puts "TODO CR27414 ALL: Error : The area of result shape is" +puts "TODO CR27414 ALL: Error : The volume of result shape is" +puts "TODO CR27414 ALL: Error : is WRONG because number of FACE entities in shape" +puts "TODO CR27414 ALL: Error: bsection of the result and s is not equal to zero." restore [locate_data_file bug26917_25926_offset_shape_trim4.brep] s OFFSETSHAPE 12 {} $calcul $type -checkprops result -v 0 -s 0 +checkprops result -v 643336 -s 45602.9 unifysamedom result_unif result -checknbshapes result_unif -shell 1 -face 1 +checknbshapes result_unif -shell 1 -face 8 checkview -display result_unif -2d -path ${imagedir}/${test_image}.png diff --git a/tests/offset/shape_type_i_c/ZD9 b/tests/offset/shape_type_i_c/ZD9 index c03747378c..22b45bf1ad 100755 --- a/tests/offset/shape_type_i_c/ZD9 +++ b/tests/offset/shape_type_i_c/ZD9 @@ -1,5 +1,6 @@ -puts "TODO CR27414 ALL: Error : The area of result shape is" -puts "TODO CR27414 ALL: Error : The volume of result shape is" +puts "TODO OCC27414 ALL: Error: The command cannot be built" +puts "TODO OCC27414 ALL: gives an empty result" +puts "TODO OCC27414 ALL: TEST INCOMPLETE" restore [locate_data_file bug26917_M2_trim9.brep] s