Merge branch 'issue527' of github.com:openscad/openscad into issue527

Conflicts:
	src/GeometryEvaluator.cc
	tests/CMakeLists.txt

src/GeometryEvaluator.cc

@@ -218,30 +218,62 @@ void GeometryEvaluator::applyResize3D(CGAL_Nef_polyhedron &N,
 	return;
 }
 
+// Helper functions for GeometryEvaluator::applyMinkowski2D()
+namespace {
+	void transform_path(ClipperLib::Path & path, ClipperLib::IntPoint delta) {
+		BOOST_FOREACH(ClipperLib::IntPoint & point, path) {
+			point.X += delta.X;
+			point.Y += delta.Y;
+		}
+	}
+
+	void transform_paths(ClipperLib::Paths & paths, ClipperLib::IntPoint delta) {
+		BOOST_FOREACH(ClipperLib::Path & path, paths) {
+			transform_path(path, delta);
+		}
+	}
+
+	// Add the polygon a translated to an arbitrary point of each separate component of b
+	void fill_minkowski_insides(ClipperLib::Paths const& a,
+								ClipperLib::Paths const& b,
+								std::vector<ClipperLib::Paths> & target) {
+		// (or easier: one arbitrary point on each positive contour)
+		BOOST_FOREACH (ClipperLib::Path const& b_path, b) {
+			if (!b_path.empty() && ClipperLib::Orientation(b_path) == 1) {
+				target.push_back(a);
+				transform_paths(target.back(), b_path[0] /* arbitrary */);
+			}
+		}
+	}
+}
+
 Polygon2d *GeometryEvaluator::applyMinkowski2D(const AbstractNode &node)
 {
 	std::vector<const Polygon2d *> children = collectChildren2D(node);
-	if (children.size() > 0) {
-		bool first = false;
-		ClipperLib::Paths result = ClipperUtils::fromPolygon2d(*children[0]);
-		for (int i=1;i<children.size();i++) {
-			ClipperLib::Path &temp = result[0];
-			const Polygon2d *chgeom = children[i];
-			ClipperLib::Path shape = ClipperUtils::fromOutline2d(chgeom->outlines()[0], false);
-			ClipperLib::MinkowskiSum(temp, shape, result, true);
+	if (!children.empty()) {
+		ClipperLib::Paths lhs = ClipperUtils::fromPolygon2d(*children[0]);
+
+		std::vector<ClipperLib::Paths> minkowski_terms;
+		for (size_t i=1; i<children.size(); i++) {
+			ClipperLib::Paths rhs = ClipperUtils::fromPolygon2d(*children[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_terms.push_back(result);
+				}
+			}
+
+			// Then, fill the central parts
+			fill_minkowski_insides(lhs, rhs, minkowski_terms);
+			fill_minkowski_insides(rhs, lhs, minkowski_terms);
 		}
 
-		// The results may contain holes due to ClipperLib failing to maintain
-		// solidity of minkowski results:
-		// https://sourceforge.net/p/polyclipping/discussion/1148419/thread/8488d4e8/
-		ClipperLib::Paths paths;
-		BOOST_FOREACH(ClipperLib::Path &p, result) {
-			if (ClipperLib::Orientation(p)) std::reverse(p.begin(), p.end());
-			paths.push_back(p);
-		}
-		std::vector<ClipperLib::Paths> pathsvector;
-		pathsvector.push_back(paths);
-		return ClipperUtils::apply(pathsvector, ClipperLib::ctUnion);
+		// Finally, merge the Minkowski terms
+		return ClipperUtils::apply(minkowski_terms, ClipperLib::ctUnion);
 	}
 	return NULL;
 }

testdata/scad/features/minkowski2-hole-tests.scad

@@ -0,0 +1,41 @@
+// HolePoly & Poly
+minkowski() {
+    difference() {
+        square([20,20], center=true);
+        square([10,10], center=true);
+    }
+    circle(r=1, $fn=16);
+}
+
+// Poly & HolePoly
+translate([25,0]) minkowski() {
+    circle(r=1, $fn=16);
+    difference() {
+        square([20,20], center=true);
+        square([10,10], center=true);
+    }
+}
+
+// IslandHolePoly
+translate([0,25]) minkowski() {
+    union() {
+        difference() {
+            square([20,20], center=true);
+            square([10,10], center=true);
+        }
+        square([2,2], center=true);
+    }
+    circle(r=1, $fn=16);
+}
+
+// HolePoly & HolePoly
+translate([25,25]) minkowski() {
+    difference() {
+        square([18,18], center=true);
+        square([12,12], center=true);
+    }
+    difference() {
+        circle(2, $fn=16);
+        circle(1, $fn=16);
+    }
+}

tests/CMakeLists.txt

@@ -627,7 +627,7 @@ target_link_libraries(csgtexttest tests-nocgal ${TESTS-NOCGAL-LIBRARIES})
 #
 add_executable(cgalcachetest cgalcachetest.cc)
 set_target_properties(cgalcachetest PROPERTIES COMPILE_FLAGS "-DENABLE_CGAL ${CGAL_CXX_FLAGS_INIT}")
-target_link_libraries(cgalcachetest tests-cgal ${TESTS-CGAL-LIBRARIES} ${GLEW_LIBRARY} ${COCOA_LIBRARY})
+target_link_libraries(cgalcachetest tests-cgal ${TESTS-CGAL-LIBRARIES} ${CLIPPER_LIBRARY} ${GLEW_LIBRARY} ${COCOA_LIBRARY})
 
 #
 # openscad no-qt

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

@@ -0,0 +1,42 @@
+group() {
+	minkowski(convexity = 0) {
+		difference() {
+			square(size = [20, 20], center = true);
+			square(size = [10, 10], center = true);
+		}
+		circle($fn = 16, $fa = 12, $fs = 2, r = 1);
+	}
+	multmatrix([[1, 0, 0, 25], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+		minkowski(convexity = 0) {
+			circle($fn = 16, $fa = 12, $fs = 2, r = 1);
+			difference() {
+				square(size = [20, 20], center = true);
+				square(size = [10, 10], center = true);
+			}
+		}
+	}
+	multmatrix([[1, 0, 0, 0], [0, 1, 0, 25], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+		minkowski(convexity = 0) {
+			union() {
+				difference() {
+					square(size = [20, 20], center = true);
+					square(size = [10, 10], center = true);
+				}
+				square(size = [2, 2], center = true);
+			}
+			circle($fn = 16, $fa = 12, $fs = 2, r = 1);
+		}
+	}
+	multmatrix([[1, 0, 0, 25], [0, 1, 0, 25], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+		minkowski(convexity = 0) {
+			difference() {
+				square(size = [18, 18], center = true);
+				square(size = [12, 12], center = true);
+			}
+			difference() {
+				circle($fn = 16, $fa = 12, $fs = 2, r = 2);
+				circle($fn = 16, $fa = 12, $fs = 2, r = 1);
+			}
+		}
+	}
+}