Merge branch 'tm_fix_pad'

src/libslic3r/ClipperUtils.cpp

@@ -679,133 +679,73 @@ ClipperLib::PolyTree union_pt(ExPolygons &&subject, bool safety_offset_)
     return _clipper_do<ClipperLib::PolyTree>(ClipperLib::ctUnion, std::move(subject), Polygons(), ClipperLib::pftEvenOdd, safety_offset_);
 }
 
-Polygons
+// Simple spatial ordering of Polynodes
-union_pt_chained(const Polygons &subject, bool safety_offset_)
+ClipperLib::PolyNodes order_nodes(const ClipperLib::PolyNodes &nodes)
-{
-    ClipperLib::PolyTree polytree = union_pt(subject, safety_offset_);
-    
-    Polygons retval;
-    traverse_pt(polytree.Childs, &retval);
-    return retval;
-}
-
-static ClipperLib::PolyNodes order_nodes(const ClipperLib::PolyNodes &nodes)
 {
     // collect ordering points
     Points ordering_points;
     ordering_points.reserve(nodes.size());
+    
     for (const ClipperLib::PolyNode *node : nodes)
-        ordering_points.emplace_back(Point(node->Contour.front().X, node->Contour.front().Y));
+        ordering_points.emplace_back(
+            Point(node->Contour.front().X, node->Contour.front().Y));
 
     // perform the ordering
-    ClipperLib::PolyNodes ordered_nodes = chain_clipper_polynodes(ordering_points, nodes);
+    ClipperLib::PolyNodes ordered_nodes =
+        chain_clipper_polynodes(ordering_points, nodes);
 
     return ordered_nodes;
 }
 
-enum class e_ordering {
+static void traverse_pt_noholes(const ClipperLib::PolyNodes &nodes, Polygons *out)
-    ORDER_POLYNODES,
-    DONT_ORDER_POLYNODES
-};
-
-template<e_ordering o>
-void foreach_node(const ClipperLib::PolyNodes &nodes,
-                  std::function<void(const ClipperLib::PolyNode *)> fn);
-
-template<> void foreach_node<e_ordering::DONT_ORDER_POLYNODES>(
-    const ClipperLib::PolyNodes &                     nodes,
-    std::function<void(const ClipperLib::PolyNode *)> fn)
 {
-    for (auto &n : nodes) fn(n);
+    foreach_node<e_ordering::ON>(nodes, [&out](const ClipperLib::PolyNode *node) 
+    {
+        traverse_pt_noholes(node->Childs, out);
+        out->emplace_back(ClipperPath_to_Slic3rPolygon(node->Contour));
+        if (node->IsHole()) out->back().reverse(); // ccw
+    });
 }
 
-template<> void foreach_node<e_ordering::ORDER_POLYNODES>(
+static void traverse_pt_old(ClipperLib::PolyNodes &nodes, Polygons* retval)
-    const ClipperLib::PolyNodes &                     nodes,
-    std::function<void(const ClipperLib::PolyNode *)> fn)
-{
-    auto ordered_nodes = order_nodes(nodes);
-    for (auto &n : ordered_nodes) fn(n);
-}
-
-template<e_ordering o>
-void _traverse_pt(const ClipperLib::PolyNodes &nodes, Polygons *retval)
 {
     /* use a nearest neighbor search to order these children
        TODO: supply start_near to chained_path() too? */
     
+    // collect ordering points
+    Points ordering_points;
+    ordering_points.reserve(nodes.size());
+    for (ClipperLib::PolyNodes::const_iterator it = nodes.begin(); it != nodes.end(); ++it) {
+        Point p((*it)->Contour.front().X, (*it)->Contour.front().Y);
+        ordering_points.push_back(p);
+    }
+    
+    // perform the ordering
+    ClipperLib::PolyNodes ordered_nodes = chain_clipper_polynodes(ordering_points, nodes);
+    
     // push results recursively
-    foreach_node<o>(nodes, [&retval](const ClipperLib::PolyNode *node) {
+    for (ClipperLib::PolyNodes::iterator it = ordered_nodes.begin(); it != ordered_nodes.end(); ++it) {
         // traverse the next depth
-        _traverse_pt<o>(node->Childs, retval);
+        traverse_pt_old((*it)->Childs, retval);
-        retval->emplace_back(ClipperPath_to_Slic3rPolygon(node->Contour));
+        retval->push_back(ClipperPath_to_Slic3rPolygon((*it)->Contour));
-        if (node->IsHole()) retval->back().reverse();  // ccw
+        if ((*it)->IsHole()) retval->back().reverse();  // ccw
-    });
+    }
 }
 
-template<e_ordering o>
+Polygons union_pt_chained(const Polygons &subject, bool safety_offset_)
-void _traverse_pt(const ClipperLib::PolyNode *tree, ExPolygons *retval)
 {
-    if (!retval || !tree) return;
+    ClipperLib::PolyTree polytree = union_pt(subject, safety_offset_);
     
-    ExPolygons &retv = *retval;
+    Polygons retval;
+    traverse_pt_old(polytree.Childs, &retval);
+    return retval;
     
-    std::function<void(const ClipperLib::PolyNode*, ExPolygon&)> hole_fn;
+// TODO: This needs to be tested:
+//    ClipperLib::PolyTree polytree = union_pt(subject, safety_offset_);
     
-    auto contour_fn = [&retv, &hole_fn](const ClipperLib::PolyNode *pptr) {
+//    Polygons retval;
-        ExPolygon poly;
+//    traverse_pt_noholes(polytree.Childs, &retval);
-        poly.contour.points = ClipperPath_to_Slic3rPolygon(pptr->Contour);
+//    return retval;
-        auto fn = std::bind(hole_fn, std::placeholders::_1, poly);
-        foreach_node<o>(pptr->Childs, fn);
-        retv.push_back(poly);
-    };
-    
-    hole_fn = [&contour_fn](const ClipperLib::PolyNode *pptr, ExPolygon& poly)
-    {   
-        poly.holes.emplace_back();
-        poly.holes.back().points = ClipperPath_to_Slic3rPolygon(pptr->Contour);
-        foreach_node<o>(pptr->Childs, contour_fn);
-    };
-    
-    contour_fn(tree);
-}
-
-template<e_ordering o>
-void _traverse_pt(const ClipperLib::PolyNodes &nodes, ExPolygons *retval)
-{
-    // Here is the actual traverse
-    foreach_node<o>(nodes, [&retval](const ClipperLib::PolyNode *node) {
-        _traverse_pt<o>(node, retval);
-    });
-}
-
-void traverse_pt(const ClipperLib::PolyNode *tree, ExPolygons *retval)
-{
-    _traverse_pt<e_ordering::ORDER_POLYNODES>(tree, retval);
-}
-
-void traverse_pt_unordered(const ClipperLib::PolyNode *tree, ExPolygons *retval)
-{
-    _traverse_pt<e_ordering::DONT_ORDER_POLYNODES>(tree, retval);
-}
-
-void traverse_pt(const ClipperLib::PolyNodes &nodes, Polygons *retval)
-{
-    _traverse_pt<e_ordering::ORDER_POLYNODES>(nodes, retval);
-}
-
-void traverse_pt(const ClipperLib::PolyNodes &nodes, ExPolygons *retval)
-{
-    _traverse_pt<e_ordering::ORDER_POLYNODES>(nodes, retval);
-}
-
-void traverse_pt_unordered(const ClipperLib::PolyNodes &nodes, Polygons *retval)
-{
-    _traverse_pt<e_ordering::DONT_ORDER_POLYNODES>(nodes, retval);
-}
-
-void traverse_pt_unordered(const ClipperLib::PolyNodes &nodes, ExPolygons *retval)
-{
-    _traverse_pt<e_ordering::DONT_ORDER_POLYNODES>(nodes, retval);
 }
 
 Polygons simplify_polygons(const Polygons &subject, bool preserve_collinear)

src/libslic3r/ClipperUtils.hpp

@@ -214,7 +214,6 @@ inline Slic3r::ExPolygons union_ex(const Slic3r::Surfaces &subject, bool safety_
     return _clipper_ex(ClipperLib::ctUnion, to_polygons(subject), Slic3r::Polygons(), safety_offset_);
 }
 
-
 ClipperLib::PolyTree union_pt(const Slic3r::Polygons &subject, bool safety_offset_ = false);
 ClipperLib::PolyTree union_pt(const Slic3r::ExPolygons &subject, bool safety_offset_ = false);
 ClipperLib::PolyTree union_pt(Slic3r::Polygons &&subject, bool safety_offset_ = false);
@@ -222,13 +221,95 @@ ClipperLib::PolyTree union_pt(Slic3r::ExPolygons &&subject, bool safety_offset_
 
 Slic3r::Polygons union_pt_chained(const Slic3r::Polygons &subject, bool safety_offset_ = false);
 
-void traverse_pt(const ClipperLib::PolyNodes &nodes, Slic3r::Polygons *retval);
+ClipperLib::PolyNodes order_nodes(const ClipperLib::PolyNodes &nodes);
-void traverse_pt(const ClipperLib::PolyNodes &nodes, Slic3r::ExPolygons *retval);
+
-void traverse_pt(const ClipperLib::PolyNode  *tree,  Slic3r::ExPolygons *retval);
+// Implementing generalized loop (foreach) over a list of nodes which can be
+// ordered or unordered (performance gain) based on template parameter
+enum class e_ordering {
+    ON,
+    OFF
+};
+
+// Create a template struct, template functions can not be partially specialized
+template<e_ordering o, class Fn> struct _foreach_node {
+    void operator()(const ClipperLib::PolyNodes &nodes, Fn &&fn);
+};
+
+// Specialization with NO ordering
+template<class Fn> struct _foreach_node<e_ordering::OFF, Fn> {
+    void operator()(const ClipperLib::PolyNodes &nodes, Fn &&fn)
+    {
+        for (auto &n : nodes) fn(n);    
+    }
+};
+
+// Specialization with ordering
+template<class Fn> struct _foreach_node<e_ordering::ON, Fn> {
+    void operator()(const ClipperLib::PolyNodes &nodes, Fn &&fn)
+    {
+        auto ordered_nodes = order_nodes(nodes);
+        for (auto &n : nodes) fn(n);    
+    }
+};
+
+// Wrapper function for the foreach_node which can deduce arguments automatically
+template<e_ordering o, class Fn>
+void foreach_node(const ClipperLib::PolyNodes &nodes, Fn &&fn)
+{
+    _foreach_node<o, Fn>()(nodes, std::forward<Fn>(fn));
+}
+
+// Collecting polygons of the tree into a list of Polygons, holes have clockwise
+// orientation.
+template<e_ordering ordering = e_ordering::OFF>
+void traverse_pt(const ClipperLib::PolyNode *tree, Polygons *out)
+{
+    if (!tree) return; // terminates recursion
+    
+    // Push the contour of the current level
+    out->emplace_back(ClipperPath_to_Slic3rPolygon(tree->Contour));
+    
+    // Do the recursion for all the children.
+    traverse_pt<ordering>(tree->Childs, out);
+}
+
+// Collecting polygons of the tree into a list of ExPolygons.
+template<e_ordering ordering = e_ordering::OFF>
+void traverse_pt(const ClipperLib::PolyNode *tree, ExPolygons *out)
+{
+    if (!tree) return;
+    else if(tree->IsHole()) {
+        // Levels of holes are skipped and handled together with the
+        // contour levels.
+        traverse_pt<ordering>(tree->Childs, out);
+        return;
+    }
+    
+    ExPolygon level;
+    level.contour = ClipperPath_to_Slic3rPolygon(tree->Contour);
+    
+    foreach_node<ordering>(tree->Childs, 
+                           [out, &level] (const ClipperLib::PolyNode *node) {
+        
+        // Holes are collected here. 
+        level.holes.emplace_back(ClipperPath_to_Slic3rPolygon(node->Contour));
+        
+        // By doing a recursion, a new level expoly is created with the contour
+        // and holes of the lower level. Doing this for all the childs.
+        traverse_pt<ordering>(node->Childs, out);
+    }); 
+    
+    out->emplace_back(level);
+}
+
+template<e_ordering o = e_ordering::OFF, class ExOrJustPolygons>
+void traverse_pt(const ClipperLib::PolyNodes &nodes, ExOrJustPolygons *retval)
+{
+    foreach_node<o>(nodes, [&retval](const ClipperLib::PolyNode *node) {
+        traverse_pt<o>(node, retval);
+    });
+}
 
-void traverse_pt_unordered(const ClipperLib::PolyNodes &nodes, Slic3r::Polygons *retval);
-void traverse_pt_unordered(const ClipperLib::PolyNodes &nodes, Slic3r::ExPolygons *retval);
-void traverse_pt_unordered(const ClipperLib::PolyNode  *tree,  Slic3r::ExPolygons *retval);
 
 /* OTHER */
 Slic3r::Polygons simplify_polygons(const Slic3r::Polygons &subject, bool preserve_collinear = false);

src/libslic3r/SLA/SLAPad.cpp

@@ -337,13 +337,10 @@ PadSkeleton divide_blueprint(const ExPolygons &bp)
     for (ClipperLib::PolyTree::PolyNode *node : ptree.Childs) {
         ExPolygon poly(ClipperPath_to_Slic3rPolygon(node->Contour));
         for (ClipperLib::PolyTree::PolyNode *child : node->Childs) {
-            if (child->IsHole()) {
             poly.holes.emplace_back(
                 ClipperPath_to_Slic3rPolygon(child->Contour));
 
-                traverse_pt_unordered(child->Childs, &ret.inner);
+            traverse_pt(child->Childs, &ret.inner);
-            }
-            else traverse_pt_unordered(child, &ret.inner);
         }
 
         ret.outer.emplace_back(poly);
@@ -430,9 +427,11 @@ public:
 
         ExPolygons fullpad = diff_ex(fullcvh, model_bp_sticks);
 
-        remove_redundant_parts(fullpad);
-
         PadSkeleton divided = divide_blueprint(fullpad);
+        
+        remove_redundant_parts(divided.outer);
+        remove_redundant_parts(divided.inner);
+
         outer = std::move(divided.outer);
         inner = std::move(divided.inner);
     }

tests/libslic3r/test_clipper_utils.cpp

@@ -1,5 +1,6 @@
 #include <catch2/catch.hpp>
 
+#include <numeric>
 #include <iostream>
 #include <boost/filesystem.hpp>
 
@@ -223,3 +224,78 @@ SCENARIO("Various Clipper operations - t/clipper.t", "[ClipperUtils]") {
         }
     }
 }
+
+template<e_ordering o = e_ordering::OFF, class P, class Tree> 
+double polytree_area(const Tree &tree, std::vector<P> *out)
+{
+    traverse_pt<o>(tree, out);
+    
+    return std::accumulate(out->begin(), out->end(), 0.0,
+                           [](double a, const P &p) { return a + p.area(); });
+}
+
+size_t count_polys(const ExPolygons& expolys)
+{
+    size_t c = 0;
+    for (auto &ep : expolys) c += ep.holes.size() + 1;
+    
+    return c;
+}
+
+TEST_CASE("Traversing Clipper PolyTree", "[ClipperUtils]") {
+    // Create a polygon representing unit box
+    Polygon unitbox;
+    const auto UNIT = coord_t(1. / SCALING_FACTOR);
+    unitbox.points = {{0, 0}, {UNIT, 0}, {UNIT, UNIT}, {0, UNIT}};
+    
+    Polygon box_frame = unitbox;
+    box_frame.scale(20, 10);
+    
+    Polygon hole_left = unitbox;
+    hole_left.scale(8);
+    hole_left.translate(UNIT, UNIT);
+    hole_left.reverse();
+    
+    Polygon hole_right = hole_left;
+    hole_right.translate(UNIT * 10, 0);
+    
+    Polygon inner_left = unitbox;
+    inner_left.scale(4);
+    inner_left.translate(UNIT * 3, UNIT * 3);
+    
+    Polygon inner_right = inner_left;
+    inner_right.translate(UNIT * 10, 0);
+    
+    Polygons reference = union_({box_frame, hole_left, hole_right, inner_left, inner_right});
+    
+    ClipperLib::PolyTree tree = union_pt(reference);
+    double area_sum = box_frame.area() + hole_left.area() +
+                      hole_right.area() + inner_left.area() +
+                      inner_right.area();
+    
+    REQUIRE(area_sum > 0);
+
+    SECTION("Traverse into Polygons WITHOUT spatial ordering") {
+        Polygons output;
+        REQUIRE(area_sum == Approx(polytree_area(tree.GetFirst(), &output)));
+        REQUIRE(output.size() == reference.size());
+    }
+    
+    SECTION("Traverse into ExPolygons WITHOUT spatial ordering") {
+        ExPolygons output;
+        REQUIRE(area_sum == Approx(polytree_area(tree.GetFirst(), &output)));
+        REQUIRE(count_polys(output) == reference.size());
+    }
+    
+    SECTION("Traverse into Polygons WITH spatial ordering") {
+        Polygons output;
+        REQUIRE(area_sum == Approx(polytree_area<e_ordering::ON>(tree.GetFirst(), &output)));
+        REQUIRE(output.size() == reference.size());
+    }
+    
+    SECTION("Traverse into ExPolygons WITH spatial ordering") {
+        ExPolygons output;
+        REQUIRE(area_sum == Approx(polytree_area<e_ordering::ON>(tree.GetFirst(), &output)));
+        REQUIRE(count_polys(output) == reference.size());
+    }
+}