Elephant foot compensation: Refactored / simplified,

fixed an error for variable ExPolygon expansion (not used in production
code yet), fixed asserts when expanding a hole produces a hole in hole,
which is a valid situation.

src/libslic3r/ClipperUtils.cpp

@@ -1,6 +1,7 @@
 #include "ClipperUtils.hpp"
 #include "Geometry.hpp"
 #include "ShortestPath.hpp"
+#include "Utils.hpp"
 
 // #define CLIPPER_UTILS_DEBUG
 
@@ -1167,34 +1168,45 @@ ClipperLib::Path mittered_offset_path_scaled(const Points &contour, const std::v
 	return out;
 }
 
-Polygons variable_offset_inner(const ExPolygon &expoly, const std::vector<std::vector<float>> &deltas, double miter_limit)
+static void variable_offset_inner_raw(const ExPolygon &expoly, const std::vector<std::vector<float>> &deltas, double miter_limit, ClipperLib::Paths &contours, ClipperLib::Paths &holes)
 {
 #ifndef NDEBUG
-	// Verify that the deltas are all non positive.
+    // Verify that the deltas are all non positive.
-	for (const std::vector<float> &ds : deltas)
+    for (const std::vector<float> &ds : deltas)
-		for (float delta : ds)
+        for (float delta : ds)
-			assert(delta <= 0.);
+            assert(delta <= 0.);
-	assert(expoly.holes.size() + 1 == deltas.size());
+    assert(expoly.holes.size() + 1 == deltas.size());
+    assert(ClipperLib::Area(expoly.contour.points) > 0.);
+    for (auto &h : expoly.holes)
+        assert(ClipperLib::Area(h.points) < 0.);
 #endif /* NDEBUG */
 
-	// 1) Offset the outer contour.
+    // 1) Offset the outer contour.
-	ClipperLib::Paths contours = fix_after_inner_offset(mittered_offset_path_scaled(expoly.contour.points, deltas.front(), miter_limit), ClipperLib::pftNegative, true);
+    contours = fix_after_inner_offset(mittered_offset_path_scaled(expoly.contour.points, deltas.front(), miter_limit), ClipperLib::pftNegative, true);
-#ifndef NDEBUG	
+#ifndef NDEBUG
-	for (auto &c : contours)
+    // Shrinking a contour may split it into pieces, but never create a new hole inside the contour.
-		assert(ClipperLib::Area(c) > 0.);
+    for (auto &c : contours)
+        assert(ClipperLib::Area(c) > 0.);
 #endif /* NDEBUG */
 
-	// 2) Offset the holes one by one, collect the results.
+    // 2) Offset the holes one by one, collect the results.
-	ClipperLib::Paths holes;
+    holes.reserve(expoly.holes.size());
-	holes.reserve(expoly.holes.size());
+    for (const Polygon &hole : expoly.holes)
-	for (const Polygon& hole : expoly.holes)
+        append(holes, fix_after_outer_offset(mittered_offset_path_scaled(hole.points, deltas[1 + &hole - expoly.holes.data()], miter_limit), ClipperLib::pftNegative, false));
-		append(holes, fix_after_outer_offset(mittered_offset_path_scaled(hole.points, deltas[1 + &hole - expoly.holes.data()], miter_limit), ClipperLib::pftNegative, false));
+#ifndef NDEBUG
-#ifndef NDEBUG	
+    // Offsetting a hole curve of a C shape may close the C into a ring with a new hole inside, thus creating a hole inside a hole shape, thus a hole will be created with negative area
-	for (auto &c : holes)
+    // and the following test will fail.
-		assert(ClipperLib::Area(c) > 0.);
+//    for (auto &c : holes)
+//        assert(ClipperLib::Area(c) > 0.);
 #endif /* NDEBUG */
+}
 
-	// 3) Subtract holes from the contours.
+Polygons variable_offset_inner(const ExPolygon &expoly, const std::vector<std::vector<float>> &deltas, double miter_limit)
+{
+    ClipperLib::Paths contours, holes;
+    variable_offset_inner_raw(expoly, deltas, miter_limit, contours, holes);
+
+	// Subtract holes from the contours.
 	ClipperLib::Paths output;
 	if (holes.empty())
 		output = std::move(contours);
@@ -1202,6 +1214,8 @@ Polygons variable_offset_inner(const ExPolygon &expoly, const std::vector<std::v
 		ClipperLib::Clipper clipper;
 		clipper.Clear();
 		clipper.AddPaths(contours, ClipperLib::ptSubject, true);
+        // Holes may contain holes in holes produced by expanding a C hole shape.
+        // The situation is processed correctly by Clipper diff operation.
 		clipper.AddPaths(holes, ClipperLib::ptClip, true);
 		clipper.Execute(ClipperLib::ctDifference, output, ClipperLib::pftNonZero, ClipperLib::pftNonZero);
 	}
@@ -1209,129 +1223,120 @@ Polygons variable_offset_inner(const ExPolygon &expoly, const std::vector<std::v
 	return to_polygons(std::move(output));
 }
 
+ExPolygons variable_offset_inner_ex(const ExPolygon &expoly, const std::vector<std::vector<float>> &deltas, double miter_limit)
+{
+    ClipperLib::Paths contours, holes;
+    variable_offset_inner_raw(expoly, deltas, miter_limit, contours, holes);
+
+    // Subtract holes from the contours.
+    ExPolygons output;
+    if (holes.empty()) {
+        output.reserve(contours.size());
+        // Shrinking a CCW contour may only produce more CCW contours, but never new holes.
+        for (ClipperLib::Path &path : contours) 
+            output.emplace_back(std::move(path));
+    } else {
+        ClipperLib::Clipper clipper;
+        clipper.AddPaths(contours, ClipperLib::ptSubject, true);
+        // Holes may contain holes in holes produced by expanding a C hole shape.
+        // The situation is processed correctly by Clipper diff operation, producing concentric expolygons.
+        clipper.AddPaths(holes, ClipperLib::ptClip, true);
+        ClipperLib::PolyTree polytree;
+        clipper.Execute(ClipperLib::ctDifference, polytree, ClipperLib::pftNonZero, ClipperLib::pftNonZero);
+        output = PolyTreeToExPolygons(std::move(polytree));
+    }
+
+    return output;
+}
+
+static void variable_offset_outer_raw(const ExPolygon &expoly, const std::vector<std::vector<float>> &deltas, double miter_limit, ClipperLib::Paths &contours, ClipperLib::Paths &holes)
+{
+#ifndef NDEBUG
+    // Verify that the deltas are all non positive.
+    for (const std::vector<float> &ds : deltas)
+        for (float delta : ds)
+            assert(delta >= 0.);
+    assert(expoly.holes.size() + 1 == deltas.size());
+    assert(ClipperLib::Area(expoly.contour.points) > 0.);
+    for (auto &h : expoly.holes)
+        assert(ClipperLib::Area(h.points) < 0.);
+#endif /* NDEBUG */
+
+    // 1) Offset the outer contour.
+    contours = fix_after_outer_offset(mittered_offset_path_scaled(expoly.contour.points, deltas.front(), miter_limit), ClipperLib::pftPositive, false);
+    // Inflating a contour must not remove it.
+    assert(contours.size() >= 1);
+#ifndef NDEBUG
+    // Offsetting a positive curve of a C shape may close the C into a ring with hole shape, thus a hole will be created with negative area
+    // and the following test will fail.
+//  for (auto &c : contours)
+//      assert(ClipperLib::Area(c) > 0.);
+#endif /* NDEBUG */
+
+    // 2) Offset the holes one by one, collect the results.
+    holes.reserve(expoly.holes.size());
+    for (const Polygon& hole : expoly.holes)
+        append(holes, fix_after_inner_offset(mittered_offset_path_scaled(hole.points, deltas[1 + &hole - expoly.holes.data()], miter_limit), ClipperLib::pftPositive, true));
+#ifndef NDEBUG
+    // Shrinking a hole may split it into pieces, but never create a new hole inside a hole.
+    for (auto &c : holes)
+        assert(ClipperLib::Area(c) > 0.);
+#endif /* NDEBUG */
+}
+
 Polygons variable_offset_outer(const ExPolygon &expoly, const std::vector<std::vector<float>> &deltas, double miter_limit)
 {
-#ifndef NDEBUG
+    ClipperLib::Paths contours, holes;
-	// Verify that the deltas are all non positive.
+    variable_offset_outer_raw(expoly, deltas, miter_limit, contours, holes);
-for (const std::vector<float>& ds : deltas)
-		for (float delta : ds)
-			assert(delta >= 0.);
-	assert(expoly.holes.size() + 1 == deltas.size());
-#endif /* NDEBUG */
 
-	// 1) Offset the outer contour.
+    // Subtract holes from the contours.
-	ClipperLib::Paths contours = fix_after_outer_offset(mittered_offset_path_scaled(expoly.contour.points, deltas.front(), miter_limit), ClipperLib::pftPositive, false);
+    ClipperLib::Paths output;
-#ifndef NDEBUG
+    if (holes.empty())
-	for (auto &c : contours)
+        output = std::move(contours);
-		assert(ClipperLib::Area(c) > 0.);
+    else {
-#endif /* NDEBUG */
+        //FIXME the difference is not needed as the holes may never intersect with other holes.
+        ClipperLib::Clipper clipper;
+        clipper.Clear();
+        clipper.AddPaths(contours, ClipperLib::ptSubject, true);
+        clipper.AddPaths(holes, ClipperLib::ptClip, true);
+        clipper.Execute(ClipperLib::ctDifference, output, ClipperLib::pftNonZero, ClipperLib::pftNonZero);
+    }
 
-	// 2) Offset the holes one by one, collect the results.
+    return to_polygons(std::move(output));
-	ClipperLib::Paths holes;
-	holes.reserve(expoly.holes.size());
-	for (const Polygon& hole : expoly.holes)
-		append(holes, fix_after_inner_offset(mittered_offset_path_scaled(hole.points, deltas[1 + &hole - expoly.holes.data()], miter_limit), ClipperLib::pftPositive, true));
-#ifndef NDEBUG
-	for (auto &c : holes)
-		assert(ClipperLib::Area(c) > 0.);
-#endif /* NDEBUG */
-
-	// 3) Subtract holes from the contours.
-	ClipperLib::Paths output;
-	if (holes.empty())
-		output = std::move(contours);
-	else {
-		ClipperLib::Clipper clipper;
-		clipper.Clear();
-		clipper.AddPaths(contours, ClipperLib::ptSubject, true);
-		clipper.AddPaths(holes, ClipperLib::ptClip, true);
-		clipper.Execute(ClipperLib::ctDifference, output, ClipperLib::pftNonZero, ClipperLib::pftNonZero);
-	}
-
-	return to_polygons(std::move(output));
 }
 
 ExPolygons variable_offset_outer_ex(const ExPolygon &expoly, const std::vector<std::vector<float>> &deltas, double miter_limit)
 {
-#ifndef NDEBUG
+    ClipperLib::Paths contours, holes;
-	// Verify that the deltas are all non positive.
+    variable_offset_outer_raw(expoly, deltas, miter_limit, contours, holes);
-for (const std::vector<float>& ds : deltas)
-		for (float delta : ds)
-			assert(delta >= 0.);
-	assert(expoly.holes.size() + 1 == deltas.size());
-#endif /* NDEBUG */
 
-	// 1) Offset the outer contour.
+	// Subtract holes from the contours.
-	ClipperLib::Paths contours = fix_after_outer_offset(mittered_offset_path_scaled(expoly.contour.points, deltas.front(), miter_limit), ClipperLib::pftPositive, false);
-#ifndef NDEBUG
-	for (auto &c : contours)
-		assert(ClipperLib::Area(c) > 0.);
-#endif /* NDEBUG */
-
-	// 2) Offset the holes one by one, collect the results.
-	ClipperLib::Paths holes;
-	holes.reserve(expoly.holes.size());
-	for (const Polygon& hole : expoly.holes)
-		append(holes, fix_after_inner_offset(mittered_offset_path_scaled(hole.points, deltas[1 + &hole - expoly.holes.data()], miter_limit), ClipperLib::pftPositive, true));
-#ifndef NDEBUG
-	for (auto &c : holes)
-		assert(ClipperLib::Area(c) > 0.);
-#endif /* NDEBUG */
-
-	// 3) Subtract holes from the contours.
 	ExPolygons output;
 	if (holes.empty()) {
-		output.reserve(contours.size());
+		output.reserve(1);
-		for (ClipperLib::Path &path : contours) 
+        if (contours.size() > 1) {
-			output.emplace_back(std::move(path));
+            // One expolygon with holes created by closing a C shape. Which is which?
-	} else {
+            output.push_back({});
-		ClipperLib::Clipper clipper;
+            ExPolygon &out = output.back();
-		clipper.AddPaths(contours, ClipperLib::ptSubject, true);
+            out.holes.reserve(contours.size() - 1);
-		clipper.AddPaths(holes, ClipperLib::ptClip, true);
+    		for (ClipperLib::Path &path : contours) {
-	    ClipperLib::PolyTree polytree;
+                if (ClipperLib::Area(path) > 0) {
-		clipper.Execute(ClipperLib::ctDifference, polytree, ClipperLib::pftNonZero, ClipperLib::pftNonZero);
+                    // Only one contour with positive area is expected to be created by an outer offset of an ExPolygon.
-	    output = PolyTreeToExPolygons(std::move(polytree));
+                    assert(out.contour.empty());
-	}
+                    out.contour.points = std::move(path);
-
+                } else
-	return output;
+                    out.holes.push_back(Polygon{ std::move(path) });
-}
+            }
-
+        } else {
-
+            // Single contour must be CCW.
-ExPolygons variable_offset_inner_ex(const ExPolygon &expoly, const std::vector<std::vector<float>> &deltas, double miter_limit)
+            assert(contours.size() == 1);
-{
+            assert(ClipperLib::Area(contours.front()) > 0);
-#ifndef NDEBUG
+            output.push_back(ExPolygon{ std::move(contours.front()) });
-	// Verify that the deltas are all non positive.
+        }
-	for (const std::vector<float>& ds : deltas)
-		for (float delta : ds)
-			assert(delta <= 0.);
-	assert(expoly.holes.size() + 1 == deltas.size());
-#endif /* NDEBUG */
-
-	// 1) Offset the outer contour.
-	ClipperLib::Paths contours = fix_after_inner_offset(mittered_offset_path_scaled(expoly.contour.points, deltas.front(), miter_limit), ClipperLib::pftNegative, true);
-#ifndef NDEBUG
-	for (auto &c : contours)
-		assert(ClipperLib::Area(c) > 0.);
-#endif /* NDEBUG */
-
-	// 2) Offset the holes one by one, collect the results.
-	ClipperLib::Paths holes;
-	holes.reserve(expoly.holes.size());
-	for (const Polygon& hole : expoly.holes)
-		append(holes, fix_after_outer_offset(mittered_offset_path_scaled(hole.points, deltas[1 + &hole - expoly.holes.data()], miter_limit), ClipperLib::pftNegative, false));
-#ifndef NDEBUG
-	for (auto &c : holes)
-		assert(ClipperLib::Area(c) > 0.);
-#endif /* NDEBUG */
-
-	// 3) Subtract holes from the contours.
-	ExPolygons output;
-	if (holes.empty()) {
-		output.reserve(contours.size());
-		for (ClipperLib::Path &path : contours) 
-			output.emplace_back(std::move(path));
 	} else {
+        //FIXME the difference is not needed as the holes may never intersect with other holes.
 		ClipperLib::Clipper clipper;
+        // Contours may have holes if they were created by closing a C shape.
 		clipper.AddPaths(contours, ClipperLib::ptSubject, true);
 		clipper.AddPaths(holes, ClipperLib::ptClip, true);
 	    ClipperLib::PolyTree polytree;
@@ -1339,6 +1344,7 @@ ExPolygons variable_offset_inner_ex(const ExPolygon &expoly, const std::vector<s
 	    output = PolyTreeToExPolygons(std::move(polytree));
 	}
 
+    assert(output.size() == 1);
 	return output;
 }
 

src/libslic3r/ElephantFootCompensation.cpp

@@ -597,7 +597,8 @@ ExPolygon elephant_foot_compensation(const ExPolygon &input_expoly, double min_c
 		}
 
 		ExPolygons out_vec = variable_offset_inner_ex(resampled, deltas, 2.);
-		if (out_vec.size() == 1)
+		if (out_vec.size() == 1 && out_vec.front().holes.size() == resampled.holes.size())
+			// No contour of the original compensated expolygon was lost.
 			out = std::move(out_vec.front());
 		else {
 			// Something went wrong, don't compensate.
@@ -610,6 +611,7 @@ ExPolygon elephant_foot_compensation(const ExPolygon &input_expoly, double min_c
 					  { { out_vec },		{ "gray", "black", "blue", coord_t(scale_(0.02)), 0.5f, "black", coord_t(scale_(0.05)) } } });
 			}
 #endif /* TESTS_EXPORT_SVGS */
+			// It may be that the source expolygons contained non-manifold vertices, for which the variable offset may not produce the same number of contours or holes.
 			assert(out_vec.size() == 1);
 		}
 	}