Merge pull request #611 from OskarLinde/minkowski2_bug

Minkowski2 bug

src/clipper-utils.cc

@@ -126,6 +126,53 @@ namespace ClipperUtils {
 		return apply(pathsvector, clipType);
 	}
 
+
+	// This is a copy-paste from ClipperLib with the modification that the union operation is not performed
+	// The reason is numeric robustness. With the insides missing, the intersection points created by the union operation may
+	// (due to rounding) be located at slightly different locations than the original geometry and this
+	// can give rise to cracks
+	static void minkowski_outline(const ClipperLib::Path& poly, const ClipperLib::Path& path,
+								  ClipperLib::Paths& quads, bool isSum, bool isClosed)
+	{
+		int delta = (isClosed ? 1 : 0);
+		size_t polyCnt = poly.size();
+		size_t pathCnt = path.size();
+		ClipperLib::Paths pp;
+		pp.reserve(pathCnt);
+		if (isSum)
+			for (size_t i = 0; i < pathCnt; ++i)
+			{
+				ClipperLib::Path p;
+				p.reserve(polyCnt);
+				for (size_t j = 0; j < poly.size(); ++j)
+					p.push_back(ClipperLib::IntPoint(path[i].X + poly[j].X, path[i].Y + poly[j].Y));
+				pp.push_back(p);
+			}
+		else
+			for (size_t i = 0; i < pathCnt; ++i)
+			{
+				ClipperLib::Path p;
+				p.reserve(polyCnt);
+				for (size_t j = 0; j < poly.size(); ++j)
+					p.push_back(ClipperLib::IntPoint(path[i].X - poly[j].X, path[i].Y - poly[j].Y));
+				pp.push_back(p);
+			}
+
+		quads.reserve((pathCnt + delta) * (polyCnt + 1));
+		for (size_t i = 0; i < pathCnt - 1 + delta; ++i)
+			for (size_t j = 0; j < polyCnt; ++j)
+			{
+				ClipperLib::Path quad;
+				quad.reserve(4);
+				quad.push_back(pp[i % pathCnt][j % polyCnt]);
+				quad.push_back(pp[(i + 1) % pathCnt][j % polyCnt]);
+				quad.push_back(pp[(i + 1) % pathCnt][(j + 1) % polyCnt]);
+				quad.push_back(pp[i % pathCnt][(j + 1) % polyCnt]);
+				if (!Orientation(quad)) ClipperLib::ReversePath(quad);
+				quads.push_back(quad);
+			}
+	}
+	
 	// Add the polygon a translated to an arbitrary point of each separate component of b.
   // Ideally, we would translate to the midpoint of component b, but the point can
 	// be chosen arbitrarily since the translated object would always stay inside
@@ -150,15 +197,20 @@ namespace ClipperUtils {
 
 	Polygon2d *applyMinkowski(const std::vector<const Polygon2d*> &polygons)
 	{
+		if (polygons.size() == 1) return new Polygon2d(*polygons[0]); // Just copy
+
+		ClipperLib::Clipper c;
 		ClipperLib::Paths lhs = ClipperUtils::fromPolygon2d(*polygons[0]);
+
 		for (size_t i=1; i<polygons.size(); i++) {
 			ClipperLib::Paths minkowski_terms;
 			ClipperLib::Paths rhs = ClipperUtils::fromPolygon2d(*polygons[i]);
+
 			// First, convolve each outline of lhs with the outlines of rhs
 			BOOST_FOREACH(ClipperLib::Path const& rhs_path, rhs) {
 				BOOST_FOREACH(ClipperLib::Path const& lhs_path, lhs) {
 					ClipperLib::Paths result;
-					ClipperLib::MinkowskiSum(lhs_path, rhs_path, result, true);
+					minkowski_outline(lhs_path, rhs_path, result, true, true);
 					minkowski_terms.insert(minkowski_terms.end(), result.begin(), result.end());
 				}
 			}
@@ -166,13 +218,20 @@ namespace ClipperUtils {
 			// Then, fill the central parts
 			fill_minkowski_insides(lhs, rhs, minkowski_terms);
 			fill_minkowski_insides(rhs, lhs, minkowski_terms);
-			lhs = minkowski_terms;
+
+			// This union operation must be performed at each interation since the minkowski_terms
+			// now contain lots of small quads
+			c.Clear();
+			c.AddPaths(minkowski_terms, ClipperLib::ptSubject, true);
+
+			if (i != polygons.size() - 1)
+				c.Execute(ClipperLib::ctUnion, lhs, ClipperLib::pftNonZero, ClipperLib::pftNonZero);
 		}
-		
-		// Finally, merge the Minkowski terms
-		std::vector<ClipperLib::Paths> pathsvec;
-		pathsvec.push_back(lhs);
-		return ClipperUtils::apply(pathsvec, ClipperLib::ctUnion);
+
+		ClipperLib::PolyTree polytree;
+		c.Execute(ClipperLib::ctUnion, polytree, ClipperLib::pftNonZero, ClipperLib::pftNonZero);
+
+		return toPolygon2d(polytree);
 	}
 
 };

testdata/scad/features/minkowski2-tests.scad

@@ -15,7 +15,6 @@ module roundedBox2dCut() {
     }
 }
 
-// Not quite correct, result does not contain a hole, since the impl currently returns the outer boundary of the polygon_with_holes.
 module roundedBox2dHole() {
     minkowski() {
         difference() {
@@ -39,3 +38,27 @@ translate([-20,-20,0]) minkowski() {
     square(2, center=true);
     circle(1);
 }
+
+module invert() render() difference() { square(1e6,center=true); child(); }
+module erode(d=.3) invert() minkowski() { circle(d); invert() child(); }
+
+// This particular combination created a hairline crack inside the resulting polygon
+translate([-5,-45]) scale(4) erode() minkowski() {
+	circle(r=.4);
+	circle(r=4);
+}
+
+// This is an even harder example
+translate([30,-30]) scale(4) erode() minkowski() {
+	difference() {
+		circle(r=.4);
+		circle(r=.399);
+	}
+	circle(r=4);
+}
+
+// Minkowski with an empty polygon should yield an empty result
+translate([30,-45]) minkowski() {
+	circle(r=1);
+	circle(r=0);
+}

tests/regression/dumptest/minkowski2-tests-expected.csg

@@ -41,4 +41,67 @@ group() {
 			circle($fn = 0, $fa = 12, $fs = 2, r = 1);
 		}
 	}
+	multmatrix([[1, 0, 0, -5], [0, 1, 0, -45], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+		multmatrix([[4, 0, 0, 0], [0, 4, 0, 0], [0, 0, 4, 0], [0, 0, 0, 1]]) {
+			group() {
+				group() {
+					render(convexity = 1) {
+						difference() {
+							square(size = [1e+06, 1e+06], center = true);
+							minkowski(convexity = 0) {
+								circle($fn = 0, $fa = 12, $fs = 2, r = 0.3);
+								group() {
+									render(convexity = 1) {
+										difference() {
+											square(size = [1e+06, 1e+06], center = true);
+											minkowski(convexity = 0) {
+												circle($fn = 0, $fa = 12, $fs = 2, r = 0.4);
+												circle($fn = 0, $fa = 12, $fs = 2, r = 4);
+											}
+										}
+									}
+								}
+							}
+						}
+					}
+				}
+			}
+		}
+	}
+	multmatrix([[1, 0, 0, 30], [0, 1, 0, -30], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+		multmatrix([[4, 0, 0, 0], [0, 4, 0, 0], [0, 0, 4, 0], [0, 0, 0, 1]]) {
+			group() {
+				group() {
+					render(convexity = 1) {
+						difference() {
+							square(size = [1e+06, 1e+06], center = true);
+							minkowski(convexity = 0) {
+								circle($fn = 0, $fa = 12, $fs = 2, r = 0.3);
+								group() {
+									render(convexity = 1) {
+										difference() {
+											square(size = [1e+06, 1e+06], center = true);
+											minkowski(convexity = 0) {
+												difference() {
+													circle($fn = 0, $fa = 12, $fs = 2, r = 0.4);
+													circle($fn = 0, $fa = 12, $fs = 2, r = 0.399);
+												}
+												circle($fn = 0, $fa = 12, $fs = 2, r = 4);
+											}
+										}
+									}
+								}
+							}
+						}
+					}
+				}
+			}
+		}
+	}
+	multmatrix([[1, 0, 0, 30], [0, 1, 0, -45], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+		minkowski(convexity = 0) {
+			circle($fn = 0, $fa = 12, $fs = 2, r = 1);
+			circle($fn = 0, $fa = 12, $fs = 2, r = 0);
+		}
+	}
 }