0029182: BOPAlgo_PaveFiller sometimes raises exception in parallel mode

Data races have been prevented in the code of BOPAlgo_PaveFiller that makes pcurves of edges on faces. For that:
- Put into treatment only unique edge-face pairs.
- If the same edge is treated with different faces in different threads simultaneously this also causes data races. To avoid this make the edge's copy in each thread and update the copy. The original edge is updated only after finishing parallel processing.

The new method BOPTools_AlgoTools::CopyEdge has been added to make a copy of an edge with vertices.

Big screenshot in the test script tests/bugs/modalg_7/bug28200 has been replaced with a small one.

diff --git a/src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx b/src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx
index 1ae2ca3417e52bff407df40b6a04b330fc78bc9c..ab5020513f973ad6266e8291701df1ae410aa03b 100644 (file)
--- a/src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx
+++ b/src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx
@@ -29,6 +29,7 @@
 #include <BOPDS_Interf.hxx>
 #include <BOPDS_Iterator.hxx>
 #include <BOPDS_ListOfPaveBlock.hxx>
+#include <BOPDS_MapOfPair.hxx>
 #include <BOPDS_MapOfPaveBlock.hxx>
 #include <BOPDS_Pave.hxx>
 #include <BOPDS_PaveBlock.hxx>
@@ -247,28 +248,42 @@ class BOPAlgo_MPC : public BOPAlgo_Algo  {
     {
       OCC_CATCH_SIGNALS
 
-      Standard_Integer iErr;
-      //
-      iErr=1;
-      if (!myEz.IsNull()) {
-        TopoDS_Edge aSpz;
-        //
-        BOPTools_AlgoTools::MakeSplitEdge(myEz,myV1, myT1, 
-                                          myV2, myT2, aSpz);
-        //
-        iErr=
-          BOPTools_AlgoTools2D::AttachExistingPCurve(aSpz, 
-                                                     myE, 
-                                                     myF, 
-                                                     myContext);
-      }
-      //
-      if (iErr) { 
-        BOPTools_AlgoTools2D::BuildPCurveForEdgeOnFace(myE, myF, myContext);
+      // Check if edge has pcurve. If no then make its copy to avoid data races,
+      // and use it to build pcurve.
+      TopoDS_Edge aCopyE = myE;
+      Standard_Real f, l;
+      Handle(Geom2d_Curve) aC2d = BRep_Tool::CurveOnSurface(aCopyE, myF, f, l);
+      if (aC2d.IsNull())
+      {
+        aCopyE = BOPTools_AlgoTools::CopyEdge(aCopyE);
+
+        Standard_Integer iErr = 1;
+        if (!myEz.IsNull())
+        {
+          // Attach pcurve from the original edge
+          TopoDS_Edge aSpz;
+          BOPTools_AlgoTools::MakeSplitEdge(myEz, myV1, myT1,
+                                            myV2, myT2, aSpz);
+          iErr = BOPTools_AlgoTools2D::AttachExistingPCurve(aSpz,
+                                                            aCopyE, 
+                                                            myF,
+                                                            myContext);
+        }
+        if (iErr)
+          BOPTools_AlgoTools2D::BuildPCurveForEdgeOnFace(aCopyE, myF, myContext);
+
+        myNewC2d = BRep_Tool::CurveOnSurface(aCopyE, myF, f, l);
+        if (myNewC2d.IsNull())
+        {
+          AddError(new BOPAlgo_AlertBuildingPCurveFailed(TopoDS_Shape()));
+          return;
+        }
+        else
+          myNewTol = BRep_Tool::Tolerance(aCopyE);
       }
-      // 
+
       if (myFlag) {
-        UpdateVertices(myE, myF);
+        UpdateVertices(aCopyE, myF);
       }
     }
     catch (Standard_Failure)
@@ -276,7 +291,17 @@ class BOPAlgo_MPC : public BOPAlgo_Algo  {
       AddError(new BOPAlgo_AlertBuildingPCurveFailed(TopoDS_Shape()));
     }
   }
-  //
+
+  const Handle(Geom2d_Curve)& GetNewPCurve() const
+  {
+    return myNewC2d;
+  }
+
+  Standard_Real GetNewTolerance() const
+  {
+    return myNewTol;
+  }
+
  protected:
   Standard_Boolean myFlag;
   TopoDS_Edge myE;
@@ -286,6 +311,8 @@ class BOPAlgo_MPC : public BOPAlgo_Algo  {
   Standard_Real myT1;
   TopoDS_Vertex myV2;
   Standard_Real myT2;
+  Handle(Geom2d_Curve) myNewC2d;
+  Standard_Real myNewTol;
   //
   Handle(IntTools_Context) myContext;
 };
@@ -542,7 +569,7 @@ void BOPAlgo_PaveFiller::MakePCurves()
       (!mySectionAttribute.PCurveOnS1() && !mySectionAttribute.PCurveOnS2()))
     return;
   Standard_Boolean bHasPC;
-  Standard_Integer i, nF1, nF2, aNbC, k, nE, aNbFF, aNbFI, nEx;
+  Standard_Integer i, nF1, aNbC, k, nE, aNbFF, aNbFI, nEx;
   Standard_Integer j, aNbPBIn, aNbPBOn;
   BOPDS_ListIteratorOfListOfPaveBlock aItLPB;
   TopoDS_Face aF1F, aF2F;
@@ -638,44 +665,52 @@ void BOPAlgo_PaveFiller::MakePCurves()
     }
   }// for (i=0; i<aNbFI; ++i) {
   //
-  // 2. Process section edges
+  // 2. Process section edges. P-curves on them must already be computed.
+  // However, we must provide the call to UpdateVertices.
   Standard_Boolean bPCurveOnS[2];
-  Standard_Integer m;
-  TopoDS_Face aFf[2];
-  //
   bPCurveOnS[0]=mySectionAttribute.PCurveOnS1();
   bPCurveOnS[1]=mySectionAttribute.PCurveOnS2();
   //
   if (bPCurveOnS[0] || bPCurveOnS[1]) {
+    // container to remember already added edge-face pairs
+    BOPDS_MapOfPair anEFPairs;
     BOPDS_VectorOfInterfFF& aFFs=myDS->InterfFF();
     aNbFF=aFFs.Extent();
     for (i=0; i<aNbFF; ++i) {
       const BOPDS_InterfFF& aFF=aFFs(i);
-      aFF.Indices(nF1, nF2);
+      const BOPDS_VectorOfCurve& aVNC = aFF.Curves();
+      aNbC = aVNC.Extent();
+      if (aNbC == 0)
+        continue;
+      Standard_Integer nF[2];
+      aFF.Indices(nF[0], nF[1]);
       //
-      aFf[0]=(*(TopoDS_Face *)(&myDS->Shape(nF1)));
+      TopoDS_Face aFf[2];
+      aFf[0] = (*(TopoDS_Face *)(&myDS->Shape(nF[0])));
       aFf[0].Orientation(TopAbs_FORWARD);
       //
-      aFf[1]=(*(TopoDS_Face *)(&myDS->Shape(nF2)));
+      aFf[1]=(*(TopoDS_Face *)(&myDS->Shape(nF[1])));
       aFf[1].Orientation(TopAbs_FORWARD);
       //
-      const BOPDS_VectorOfCurve& aVNC=aFF.Curves();
-      aNbC=aVNC.Extent();
-      for (k=0; k<aNbC; ++k) {
+      for (k=0; k<aNbC; ++k)
+      {
         const BOPDS_Curve& aNC=aVNC(k);
         const BOPDS_ListOfPaveBlock& aLPB=aNC.PaveBlocks();
         aItLPB.Initialize(aLPB);
-        for(; aItLPB.More(); aItLPB.Next()) {
+        for(; aItLPB.More(); aItLPB.Next())
+        {
           const Handle(BOPDS_PaveBlock)& aPB=aItLPB.Value();
           nE=aPB->Edge();
           const TopoDS_Edge& aE=(*(TopoDS_Edge *)(&myDS->Shape(nE))); 
           //
-          for (m=0; m<2; ++m) {
-            if (bPCurveOnS[m]) {
-              BOPAlgo_MPC& aMPC=aVMPC.Append1();
+          for (Standard_Integer m = 0; m<2; ++m)
+          {
+            if (bPCurveOnS[m] && anEFPairs.Add(BOPDS_Pair(nE, nF[m])))
+            {
+              BOPAlgo_MPC& aMPC = aVMPC.Append1();
               aMPC.SetEdge(aE);
               aMPC.SetFace(aFf[m]);
-              aMPC.SetFlag(bPCurveOnS[m]);
+              aMPC.SetFlag(Standard_True);
               aMPC.SetProgressIndicator(myProgressIndicator);
             }
           }
@@ -688,18 +723,27 @@ void BOPAlgo_PaveFiller::MakePCurves()
   BOPAlgo_MPCCnt::Perform(myRunParallel, aVMPC, myContext);
   //======================================================
 
-  // Add warnings of the failed projections
+  // Add warnings of the failed projections and update edges with new pcurves
   Standard_Integer aNb = aVMPC.Extent();
   for (i = 0; i < aNb; ++i)
   {
-    if (aVMPC(i).HasErrors())
+    const BOPAlgo_MPC& aMPC = aVMPC(i);
+    if (aMPC.HasErrors())
     {
       TopoDS_Compound aWC;
       BRep_Builder().MakeCompound(aWC);
-      BRep_Builder().Add(aWC, aVMPC(i).Edge());
-      BRep_Builder().Add(aWC, aVMPC(i).Face());
+      BRep_Builder().Add(aWC, aMPC.Edge());
+      BRep_Builder().Add(aWC, aMPC.Face());
       AddWarning(new BOPAlgo_AlertBuildingPCurveFailed(aWC));
     }
+    else
+    {
+      const Handle(Geom2d_Curve)& aNewPC = aMPC.GetNewPCurve();
+      // if aNewPC is null we do not need to update the edge because it already contains
+      // valid p-curve, and only vertices have been updated.
+      if (!aNewPC.IsNull())
+        BRep_Builder().UpdateEdge(aMPC.Edge(), aNewPC, aMPC.Face(), aMPC.GetNewTolerance());
+    }
   }
 }
 //=======================================================================

diff --git a/src/BOPTools/BOPTools_AlgoTools.hxx b/src/BOPTools/BOPTools_AlgoTools.hxx
index 84a75e7eeb19f97a0da79e7a8578d352e32a0563..0a430af2b2c55c30200e39e195f4d025a49ddba1 100644 (file)
--- a/src/BOPTools/BOPTools_AlgoTools.hxx
+++ b/src/BOPTools/BOPTools_AlgoTools.hxx
@@ -273,7 +273,9 @@ public:
 
   //! Compute a 3D-point on the edge <aEdge> at parameter <aPrm>
   Standard_EXPORT static void PointOnEdge (const TopoDS_Edge& aEdge, const Standard_Real aPrm, gp_Pnt& aP);
-  
+
+  //! Makes a copy of <theEdge> with vertices.
+  Standard_EXPORT static TopoDS_Edge CopyEdge(const TopoDS_Edge& theEdge);
 
   //! Make the edge from base edge <aE1> and two vertices <aV1,aV2>
   //! at parameters <aP1,aP2>

diff --git a/src/BOPTools/BOPTools_AlgoTools_2.cxx b/src/BOPTools/BOPTools_AlgoTools_2.cxx
index d78965c85e08eab9cdbd371e46dc1117fb148784..90baa8a9f60d0bf466bdb81fb2274ad1c9c3a556 100644 (file)
--- a/src/BOPTools/BOPTools_AlgoTools_2.cxx
+++ b/src/BOPTools/BOPTools_AlgoTools_2.cxx
@@ -132,6 +132,20 @@ void BOPTools_AlgoTools::MakeSectEdge(const IntTools_Curve& aIC,
   
 }
 
+//=======================================================================
+// function: CopyEdge
+// purpose: 
+//=======================================================================
+TopoDS_Edge BOPTools_AlgoTools::CopyEdge(const TopoDS_Edge& theEdge)
+{
+  TopoDS_Edge aNewEdge = TopoDS::Edge(theEdge.Oriented(TopAbs_FORWARD));
+  aNewEdge.EmptyCopy();
+  for (TopoDS_Iterator it(theEdge, Standard_False); it.More(); it.Next())
+    BRep_Builder().Add(aNewEdge, it.Value());
+  aNewEdge.Orientation(theEdge.Orientation());
+  return aNewEdge;
+}
+
 //=======================================================================
 // function: MakeSplitEdge
 // purpose: 
@@ -143,9 +157,6 @@ void BOPTools_AlgoTools::MakeSplitEdge(const TopoDS_Edge&   aE,
                                        const Standard_Real  aP2,
                                        TopoDS_Edge& aNewEdge)
 {
-  Standard_Real aTol;//f, l, 
-  aTol=BRep_Tool::Tolerance(aE);
-  //
   TopoDS_Edge E = TopoDS::Edge(aE.Oriented(TopAbs_FORWARD));
   E.EmptyCopy();
   //
@@ -157,7 +168,6 @@ void BOPTools_AlgoTools::MakeSplitEdge(const TopoDS_Edge&   aE,
     BB.Add  (E, aV2);
   }
   BB.Range(E, aP1, aP2);
-  BB.UpdateEdge(E, aTol);
   aNewEdge=E;
   aNewEdge.Orientation(aE.Orientation());
 }

diff --git a/tests/bugs/modalg_7/bug28200 b/tests/bugs/modalg_7/bug28200
index 6a043ebf0b749c1d2a7b6d78f3a47f818e2a96d1..4d517fb3d498b6aa80dd0804ba4fd032b3bf35c7 100644 (file)
--- a/tests/bugs/modalg_7/bug28200
+++ b/tests/bugs/modalg_7/bug28200
@@ -33,7 +33,7 @@ donly result
 fit
 checkview -screenshot -2d -path ${imagedir}/${test_image}_1.png
 
-left
+smallview -Y+Z
 fit
 checkview -screenshot -2d -path ${imagedir}/${test_image}_2.png
 

diff --git a/tests/bugs/modalg_7/bug29182 b/tests/bugs/modalg_7/bug29182
new file mode 100644 (file)
index 0000000..15b30d7
--- /dev/null
+++ b/tests/bugs/modalg_7/bug29182
@@ -0,0 +1,24 @@
+puts "========"
+puts "OCC29182"
+puts "========"
+puts ""
+#################################################
+# BOPAlgo_PaveFiller sometimes raises exception in parallel mode
+#################################################
+
+binrestore [locate_data_file bug29182_access_violation.bin] a
+explode a So
+bclearobjects
+bcleartools
+baddobjects a_1
+baddtools a_2 a_3 a_4 a_5 a_6 a_7 a_8 a_9 a_10 a_11 a_12
+brunparallel 1
+bfillds
+bbuild result
+
+checknbshapes result -m result -shell 17 -solid 17
+checkprops result -v 1.18596e+006 -s 1.28028e+006
+checkshape result
+if ![regexp "This shape seems to be OK" [bopcheck result]] {
+  puts "Error: result is self-intersected"
+}

diff --git a/tests/bugs/modalg_7/bug29182_1 b/tests/bugs/modalg_7/bug29182_1
new file mode 100644 (file)
index 0000000..e9433f6
--- /dev/null
+++ b/tests/bugs/modalg_7/bug29182_1
@@ -0,0 +1,41 @@
+puts "========"
+puts "OCC29182"
+puts "========"
+puts ""
+#################################################
+# BOPAlgo_PaveFiller sometimes raises exception in parallel mode
+#################################################
+
+# This is synthetic case reproducing the bug.
+# Make a set of cylindrical surface patches intersecting in the same
+# 3d line and run bfillds on them repeatedly 100 times.
+
+cylinder c 0 10 0 1 0 0 10
+mkface fc c pi/3 2*pi/3 -10 10
+
+shape a C
+add fc a
+don a
+
+for {set i 1} {$i <= 10} {incr i} {
+  copy fc f1
+  trotate f1 0 0 0 1 0 0 $i*5
+  add f1 a
+}
+
+brunparallel 1
+bclearobjects
+bcleartools
+baddcompound a
+
+for {set i 1} {$i <= 100} {incr i} {
+  puts "pass $i"
+  set info [string trim [bfillds]]
+  if {$info != "" && [regexp "2D curve" $info]} {
+    break
+  }
+}
+
+if {$i <= 100} {
+  puts "Error: the bug with failure building 2D curve of edge on face is reproduced"
+}