Don't merge two coincident surfaces unless they share an edge.

Otherwise, we might merge in ways that make things slower (because
the bboxes aren't as tight) or less robust (because the
intersection needs to be split in more places, and that might fail).

[git-p4: depot-paths = "//depot/solvespace/": change = 2003]

polygon.cpp

@@ -225,6 +225,26 @@ intersects:
     return cnt;
 }
 
+//-----------------------------------------------------------------------------
+// Returns true if the intersecting edge list contains an edge that shares
+// an endpoint with one of our edges.
+//-----------------------------------------------------------------------------
+bool SEdgeList::ContainsEdgeFrom(SEdgeList *sel) {
+    SEdge *se;
+    for(se = l.First(); se; se = l.NextAfter(se)) {
+        if(sel->ContainsEdge(se)) return true;
+    }
+    return false;
+}
+bool SEdgeList::ContainsEdge(SEdge *set) {
+    SEdge *se;
+    for(se = l.First(); se; se = l.NextAfter(se)) {
+        if((se->a).Equals(set->a) && (se->b).Equals(set->b)) return true;
+        if((se->b).Equals(set->a) && (se->a).Equals(set->b)) return true;
+    }
+    return false;
+}
+
 //-----------------------------------------------------------------------------
 // Remove unnecessary edges: if two are anti-parallel then remove both, and if
 // two are parallel then remove one.

polygon.h

@@ -26,6 +26,8 @@ public:
     bool AssembleContour(Vector first, Vector last, SContour *dest,
                             SEdge *errorAt, bool keepDir);
     int AnyEdgeCrossings(Vector a, Vector b, Vector *pi=NULL);
+    bool ContainsEdgeFrom(SEdgeList *sel);
+    bool ContainsEdge(SEdge *se);
     void CullExtraneousEdges(void);
     void MergeCollinearSegments(Vector a, Vector b);
 };

srf/merge.cpp

@@ -16,36 +16,60 @@ void SShell::MergeCoincidentSurfaces(void) {
         // Let someone else clean up the empty surfaces; we can certainly merge
         // them, but we don't know how to calculate a reasonable bounding box.
         if(si->trim.n == 0) continue;
+        // And for now we handle only coincident planes, so no sense wasting
+        // time on other surfaces.
+        if(si->degm != 1 || si->degn != 1) continue;
 
         SEdgeList sel;
         ZERO(&sel);
+        si->MakeEdgesInto(this, &sel, false);
 
-        bool merged = false;
+        bool mergedThisTime, merged = false;
+        do {
+            mergedThisTime = false;
 
-        for(j = i + 1; j < surface.n; j++) {
-            sj = &(surface.elem[j]);
-            if(sj->tag) continue;
-            if(!sj->CoincidentWith(si, true)) continue;
-            if(sj->color != si->color) continue;
-            // But we do merge surfaces with different face entities, since
-            // otherwise we'd hardly ever merge anything.
+            for(j = i + 1; j < surface.n; j++) {
+                sj = &(surface.elem[j]);
+                if(sj->tag) continue;
+                if(!sj->CoincidentWith(si, true)) continue;
+                if(sj->color != si->color) continue;
+                // But we do merge surfaces with different face entities, since
+                // otherwise we'd hardly ever merge anything.
 
-            // This surface is coincident, so it gets merged.
-            sj->tag = 1;
-            merged = true;
-            sj->MakeEdgesInto(this, &sel, false);
-            sj->trim.Clear();
+                // This surface is coincident. But let's not merge coincident
+                // surfaces if they contain disjoint contours; that just makes
+                // the bounding box tests less effective, and possibly things
+                // less robust.
+                SEdgeList tel;
+                ZERO(&tel);
+                sj->MakeEdgesInto(this, &tel, false);
+                if(!sel.ContainsEdgeFrom(&tel)) {
+                    tel.Clear();
+                    continue;
+                }
+                tel.Clear();
 
-            // All the references to this surface get replaced with the new srf
-            SCurve *sc;
-            for(sc = curve.First(); sc; sc = curve.NextAfter(sc)) {
-                if(sc->surfA.v == sj->h.v) sc->surfA = si->h;
-                if(sc->surfB.v == sj->h.v) sc->surfB = si->h;
+                sj->tag = 1;
+                merged = true;
+                mergedThisTime = true;
+                sj->MakeEdgesInto(this, &sel, false);
+                sj->trim.Clear();
+
+                // All the references to this surface get replaced with the
+                // new srf
+                SCurve *sc;
+                for(sc = curve.First(); sc; sc = curve.NextAfter(sc)) {
+                    if(sc->surfA.v == sj->h.v) sc->surfA = si->h;
+                    if(sc->surfB.v == sj->h.v) sc->surfB = si->h;
+                }
             }
-        }
+
+            // If this iteration merged a contour onto ours, then we have to
+            // go through the surfaces again; that might have made a new
+            // surface touch us.
+        } while(mergedThisTime);
 
         if(merged) {
-            si->MakeEdgesInto(this, &sel, false);
             sel.CullExtraneousEdges();
             si->trim.Clear();
             si->TrimFromEdgeList(&sel, false);
@@ -95,6 +119,7 @@ void SShell::MergeCoincidentSurfaces(void) {
             si->ctrl[1][0] =
                 Vector::From(umax, vmin, nt).ScaleOutOfCsys(u, v, n);
         }
+        sel.Clear();
     }
 
     surface.RemoveTagged();