0029586: Revolution creates solid with degenerated edges shared among faces

Substitution of degenerated edges by their copies is implemented to make edges not shared
in BRepPrimAPI_MakeRevol::Build().
Method BRepPrimAPI_MakeRevol::Generated(...) is changed in order to use history of substitutions.
Test case is added.

dox/dev_guides/upgrade/upgrade.md

@@ -1578,3 +1578,8 @@ Class *Message_PrinterOStream* can be used instead of *CDM_COutMessageDriver* to
 If custom driver class is used in the application, that class shall be reimplemented inheriting from *Message_Printer* instead of *CDM_MessageDriver*.
 Method *Send()* should be redefined instead of method *Write()* of *CDM_MessageDriver*.
 To use the custom printer in OCAF, it can be either added to default messenger or set into the custom *Message_Messenger* object created in the method *MessageDriver()* of a class inheriting *CDF_Application*.
+
+@section upgrade_occt740 Upgrade to OCCT 7.4.0
+
+@subsection upgrade_740_BRepPrimAPI_MakeRevol Changes in BRepPrimAPI_MakeRevol algorithm
+Previously the algorithm could create a shape with the same degenerated edge shared between some faces. Now it is prevented. The algorithm creates the different copy of this edge for each face. The method *Generated(...)* has been changed in order to apply restriction to the input shape: input shape can be only of type VERTEX, EDGE, FACE or SOLID. For input shape of another type the method always returns empty list.

src/BRepPrimAPI/BRepPrimAPI_MakeRevol.cxx

@@ -24,7 +24,8 @@
 #include <gp_Ax1.hxx>
 #include <TopExp_Explorer.hxx>
 #include <TopoDS_Shape.hxx>
-
+#include <TopTools_DataMapOfShapeListOfShape.hxx>
+#include <BRepTools_ReShape.hxx>
 // perform checks on the argument
 static const TopoDS_Shape& check(const TopoDS_Shape& S)
 {
@@ -83,19 +84,131 @@ void  BRepPrimAPI_MakeRevol::Build()
   BRepLib::UpdateInnerTolerances(myShape);
   
   Done();
-// Modified by skv - Fri Mar  4 15:50:09 2005 Begin
+  myHist.Nullify();
   myDegenerated.Clear();
+  TopTools_DataMapOfShapeListOfShape aDegE;
+  BRep_Builder aBB;
 
   TopExp_Explorer anExp(myShape, TopAbs_EDGE);
-
+  //Problem is that some degenerated edges can be shared by different faces.
+  //It is not valid for correct shape.
+  //To solve problem it is possible to copy shared degenerated edge for each face, which has it, and 
+  //replace shared edge by its copy
   for (; anExp.More(); anExp.Next()) {
     const TopoDS_Shape &anEdge = anExp.Current();
     Handle(BRep_TEdge)  aTEdge = Handle(BRep_TEdge)::DownCast(anEdge.TShape());
 
     if (aTEdge->Degenerated())
+    {
+      TopTools_ListOfShape* anL = aDegE.ChangeSeek(anEdge);
+      if (anL)
+      {
+        //Make the copy if degenerated edge occurs more then once
+        TopoDS_Shape aCopyE = anEdge.EmptyCopied();
+        aCopyE.Orientation(TopAbs_FORWARD);
+        TopoDS_Iterator aVIter(anEdge.Oriented(TopAbs_FORWARD), Standard_False);
+        for (; aVIter.More(); aVIter.Next())
+        {
+          aBB.Add(aCopyE, aVIter.Value());
+        }
+        aCopyE.Orientation(anEdge.Orientation());
+        anL->Append(aCopyE);
+        myDegenerated.Append(aCopyE);
+      }
+      else
+      {
+        anL = aDegE.Bound(anEdge, TopTools_ListOfShape());
+        anL->Append(anEdge);
         myDegenerated.Append(anEdge);
       }
-// Modified by skv - Fri Mar  4 15:50:09 2005 End
+    }
+  }
+  if (!myDegenerated.IsEmpty())
+  {
+    BRepTools_ReShape aSubs;
+    TopTools_DataMapOfShapeListOfShape aDegF;
+    Standard_Boolean isReplaced = Standard_False;
+    anExp.Init(myShape, TopAbs_FACE);
+    //Replace degenerated edge by its copies for different faces
+    //First, for each face list of d.e. is created
+    for (; anExp.More(); anExp.Next())
+    {
+      const TopoDS_Shape& aF = anExp.Current();
+      TopExp_Explorer anExpE(aF, TopAbs_EDGE);
+      for (; anExpE.More(); anExpE.Next())
+      {
+        const TopoDS_Shape &anE = anExpE.Current();
+        if (BRep_Tool::Degenerated(TopoDS::Edge(anE)))
+        {
+          TopTools_ListOfShape* anL = aDegF.ChangeSeek(aF);
+          if (!anL)
+          {
+            anL = aDegF.Bound(aF, TopTools_ListOfShape());
+          }
+          anL->Append(anE);
+        }
+      }
+    }
+    //
+    //Second, replace edges by copies using ReShape
+    BRepTools_ReShape aSubsF;
+    TopTools_DataMapIteratorOfDataMapOfShapeListOfShape aFIter(aDegF);
+    for (; aFIter.More(); aFIter.Next())
+    {
+      aSubs.Clear();
+      isReplaced = Standard_False;
+      const TopoDS_Shape& aF = aFIter.Key();
+      const TopTools_ListOfShape& aDEL = aFIter.ChangeValue();
+      TopTools_ListIteratorOfListOfShape anEIter(aDEL);
+      for (; anEIter.More(); anEIter.Next())
+      {
+        const TopoDS_Shape& anE = anEIter.Value();
+        if (aDegE.IsBound(anE))
+        {
+          TopTools_ListOfShape& aCEL = aDegE.ChangeFind(anE);
+          TopTools_ListIteratorOfListOfShape anIt(aCEL);
+          for (; anIt.More(); anIt.Next())
+          {
+            if (anIt.Value().IsEqual(anE))
+            {
+              //First occurence of initial deg. edge is not replaced
+              aCEL.Remove(anIt);
+              break;
+            }
+            if (anIt.Value().Orientation() == anE.Orientation())
+            {
+              //All other occurences of anE are replaced by any copy
+              //with suitable orientation
+              isReplaced = Standard_True;
+              aSubs.Replace(anE, anIt.Value());
+              aCEL.Remove(anIt);
+              break;
+            }
+          }
+        }
+      }
+      if (isReplaced)
+      {
+        TopoDS_Shape aNF = aSubs.Apply(aF);
+        aSubsF.Replace(aF, aNF);
+        if (myHist.IsNull())
+        {
+          myHist = aSubs.History();
+        }
+        else
+        {
+          myHist->Merge(aSubs.History());
+        }
+        myShape = aSubsF.Apply(myShape);
+        myHist->Merge(aSubsF.History());
+        //Pair aF->aNF is in history after first replacing of edge by aNF = aSubs.Apply(aF)
+        //After merging history for replacing faces, modified list for aF contains two exemplar of aNF
+        //So, using ReplaceModified clears modified list for aF and leaves only one exemplar of aNF
+        myHist->ReplaceModified(aF, aNF);
+        aSubsF.Clear();
+      }
+    }
+  }
 }
 
 
@@ -120,22 +233,46 @@ TopoDS_Shape BRepPrimAPI_MakeRevol::LastShape()
   return myRevol.LastShape();
 }
 
-
 //=======================================================================
 //function : Generated
 //purpose  : 
 //=======================================================================
-
 const TopTools_ListOfShape& BRepPrimAPI_MakeRevol::Generated (const TopoDS_Shape& S)
 {
   myGenerated.Clear();
-  if (!myRevol.Shape (S).IsNull())
+  TopoDS_Shape aGS = myRevol.Shape(S);
-    myGenerated.Append (myRevol.Shape (S));
+  if (!aGS.IsNull())
+  { 
+    if (BRepTools_History::IsSupportedType(aGS))
+    {
+      if (myHist.IsNull())
+      {
+        myGenerated.Append(aGS);
+        return myGenerated;
+      }
+      if (myHist->Modified(aGS).IsEmpty())
+      {
+        myGenerated.Append(aGS);
+        return myGenerated;
+      }
+      //
+      TopTools_ListIteratorOfListOfShape anIt(myHist->Modified(aGS));
+      for (; anIt.More(); anIt.Next())
+      {
+        myGenerated.Append(anIt.Value());
+      }
+      if (aGS.ShapeType() == TopAbs_EDGE)
+      {
+        if (BRep_Tool::Degenerated(TopoDS::Edge(aGS)))
+        {
+          //Append initial common deg. edge
+          myGenerated.Append(aGS);
+        }
+      }
+    }
+  }
   return myGenerated;
 }
-
-
-// Modified by skv - Fri Mar  4 15:50:09 2005 Begin
 
 //=======================================================================
 //function : FirstShape
@@ -180,4 +317,4 @@ const TopTools_ListOfShape& BRepPrimAPI_MakeRevol::Degenerated () const
 {
   return myDegenerated;
 }
-// Modified by skv - Fri Mar  4 15:50:09 2005 End
+

src/BRepPrimAPI/BRepPrimAPI_MakeRevol.hxx

@@ -24,6 +24,7 @@
 #include <BRepSweep_Revol.hxx>
 #include <TopTools_ListOfShape.hxx>
 #include <BRepPrimAPI_MakeSweep.hxx>
+#include <BRepTools_History.hxx>
 #include <Standard_Real.hxx>
 #include <Standard_Boolean.hxx>
 class TopoDS_Shape;
@@ -90,6 +91,9 @@ public:
   //! Returns the TopoDS Shape of the end of the revol.
   Standard_EXPORT TopoDS_Shape LastShape() Standard_OVERRIDE;
   
+  //! Returns list of shape generated from shape S
+  //! Warning: shape S must be shape of type VERTEX, EDGE, FACE, SOLID.
+  //! For shapes of other types method always returns empty list
   Standard_EXPORT virtual const TopTools_ListOfShape& Generated (const TopoDS_Shape& S) Standard_OVERRIDE;
   
   //! Returns the TopoDS Shape of the beginning of the revolution,
@@ -106,8 +110,6 @@ public:
   Standard_EXPORT const TopTools_ListOfShape& Degenerated() const;
 
 
-
-
 protected:
 
 
@@ -120,6 +122,7 @@ private:
 
   BRepSweep_Revol myRevol;
   TopTools_ListOfShape myDegenerated;
+  Handle(BRepTools_History) myHist;
 
 
 };

src/BRepTest/BRepTest_SweepCommands.cxx

@@ -100,7 +100,6 @@ static Standard_Integer prism(Draw_Interpretor& , Standard_Integer n, const char
 //=======================================================================
 // revol
 //=======================================================================
-
 static Standard_Integer revol(Draw_Interpretor& ,
 			      Standard_Integer n, const char** a)
 {
@@ -117,10 +116,18 @@ static Standard_Integer revol(Draw_Interpretor& ,
   
   Standard_Boolean copy = n > 10;
 
-  TopoDS_Shape res = BRepPrimAPI_MakeRevol(base,A,angle,copy);
+
+  BRepPrimAPI_MakeRevol Revol(base, A, angle, copy);
+
+  TopoDS_Shape res = Revol.Shape();
 
   DBRep::Set(a[1],res);
 
+  //History 
+  TopTools_ListOfShape anArgs;
+  anArgs.Append(base);
+  BRepTest_Objects::SetHistory(anArgs, Revol);
+
   return 0;
 }
 

tests/bugs/modalg_1/bug15036

@@ -31,5 +31,5 @@ revol result fr 0.001428571428572667 640.5285714285715 20 -1.691188905560395E-05
 
 checkprops result -s 1.03597e+06 
 checkshape result
-checknbshapes result -vertex 9 -edge 14 -wire 13 -face 9 -shell 1 -solid 1 -compsolid 0 -compound 0 -shape 47
+checknbshapes result -vertex 9 -edge 15 -wire 13 -face 9 -shell 1 -solid 1 -compsolid 0 -compound 0 -shape 48
 checkview -display result -2d -path ${imagedir}/${test_image}.png

tests/bugs/modalg_7/bug29580_2

@@ -31,7 +31,7 @@ bfillds
 bbuild result
 
 checkshape result
-checknbshapes result -vertex 4 -edge 14 -wire 10 -face 10 -shell 3 -solid 3 -t
+checknbshapes result -vertex 4 -edge 16 -wire 10 -face 10 -shell 3 -solid 3 -t
 checkprops result -s 3879.55 -v 6295.15
 
 checkview -display result -2d -path ${imagedir}/${test_image}.png

tests/bugs/modalg_7/bug29586

@@ -0,0 +1,43 @@
+puts "========"
+puts "OCC29586"
+puts "========"
+puts ""
+#################################################
+# Revolution creates solid with degenerated edges shared among faces
+#################################################
+
+circle c1 0 -5 0 1 0 0 10
+circle c2 0 5 0 1 0 0 10
+mkedge e1 c1
+mkedge e2 c2
+wire w1 e1
+wire w2 e2
+mkplane f1 w1
+mkplane f2 w2
+bcut f12 f1 f2
+revol r f12 0 0 0 0 0 1 180
+
+explode r f
+
+explode r_1 e
+
+explode r_2 e
+
+explode r_3 e
+
+if {[regexp "same shapes" [compare r_1_1 r_2_1]]} { puts "Error" }
+# Error
+
+if {[regexp "same shapes" [compare r_3_2 r_2_2]]} { puts "Error" }
+# Error
+
+#testing generated
+savehistory h
+explode f12 v
+generated g1 h f12_1
+checknbshapes g1 -edge 2
+generated g2 h f12_2
+checknbshapes g2 -edge 1
+generated g3 h f12_3
+checknbshapes g3 -edge 2
+