0028273: Inefficient implementation of the BRepOffset_MakeOffset::BuildSplitsOfFace...

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.

diff --git a/src/BRepOffset/BRepOffset_Analyse.cxx b/src/BRepOffset/BRepOffset_Analyse.cxx
index 825abd54f3512281ec639273e7548f50eb9b9150..ca2281ae086595b5dbc6f087e1f5e34f83fd1be2 100644 (file)
--- a/src/BRepOffset/BRepOffset_Analyse.cxx
+++ b/src/BRepOffset/BRepOffset_Analyse.cxx
@@ -46,7 +46,7 @@
 #include <TopTools_MapOfShape.hxx>
 
 //
-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 0a4b37d7de414540e111c0b5e3e67dbc9d888fec..ba00c3f72e4374b7fbd020d4d43cd1ca57e908b3 100644 (file)
--- a/src/BRepOffset/BRepOffset_MakeOffset_1.cxx
+++ b/src/BRepOffset/BRepOffset_MakeOffset_1.cxx
@@ -49,6 +49,7 @@
 #include <BOPAlgo_Builder.hxx>
 #include <BOPAlgo_Section.hxx>
 #include <BOPAlgo_MakerVolume.hxx>
+#include <BOPAlgo_BuilderFace.hxx>
 
 #include <BOPCol_ListOfShape.hxx>
 #include <BOPCol_DataMapOfShapeShape.hxx>
@@ -57,6 +58,7 @@
 #include <BOPTools.hxx>
 #include <BOPTools_AlgoTools3D.hxx>
 #include <BOPTools_AlgoTools.hxx>
+#include <BOPTools_AlgoTools2D.hxx>
 
 #include <IntTools_Context.hxx>
 #include <IntTools_ShrunkRange.hxx>
@@ -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 d60a5996d0333bc493e4698d9a013b8b3b3069d8..c2f2bfbf764417c552e72e291c1f4176cde81ee0 100644 (file)
--- 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
+
+checkprops result -s 3.28125e+008 -v 3.07334e+010
 
-donly result
-fit
+unifysamedom result_unif result
+checknbshapes result_unif -face 114 -shell 1
 
-checkview -screenshot -2d -path ${imagedir}/${test_image}.png
+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 05655de9753249ea9b851184f5621415273bc9e9..910ba6654c9da4218bd11dcb73c5eabea0ccb87f 100755 (executable)
--- 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 c03747378cf3c7b79fb79361e9e48602c4809e9e..22b45bf1ad506221dd1f7945b96114a14517f371 100755 (executable)
--- 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