From 4ca7548ffe0ce934595d606b1c7f923f568638d4 Mon Sep 17 00:00:00 2001
From: Jonathan Westhues <jwesthues@cq.cx>
Date: Mon, 29 Jun 2009 20:38:40 -0800
Subject: [PATCH] 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   | 20 ++++++++++++++++
 polygon.h     |  2 ++
 srf/merge.cpp | 65 +++++++++++++++++++++++++++++++++++----------------
 3 files changed, 67 insertions(+), 20 deletions(-)

diff --git a/polygon.cpp b/polygon.cpp
index e7c08868..b320f07b 100644
--- a/polygon.cpp
+++ b/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.
diff --git a/polygon.h b/polygon.h
index 26288207..2bb19b82 100644
--- a/polygon.h
+++ b/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);
 };
diff --git a/srf/merge.cpp b/srf/merge.cpp
index 6d08560f..b9f9bbb7 100644
--- a/srf/merge.cpp
+++ 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();