From 56c62737ee9621c4e95a9cdac280d6a222885bf0 Mon Sep 17 00:00:00 2001 From: msv Date: Fri, 6 Oct 2017 17:07:50 +0300 Subject: [PATCH] 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. --- src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx | 126 +++++++++++++++++--------- src/BOPTools/BOPTools_AlgoTools.hxx | 4 +- src/BOPTools/BOPTools_AlgoTools_2.cxx | 18 +++- tests/bugs/modalg_7/bug28200 | 2 +- tests/bugs/modalg_7/bug29182 | 24 +++++ tests/bugs/modalg_7/bug29182_1 | 41 +++++++++ 6 files changed, 168 insertions(+), 47 deletions(-) create mode 100644 tests/bugs/modalg_7/bug29182 create mode 100644 tests/bugs/modalg_7/bug29182_1 diff --git a/src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx b/src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx index 1e91f5529a..20175dfd1c 100644 --- a/src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx +++ b/src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -246,28 +247,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); + // 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 (iErr) { - BOPTools_AlgoTools2D::BuildPCurveForEdgeOnFace(myE, myF, myContext); - } - // + if (myFlag) { - UpdateVertices(myE, myF); + UpdateVertices(aCopyE, myF); } } catch (Standard_Failure) @@ -275,7 +290,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; @@ -285,6 +310,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; }; @@ -541,7 +568,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; @@ -637,44 +664,52 @@ void BOPAlgo_PaveFiller::MakePCurves() } }// for (i=0; iInterfFF(); aNbFF=aFFs.Extent(); for (i=0; iShape(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; kEdge(); 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); } } @@ -687,18 +722,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 dddfb07ae2..4adb7ada67 100644 --- a/src/BOPTools/BOPTools_AlgoTools.hxx +++ b/src/BOPTools/BOPTools_AlgoTools.hxx @@ -248,7 +248,9 @@ public: //! Compute a 3D-point on the edge at parameter Standard_EXPORT static void PointOnEdge (const TopoDS_Edge& aEdge, const Standard_Real aPrm, gp_Pnt& aP); - + + //! Makes a copy of with vertices. + Standard_EXPORT static TopoDS_Edge CopyEdge(const TopoDS_Edge& theEdge); //! Make the edge from base edge and two vertices //! at parameters diff --git a/src/BOPTools/BOPTools_AlgoTools_2.cxx b/src/BOPTools/BOPTools_AlgoTools_2.cxx index d78965c85e..90baa8a9f6 100644 --- 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 6a043ebf0b..4d517fb3d4 100644 --- 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 index 0000000000..15b30d7c1e --- /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 index 0000000000..e9433f6749 --- /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" +}