Bugfix: thin walls forming a closed loop had overlapping segments at their endpoints. #1948 #1875

t/thin.t

@@ -1,4 +1,4 @@
-use Test::More tests => 11;
+use Test::More tests => 13;
 use strict;
 use warnings;
 
@@ -62,7 +62,9 @@ if (0) {
     );
     my $expolygon = Slic3r::ExPolygon->new($square, $hole_in_square);
     my $res = $expolygon->medial_axis(scale 1, scale 0.5);
-    is scalar(@$res), 1, 'medial axis of a square shape is a single closed loop';
+    is scalar(@$res), 1, 'medial axis of a square shape is a single path';
+    isa_ok $res->[0], 'Slic3r::Polyline', 'medial axis result is a polyline';
+    ok $res->[0]->first_point->coincides_with($res->[0]->last_point), 'polyline forms a closed loop';
     ok $res->[0]->length > $hole_in_square->length && $res->[0]->length < $square->length,
         'medial axis loop has reasonable length';
 }

xs/src/ClipperUtils.cpp

@@ -339,36 +339,25 @@ void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polygons &subject,
 void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polylines &subject, 
     const Slic3r::Polygons &clip, Slic3r::Polylines &retval, bool safety_offset_)
 {
+    
+    /* Clipper will remove a polyline segment if first point coincides with last one.
+       Until that bug is not fixed upstream, we move one of those points slightly. */
+    Slic3r::Polylines polylines = subject;  // temp copy to avoid dropping the const qualifier
+    for (Slic3r::Polylines::iterator polyline = polylines.begin(); polyline != polylines.end(); ++polyline)
+        polyline->points.front().translate(1, 0);
+    
     // perform operation
     ClipperLib::PolyTree polytree;
-    _clipper_do(clipType, subject, clip, polytree, ClipperLib::pftNonZero, safety_offset_);
+    _clipper_do(clipType, polylines, clip, polytree, ClipperLib::pftNonZero, safety_offset_);
     
     // convert into Polylines
     ClipperLib::Paths output;
     ClipperLib::PolyTreeToPaths(polytree, output);
     ClipperPaths_to_Slic3rMultiPoints(output, retval);
-}
-
-void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polygons &subject, 
-    const Slic3r::Polygons &clip, Slic3r::Polylines &retval, bool safety_offset_)
-{
-    // transform input polygons into polylines
-    Slic3r::Polylines polylines;
-    polylines.reserve(subject.size());
-    for (Slic3r::Polygons::const_iterator polygon = subject.begin(); polygon != subject.end(); ++polygon)
-        polylines.push_back(*polygon);  // implicit call to split_at_first_point()
-    
-    /* Clipper will remove a polyline segment if first point coincides with last one.
-       Until that bug is not fixed upstream, we move one of those points slightly. */
-    for (Slic3r::Polylines::iterator polyline = polylines.begin(); polyline != polylines.end(); ++polyline)
-        polyline->points.front().translate(1, 0);
-    
-    // perform clipping
-    _clipper(clipType, polylines, clip, retval, safety_offset_);
     
     // compensate for the above hack
     for (Slic3r::Polylines::iterator polyline = retval.begin(); polyline != retval.end(); ++polyline) {
-        for (Slic3r::Polylines::iterator subj_polyline = polylines.begin(); subj_polyline != polylines.end(); ++subj_polyline) {
+        for (Slic3r::Polylines::const_iterator subj_polyline = polylines.begin(); subj_polyline != polylines.end(); ++subj_polyline) {
             // if first point of clipped line coincides with first point of subject line, compensate for hack
             if (polyline->points.front().coincides_with(subj_polyline->points.front())) {
                 polyline->points.front().translate(-1, 0);
@@ -381,6 +370,19 @@ void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polygons &subject,
             }
         }
     }
+}
+
+void _clipper(ClipperLib::ClipType clipType, const Slic3r::Polygons &subject, 
+    const Slic3r::Polygons &clip, Slic3r::Polylines &retval, bool safety_offset_)
+{
+    // transform input polygons into polylines
+    Slic3r::Polylines polylines;
+    polylines.reserve(subject.size());
+    for (Slic3r::Polygons::const_iterator polygon = subject.begin(); polygon != subject.end(); ++polygon)
+        polylines.push_back(*polygon);  // implicit call to split_at_first_point()
+    
+    // perform clipping
+    _clipper(clipType, polylines, clip, retval, safety_offset_);
     
     /* If the split_at_first_point() call above happens to split the polygon inside the clipping area
        we would get two consecutive polylines instead of a single one, so we go through them in order

xs/src/ExPolygon.cpp

@@ -155,7 +155,9 @@ ExPolygon::medial_axis(double max_width, double min_width, Polylines* polylines)
     ma.build(polylines);
     
     // extend initial and final segments of each polyline (they will be clipped)
+    // unless they represent closed loops
     for (Polylines::iterator polyline = polylines->begin(); polyline != polylines->end(); ++polyline) {
+        if (polyline->points.front().coincides_with(polyline->points.back())) continue;
         polyline->extend_start(max_width);
         polyline->extend_end(max_width);
     }

xs/src/Geometry.cpp

@@ -123,6 +123,21 @@ MedialAxis::build(Polylines* polylines)
     
     construct_voronoi(this->lines.begin(), this->lines.end(), &this->vd);
     
+    /*
+    // DEBUG: dump all Voronoi edges
+    {
+        for (VD::const_edge_iterator edge = this->vd.edges().begin(); edge != this->vd.edges().end(); ++edge) {
+            if (edge->is_infinite()) continue;
+            
+            Polyline polyline;
+            polyline.points.push_back(Point( edge->vertex0()->x(), edge->vertex0()->y() ));
+            polyline.points.push_back(Point( edge->vertex1()->x(), edge->vertex1()->y() ));
+            polylines->push_back(polyline);
+        }
+        return;
+    }
+    */
+    
     // collect valid edges (i.e. prune those not belonging to MAT)
     // note: this keeps twins, so it contains twice the number of the valid edges
     this->edges.clear();