0032857: Error when finding the intersection between a new face made after a draft and the inner face of body

Intersection surface cone is calculated for the part of the cone that corresponds to the draft plane.

src/Draft/Draft_Modification_1.cxx

@@ -16,6 +16,7 @@
 
 #include <Adaptor3d_CurveOnSurface.hxx>
 #include <Adaptor3d_CurveOnSurface.hxx>
+#include <Adaptor3d_TopolTool.hxx>
 #include <GeomAdaptor_SurfaceOfLinearExtrusion.hxx>
 #include <Approx_CurveOnSurface.hxx>
 #include <BRep_Builder.hxx>
@@ -898,8 +899,29 @@ void Draft_Modification::Perform ()
             // 1 - If ProjLib should make an Approx, it is stupid to approximate the 
             //     entire intersection curve.
             // 2 - Additionally, if YaRev, there is a risk to not be SameRange.
-            i2s.Perform(S1,S2,Precision::Confusion(),
+            Handle(GeomAdaptor_Surface) HS[2] = {new GeomAdaptor_Surface(S1), new GeomAdaptor_Surface(S2)};
-              Standard_True,Standard_False,Standard_False);
+            for (Standard_Integer i = 0; i < 2; ++i) 
+            {
+              Handle(Geom_Surface) S = (i == 0 ? S1 : S2);
+              if (S->DynamicType() == STANDARD_TYPE(Geom_ConicalSurface))
+              {
+                Standard_Real aVs, aVe;
+                Adaptor3d_TopolTool::GetConeApexParam(Handle(Geom_ConicalSurface)::DownCast(S)->Cone(), aVs, aVe);
+                if (aVe < 0.)
+                {
+                  aVs = aVe;
+                  aVe = Precision::Infinite();
+                }
+                else
+                {
+                  aVs = -Precision::Infinite();
+                  aVe = aVe;
+                }
+                HS[i]->Load(S, 0., 2 * M_PI, aVs, aVe, 0., 0.);
+              }
+            }
+            i2s.Perform(HS[0], HS[1], Precision::Confusion(),
+              Standard_True, Standard_False, Standard_False);
 
             if (!i2s.IsDone() || i2s.NbLines() <= 0) {
               errStat = Draft_EdgeRecomputation;
@@ -1108,8 +1130,9 @@ void Draft_Modification::Perform ()
                 for (i = 1; i <= Candidates.Length(); i++)
                 {
                   Handle( Geom_Curve ) aCurve = Candidates(i);
-                  gp_Pnt Pnt = aCurve->Value( aCurve->FirstParameter() );
+                  GeomAPI_ProjectPointOnCurve Projector(pfv, aCurve );
-                  const Standard_Real Dist = Pnt.Distance( pfv );
+                  gp_Pnt aPnt = Projector.NbPoints() ? Projector.NearestPoint() : aCurve->Value(aCurve->FirstParameter());
+                  const Standard_Real Dist = aPnt.Distance( pfv );
                   if (Dist - DistMin < -Precision::Confusion())
                   {
                     DistMin = Dist;

src/NCollection/NCollection_BaseSequence.hxx

@@ -58,40 +58,34 @@ public:
   {
   public:
     //! Empty constructor
-    Iterator (void) : myCurrent (NULL), myPrevious(NULL), myIsReversed(Standard_False) {}
+    Iterator (void) : myCurrent (NULL), myPrevious(NULL) {}
 
-    //! Constructor with initialization
+    //! Constructor with initialisation
     Iterator (const NCollection_BaseSequence& theSeq,
               const Standard_Boolean isStart)
     {
       Init (theSeq, isStart);
     }
 
-     //! Initialization
+     //! Initialisation
     void Init (const NCollection_BaseSequence& theSeq,
                const Standard_Boolean isStart = Standard_True)
     {
-      myCurrent = (isStart ? theSeq.myFirstItem : theSeq.myLastItem);
+      myCurrent  = (isStart ? theSeq.myFirstItem : NULL);
-      myPrevious = NULL;
+      myPrevious = (isStart ? NULL : theSeq.myLastItem);
-      myIsReversed = !isStart;
     }
 
     //! Switch to previous element; note that it will reset 
     void Previous()
     {
       myCurrent = myPrevious;
-      if (myCurrent) {
+      if (myCurrent)
-        if (!myIsReversed)
+        myPrevious = myCurrent->Previous();
-          myPrevious = myCurrent->Previous();
-        else
-          myPrevious = myCurrent->Next();
-      }
     }
       
   protected:
     NCollection_SeqNode* myCurrent;  //!< Pointer to the current node
     NCollection_SeqNode* myPrevious; //!< Pointer to the previous node
-    Standard_Boolean myIsReversed;   //!< Flag to create reversed iterator
     friend class NCollection_BaseSequence;
   };
 

src/NCollection/NCollection_Sequence.hxx

@@ -71,10 +71,7 @@ public:
       if (myCurrent)
       {
         myPrevious = myCurrent;
-        if (myIsReversed)
+        myCurrent = myCurrent->Next();
-          myCurrent = myCurrent->Previous();
-        else
-          myCurrent = myCurrent->Next();
       }
     }
     //! Constant value access
@@ -96,38 +93,19 @@ public:
   //! Shorthand for a constant iterator type.
   typedef NCollection_StlIterator<std::bidirectional_iterator_tag, Iterator, TheItemType, true> const_iterator;
 
-  //! Shorthand for a reverse iterator type.
-  typedef NCollection_StlIterator<std::reverse_iterator<iterator>, Iterator, TheItemType, false> reverse_iterator;
-
-  //! Shorthand for a constant reverse iterator type.
-  typedef NCollection_StlIterator<std::reverse_iterator<const_iterator>, Iterator, TheItemType, true> const_reverse_iterator;
-
   //! Returns an iterator pointing to the first element in the sequence.
   iterator begin() const { return Iterator (*this, true); }
 
   //! Returns an iterator referring to the past-the-end element in the sequence.
-  iterator end() const { Iterator anIter (*this, false); anIter.Previous(); return anIter; }
+  iterator end() const { Iterator anIter (*this, false); anIter.Next(); return anIter; }
   
   //! Returns a const iterator pointing to the first element in the sequence.
   const_iterator cbegin() const { return Iterator (*this, true); }
 
   //! Returns a const iterator referring to the past-the-end element in the sequence.
-  const_iterator cend() const { Iterator anIter(*this, false); anIter.Previous(); return anIter; }
+  const_iterator cend() const { Iterator anIter (*this, false); anIter.Next(); return anIter; }
 
-  //! Returns a reverse iterator pointing to the last element in the sequence.
+ public:
-  reverse_iterator rbegin() const { return Iterator(*this, false); }
-
-  //! Returns a reverse iterator referring to the past-the-end element in the reversed sequence.
-  reverse_iterator rend() const { Iterator anIter(*this, true); anIter.Previous(); return anIter; }
-
-  //! Returns a const reverse iterator pointing to the last element in the sequence.
-  const_reverse_iterator crbegin() const { return Iterator(*this, false); }
-
-  //! Returns a const reverse iterator referring to the past-the-end element in the reversed sequence.
-  const_reverse_iterator crend() const { Iterator anIter(*this, true); anIter.Previous(); return anIter; }
-
-
-public:
   // ---------- PUBLIC METHODS ------------
 
   //! Empty constructor.

src/NCollection/NCollection_StlIterator.hxx

@@ -134,9 +134,7 @@ public: //! @name methods related to bidirectional STL iterator
   NCollection_StlIterator& operator--()
   {
     Standard_STATIC_ASSERT((opencascade::std::is_same<std::bidirectional_iterator_tag,Category>::value ||
-                            opencascade::std::is_same<std::random_access_iterator_tag,Category>::value ||
+                            opencascade::std::is_same<std::random_access_iterator_tag,Category>::value));
-                            opencascade::std::is_same<std::reverse_iterator<NCollection_StlIterator<std::bidirectional_iterator_tag, BaseIterator, ItemType, false>>, Category>::value ||
-                            opencascade::std::is_same<std::reverse_iterator<NCollection_StlIterator<std::bidirectional_iterator_tag, BaseIterator, ItemType, true>>, Category>::value));
     myIterator.Previous();
     return *this;
   }

src/QABugs/QABugs_20.cxx

@@ -4359,74 +4359,7 @@ static Standard_Integer QACheckBends(Draw_Interpretor& theDI,
   return 0;
 }
 
-#include <NCollection_Sequence.hxx>
-static Standard_Integer OCC27571(Draw_Interpretor& di, Standard_Integer /*nb*/, const char ** /*a*/)
-{
-  // Create NCollection_Sequence<TCollection_AsciiString>
-  NCollection_Sequence<TCollection_AsciiString> aSequence;
-  aSequence.Append("1");
-  aSequence.Append("2");
-  aSequence.Append("3");
-  aSequence.Append("4");
-  aSequence.Append("5");
 
-  // Create iterator
-  NCollection_Sequence<TCollection_AsciiString>::Iterator anIterator(aSequence);
-
-  // Create reverse iterator
-  NCollection_Sequence<TCollection_AsciiString>::Iterator aReverseIterator(aSequence, Standard_False);
-
-  // Display elements using iterator (with More/Next)
-  di << "Iterator (More/Next):";
-  for (; anIterator.More(); anIterator.Next())
-    di << " " << anIterator.Value();
-  anIterator.Previous();
-  di << "\nCurrent (last) element - Iterator: " << anIterator.Value();
-  anIterator.Previous();
-  di << "\nPrevious element - Iterator: " << anIterator.Value();
-
-  // Display elements using reverse iterator (with More/Next)
-  di << "\nReverse iterator (More/Next):";
-  for (; aReverseIterator.More(); aReverseIterator.Next())
-    di << " " << aReverseIterator.Value();
-  aReverseIterator.Previous();
-  di << "\nCurrent (last) element - Reverse iterator: " << aReverseIterator.Value();
-  aReverseIterator.Previous();
-  di << "\nPrevious element - Reverse iterator: " << aReverseIterator.Value();
-
-  // Display elements using iterator (with begin/end)
-  di << "\nIterator (begin/end):";
-  NCollection_Sequence<TCollection_AsciiString>::iterator anIteratorBeginEnd = aSequence.begin();
-  for (; anIteratorBeginEnd != aSequence.end(); ++anIteratorBeginEnd)
-    di << " " << *anIteratorBeginEnd;
-  --anIteratorBeginEnd;
-  di << "\nCurrent (last) element - Iterator (begin/end): " << *anIteratorBeginEnd;
-
-  // Display elements using const_iterator (with cbegin/cend)
-  di << "\nIterator (cbegin/cend) Const:";
-  NCollection_Sequence<TCollection_AsciiString>::const_iterator anIteratorBeginEndConst = aSequence.cbegin();
-  for (; anIteratorBeginEndConst != aSequence.cend(); ++anIteratorBeginEndConst)
-    di << " " << *anIteratorBeginEndConst;
-  --anIteratorBeginEndConst;
-  di << "\nCurrent (last) element - Iterator (cbegin/cend) Const: " << *anIteratorBeginEndConst;
-
-  // Display elements using reverse_iterator (with rbegin/rend)
-  di << "\nReverse iterator (rbegin/rend):";
-  NCollection_Sequence<TCollection_AsciiString>::reverse_iterator aReverseIteratorBeginEnd = aSequence.rbegin();
-  for (; aReverseIteratorBeginEnd != aSequence.rend(); ++aReverseIteratorBeginEnd)
-    di << " " << *aReverseIteratorBeginEnd;
-  --aReverseIteratorBeginEnd;
-  di << "\nCurrent (last) element - Reverse iterator (rbegin/rend): " << *aReverseIteratorBeginEnd;
-
-  // Display elements using const_reverse_iterator (with crbegin/crend)
-  di << "\nReverse iterator (crbegin/crend) Const:";
-  NCollection_Sequence<TCollection_AsciiString>::const_reverse_iterator aReverseIteratorBeginEndConst = aSequence.crbegin();
-  for (; aReverseIteratorBeginEndConst != aSequence.crend(); ++aReverseIteratorBeginEndConst)
-    di << " " << *aReverseIteratorBeginEndConst;
-  --aReverseIteratorBeginEndConst;
-  di << "\nCurrent (last) element - Reverse iterator (crbegin/crend) Const: " << *aReverseIteratorBeginEndConst;
-  return 0;
-}
 
 void QABugs::Commands_20(Draw_Interpretor& theCommands) {
   const char *group = "QABugs";
@@ -4534,7 +4467,5 @@ void QABugs::Commands_20(Draw_Interpretor& theCommands) {
     __FILE__,
     QACheckBends, group);
 
-  theCommands.Add("OCC27571", "OCC27571", __FILE__, OCC27571, group);
-
   return;
 }

tests/bugs/fclasses/bug27571

@@ -1,118 +0,0 @@
-puts "========"
-puts "OCC27571"
-puts "========"
-puts ""
-########################################################
-# Bad implementation of NCollection_Sequence::Iterator
-########################################################
-
-pload QAcommands
-
-set info [OCC27571]
-
-puts ""
-
-set Sequence "1 2 3 4 5"
-set Sequence_Last "5"
-set Sequence_Preult "4"
-set ReversedSequence "5 4 3 2 1"
-set ReversedSequence_Last "1"
-set ReversedSequence_Preult "2"
-
-# Iterator using Init(aSequence, Standard_True) method.
-if { [regexp "Iterator \\(More/Next\\): ${Sequence}" $info] } {
-  puts "OK : Iterator (More/Next)"
-} else {
-  puts "Error : Iterator (More/Next)"
-}
-
-# Method Previous() called after full iteration, current value of Iterator is equal to last element of sequence
-if { [regexp "Current \\(last\\) element - Iterator: ${Sequence_Last}" $info] } {
-  puts "OK : Current (last) element - Iterator"
-} else {
-  puts "Error : Current (last) element - Iterator"
-}
-
-# Method Previous() called once more again, current value of Iterator is equal to penult element of sequence
-if { [regexp "Previous element - Iterator: ${Sequence_Preult}" $info] } {
-  puts "OK : Previous element - Iterator"
-} else {
-  puts "Error : Previous element - Iterator"
-}
-
-# Reverse iterator using Init(aSequence, Standard_False) method.
-if { [regexp "Reverse iterator \\(More/Next\\): ${ReversedSequence}" $info] } {
-  puts "OK : Reverse iterator (More/Next)"
-} else {
-  puts "Error : Reverse iterator (More/Next)"
-}
-
-# Method Previous() called after full iteration, current value of Iterator is equal to last element of sequence
-if { [regexp "Current \\(last\\) element - Reverse iterator: ${ReversedSequence_Last}" $info] } {
-  puts "OK : Current (last) element - Reverse iterator"
-} else {
-  puts "Error : Current (last) element - Reverse iterator"
-}
-
-# Method Previous() called once more again, current value of Iterator is equal to penult element of sequence
-if { [regexp "Previous element - Reverse iterator: ${ReversedSequence_Preult}" $info] } {
-  puts "OK : Previous element - Reverse iterator"
-} else {
-  puts "Error : Previous element - Reverse iterator"
-}
-
-# Iterator using method "begin".
-if { [regexp "Iterator \\(begin/end\\): ${Sequence}" $info] } {
-  puts "OK : Iterator (begin/end)"
-} else {
-  puts "Error : Iterator (begin/end)"
-}
-
-# Check previous element after full iteration, current value of Iterator is equal to last element of sequence
-if { [regexp "Current \\(last\\) element - Iterator \\(begin/end\\): ${Sequence_Last}" $info] } {
-  puts "OK : Current (last) element - Iterator (begin/end)"
-} else {
-  puts "Error : Current (last) element - Iterator (begin/end)"
-}
-
-# Const iterator using method "begin".
-if { [regexp "Iterator \\(cbegin/cend\\) Const: ${Sequence}" $info] } {
-  puts "OK : Iterator (cbegin/cend) Const"
-} else {
-  puts "Error : Iterator (cbegin/cend) Const"
-}
-
-# Check previous element after full iteration, current value of Iterator is equal to last element of sequence
-if { [regexp "Current \\(last\\) element - Iterator \\(cbegin/cend\\) Const: ${Sequence_Last}" $info] } {
-  puts "OK : Current (last) element - Iterator (cbegin/cend) Const"
-} else {
-  puts "Error : Current (last) element - Iterator (cbegin/cend) Const"
-}
-
-# Reverse iterator using method "rbegin".
-if { [regexp "Reverse iterator \\(rbegin/rend\\): ${ReversedSequence}" $info] } {
-  puts "OK : Reverse iterator (rbegin/rend)"
-} else {
-  puts "Error : Reverse iterator (rbegin/rend)"
-}
-
-# Check previous element after full iteration, current value of Iterator is equal to last element of sequence
-if { [regexp "Current \\(last\\) element - Reverse iterator \\(rbegin/rend\\): ${ReversedSequence_Last}" $info] } {
-  puts "OK : Current (last) element - Iterator (rbegin/rend)"
-} else {
-  puts "Error : Current (last) element - Iterator (rbegin/rend)"
-}
-
-# Const reverse iterator using method "crbegin".
-if { [regexp "Reverse iterator \\(crbegin/crend\\) Const: ${ReversedSequence}" $info] } {
-  puts "OK : Reverse iterator (crbegin/crend) Const"
-} else {
-  puts "Error : Reverse iterator (crbegin/crend) Const"
-}
-
-# Check previous element after full iteration, current value of Iterator is equal to last element of sequence
-if { [regexp "Current \\(last\\) element - Reverse iterator \\(crbegin/crend\\) Const: ${ReversedSequence_Last}" $info] } {
-  puts "OK : Current (last) element - Iterator (rbegin/rend) Const"
-} else {
-  puts "Error : Current (last) element - Iterator (rbegin/rend) Const"
-}

tests/bugs/modalg_8/bug32857

@@ -0,0 +1,72 @@
+puts "================"
+puts "0032857: Problem when finding the intersection between a new face made after a draft and the inner face of body"
+puts "================"
+puts ""
+# Script reproducing the problematic draft case in FreeCAD issue #2497
+#Category: Modeling
+#Title: OCCT Tutorial pocketed ring
+
+pload MODELING VISUALIZATION
+
+# Set basic dimensions. Problems appear when inner_rad < pocket_center.
+dset height 10
+dset inner_rad 39
+dset outer_rad 50
+dset pocket_center 40
+dset pocket_rad 20
+dset pocket_depth 5
+
+# Construct base profile (the "my_ring")
+puts "Constructing my_ring..."
+
+circle c_inner 0 0 0 0 0 1 inner_rad
+circle c_outer 0 0 0 0 0 1 outer_rad
+mkedge e_inner c_inner
+mkedge e_outer c_outer
+wire w_inner e_inner
+wire w_outer e_outer
+
+plane p0
+mkface my_ring_inner_base p0 w_inner
+mkface my_ring_outer_base p0 w_outer
+
+prism my_ring_inner my_ring_inner_base 0 0 height
+prism my_ring_outer my_ring_outer_base 0 0 height
+
+bcut my_ring my_ring_outer my_ring_inner
+
+# Make the pocket
+puts "Constructing pocket..."
+
+circle pocket_base pocket_center 0 0 0 0 1 pocket_rad
+mkedge pocket_base pocket_base
+wire pocket_base pocket_base
+
+mkface pocket_base p0 pocket_base
+
+prism my_pocket pocket_base 0 0 pocket_depth
+
+# Make the cut
+puts "Making the cut"
+
+bcut slotted_ring my_ring my_pocket
+
+explode slotted_ring F
+
+# Make the draft
+puts "Drafting the face"
+
+# Found face by trial and error: slotted_ring_3
+# Perform the draft
+depouille slotted_ring_with_draft slotted_ring 0 0 -1 slotted_ring_3 44 0 1 0 0 0 1
+
+#checking result
+checkshape slotted_ring_with_draft
+checknbshapes slotted_ring_with_draft -shape 48 -vertex 12 -edge 18 -face 7 -wire 8
+checkprops slotted_ring_with_draft -s 11466.5 -v 28662.9
+
+puts "Showing result..."
+
+# Display result
+checkview -display slotted_ring_with_draft -2d -path ${imagedir}/${test_image}.png
+