0033171: Modeling Algorithms - Invalid result of faces unification

Avoid unification of the faces belonging to the different shells.

src/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.cxx

@@ -2745,6 +2745,26 @@ void ShapeUpgrade_UnifySameDomain::UnifyFaces()
   for (Standard_Integer i = 1; i <= aFaceMap.Extent(); i++)
     TopExp::MapShapesAndAncestors (aFaceMap(i), TopAbs_EDGE, TopAbs_FACE, aGMapEdgeFaces);
 
+  // creating map of face shells for the whole shape to avoid
+  // unification of faces belonging to the different shells
+  DataMapOfShapeMapOfShape aGMapFaceShells;
+  for (TopExp_Explorer anExp (myShape, TopAbs_SHELL); anExp.More(); anExp.Next())
+  {
+    const TopoDS_Shape& aShell = anExp.Current();
+    for (TopoDS_Iterator anItF (aShell); anItF.More(); anItF.Next())
+    {
+      const TopoDS_Shape& aF = anItF.Value();
+      if (TopTools_MapOfShape* pShells = aGMapFaceShells.ChangeSeek (aF))
+      {
+        pShells->Add (aShell);
+      }
+      else
+      {
+        (aGMapFaceShells.Bound (aF, TopTools_MapOfShape()))->Add (aShell);
+      }
+    }
+  }
+
   // creating map of free boundaries
   TopTools_MapOfShape aFreeBoundMap;
   // look at only shells not belonging to solids
@@ -2766,7 +2786,7 @@ void ShapeUpgrade_UnifySameDomain::UnifyFaces()
   // unify faces in each shell separately
   TopExp_Explorer exps;
   for (exps.Init(myShape, TopAbs_SHELL); exps.More(); exps.Next())
-    IntUnifyFaces(exps.Current(), aGMapEdgeFaces, aFreeBoundMap);
+    IntUnifyFaces(exps.Current(), aGMapEdgeFaces, aGMapFaceShells, aFreeBoundMap);
 
   // gather all faces out of shells in one compound and unify them at once
   BRep_Builder aBB;
@@ -2777,7 +2797,10 @@ void ShapeUpgrade_UnifySameDomain::UnifyFaces()
     aBB.Add(aCmp, exps.Current());
 
   if (nbf > 0)
-    IntUnifyFaces(aCmp, aGMapEdgeFaces, aFreeBoundMap);
+  {
+    // No connection to shells, thus no need to pass the face-shell map
+    IntUnifyFaces(aCmp, aGMapEdgeFaces, DataMapOfShapeMapOfShape(), aFreeBoundMap);
+  }
   
   myShape = myContext->Apply(myShape);
 }
@@ -2800,13 +2823,52 @@ static void SetFixWireModes(ShapeFix_Face& theSff)
   aFixWire->FixSmallMode() = 0;
 }
 
+//=======================================================================
+//function : isSameSets
+//purpose  : Compares two sets of shapes. Returns true if they are the same,
+//           false otherwise.
+//=======================================================================
+
+template<class Container>
+static Standard_Boolean isSameSets(const Container* theFShells1,
+                                   const Container* theFShells2)
+{
+  // If both are null - no problem
+  if (theFShells1 == nullptr && theFShells2 == nullptr)
+  {
+    return Standard_True;
+  }
+  // If only one is null - not the same
+  if (theFShells1 == nullptr || theFShells2 == nullptr)
+  {
+    return Standard_False;
+  }
+  // Both not null
+  if (theFShells1->Extent() != theFShells2->Extent())
+  {
+    return Standard_False;
+  }
+  // number of shells in each set should be very small in normal cases - max 2.
+  // thus just check if all objects of one are contained in the other and vice versa.
+  for (typename Container::Iterator it1(*theFShells1), it2(*theFShells2);
+       it1.More() && it2.More(); it1.Next(), it2.Next())
+  {
+    if (!theFShells1->Contains(it2.Value()) || !theFShells2->Contains(it1.Value()))
+    {
+      return Standard_False;
+    }
+  }
+  return Standard_True;
+}
+
 //=======================================================================
 //function : IntUnifyFaces
 //purpose  : 
 //=======================================================================
 
 void ShapeUpgrade_UnifySameDomain::IntUnifyFaces(const TopoDS_Shape& theInpShape,
-                                                 TopTools_IndexedDataMapOfShapeListOfShape& theGMapEdgeFaces,
+                                                 const TopTools_IndexedDataMapOfShapeListOfShape& theGMapEdgeFaces,
+                                                 const DataMapOfShapeMapOfShape& theGMapFaceShells,
                                                  const TopTools_MapOfShape& theFreeBoundMap)
 {
   // creating map of edge faces for the shape
@@ -2855,6 +2917,9 @@ void ShapeUpgrade_UnifySameDomain::IntUnifyFaces(const TopoDS_Shape& theInpShape
     Standard_Real Uperiod = (aBaseSurface->IsUPeriodic())? aBaseSurface->UPeriod() : 0.;
     Standard_Real Vperiod = (aBaseSurface->IsVPeriodic())? aBaseSurface->VPeriod() : 0.;
 
+    // Get shells connected to the face (in normal cases should not be more than 2)
+    const TopTools_MapOfShape* pFShells1 = theGMapFaceShells.Seek (aFace);
+
     // find adjacent faces to union
     Standard_Integer i;
     for (i = 1; i <= edges.Length(); i++) {
@@ -2903,6 +2968,15 @@ void ShapeUpgrade_UnifySameDomain::IntUnifyFaces(const TopoDS_Shape& theInpShape
         if (aProcessed.Contains(aCheckedFace))
           continue;
 
+        // Get shells connected to the checked face
+        const TopTools_MapOfShape* pFShells2 = theGMapFaceShells.Seek (aCheckedFace);
+        // Faces can be unified only if the shells of faces connected to
+        // these faces are the same. Otherwise, topology would be broken.
+        if (!isSameSets (pFShells1, pFShells2))
+        {
+          continue;
+        }
+
         if (bCheckNormals) {
           // get normal of checked face using the same parameter on edge
           gp_Dir aDN2;

src/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.hxx

@@ -70,6 +70,7 @@ class ShapeUpgrade_UnifySameDomain : public Standard_Transient
 public:
 
   typedef NCollection_DataMap<TopoDS_Shape, Handle(Geom_Plane), TopTools_ShapeMapHasher> DataMapOfFacePlane;
+  typedef NCollection_DataMap<TopoDS_Shape, TopTools_MapOfShape, TopTools_ShapeMapHasher> DataMapOfShapeMapOfShape;
   
   //! Empty constructor
   Standard_EXPORT ShapeUpgrade_UnifySameDomain();
@@ -168,7 +169,8 @@ protected:
   Standard_EXPORT void UnifyEdges();
 
   void IntUnifyFaces(const TopoDS_Shape& theInpShape,
-                     TopTools_IndexedDataMapOfShapeListOfShape& theGMapEdgeFaces,
+                     const TopTools_IndexedDataMapOfShapeListOfShape& theGMapEdgeFaces,
+                     const DataMapOfShapeMapOfShape& theGMapFaceShells,
                      const TopTools_MapOfShape& theFreeBoundMap);
 
   //! Splits the sequence of edges into the sequence of chains

tests/bugs/heal/bug33171_1

@@ -0,0 +1,71 @@
+puts "========================"
+puts " 	0033171: Modeling Algorithms - Invalid result of faces unification"
+puts "========================"
+puts ""
+
+# make outer prism
+polyline p 0 0 0  10 0 0  10 10 0  0 10 0  0 0 0
+mkplane f p
+prism s f 0 0 5
+
+# make section shells
+polyline p1 3 10 0  3 7 0  6 7 0  6 3 0  10 3 0
+polyline p2 6 7 0  10 7 0
+polyline p3 8 7 0  8 10 0
+polyline p4 0 5 0  10 5 0
+
+prism sh1 p1 0 0 5
+prism sh2 p2 0 0 5
+prism sh3 p3 0 0 5
+prism sh4 p4 0 0 5
+
+# split the prism
+bclearobjects
+bcleartools
+baddobjects s
+baddtools sh1 sh2 sh3 sh4
+bfillds
+bsplit r
+
+checkshape r
+if {![regexp "This shape seems to be OK" [bopcheck r]]} {
+    puts "Error: invalid shape after split"
+}
+
+# try to unify faces in the result compound
+unifysamedom ru1 r
+unifysamedom ru2 r +i
+
+checkshape ru1
+checkshape ru2
+
+checknbshapes ru1 -ref [nbshapes r -t] -t
+checknbshapes ru2 -ref [nbshapes r -t] -t
+
+if {![regexp "This shape seems to be OK" [bopcheck ru1]]} {
+    puts "Error: invalid shape after faces unification"
+}
+if {![regexp "This shape seems to be OK" [bopcheck ru2]]} {
+    puts "Error: invalid shape after faces unification"
+}
+
+# make compound of shells
+eval compound [explode r] shs
+
+unifysamedom shsu1 r
+unifysamedom shsu2 r +i
+
+checkshape shsu1
+checkshape shsu2
+
+checknbshapes shsu1 -ref [nbshapes shs -t] -t
+checknbshapes shsu2 -ref [nbshapes shs -t] -t
+
+if {![regexp "This shape seems to be OK" [bopcheck shsu1]]} {
+    puts "Error: invalid shape after faces unification"
+}
+if {![regexp "This shape seems to be OK" [bopcheck shsu2]]} {
+    puts "Error: invalid shape after faces unification"
+}
+
+checkview -display ru2 -2d -path ${imagedir}/${test_image}.png

tests/bugs/heal/bug33171_2

@@ -0,0 +1,57 @@
+puts "========================"
+puts " 	0033171: Modeling Algorithms - Invalid result of faces unification"
+puts "========================"
+puts ""
+
+# make two solids
+box b1 10 10 5
+box b2 10 0 0 5 5 5
+# make shared
+bclearobjects
+bcleartools
+baddobjects b1 b2
+bfillds
+bbuild r
+
+checkshape r
+if {![regexp "This shape seems to be OK" [bopcheck r]]} {
+    puts "Error: invalid shape after fuse"
+}
+
+# try to unify faces in the result compound
+unifysamedom ru1 r
+unifysamedom ru2 r +i
+
+checkshape ru1
+checkshape ru2
+
+checknbshapes ru1 -ref [nbshapes r -t] -t
+checknbshapes ru2 -ref [nbshapes r -t] -t
+
+if {![regexp "This shape seems to be OK" [bopcheck ru1]]} {
+    puts "Error: invalid shape after faces unification"
+}
+if {![regexp "This shape seems to be OK" [bopcheck ru2]]} {
+    puts "Error: invalid shape after faces unification"
+}
+
+# make compound of shells
+eval compound [explode r] shs
+
+unifysamedom shsu1 r
+unifysamedom shsu2 r +i
+
+checkshape shsu1
+checkshape shsu2
+
+checknbshapes shsu1 -ref [nbshapes shs -t] -t
+checknbshapes shsu2 -ref [nbshapes shs -t] -t
+
+if {![regexp "This shape seems to be OK" [bopcheck shsu1]]} {
+    puts "Error: invalid shape after faces unification"
+}
+if {![regexp "This shape seems to be OK" [bopcheck shsu2]]} {
+    puts "Error: invalid shape after faces unification"
+}
+
+checkview -display ru2 -2d -path ${imagedir}/${test_image}.png