Enforce seam alignment and blend in spiral vase. #2023

lib/Slic3r/GCode.pm

@@ -185,7 +185,11 @@ sub extrude_loop {
     }
     
     # split the loop at the starting point
-    $loop->split_at($last_pos->nearest_point(\@candidates));
+    if ($self->config->spiral_vase) {
+        $loop->split_at($last_pos);
+    } else {
+        $loop->split_at_vertex($last_pos->nearest_point(\@candidates));
+    }
     
     # clip the path to avoid the extruder to get exactly on the first point of the loop;
     # if polyline was shorter than the clipping distance we'd get a null polyline, so

lib/Slic3r/GCode/SpiralVase.pm

@@ -43,7 +43,7 @@ sub process_layer {
         }
     });
     
-    #use XXX; YYY [ $gcode, $layer_height, $z, $total_layer_length ];
+    #use XXX; XXX [ $gcode, $layer_height, $z, $total_layer_length ];
     # remove layer height from initial Z
     $z -= $layer_height;
     
@@ -57,16 +57,19 @@ sub process_layer {
             my $line = $info->{raw};
             $line =~ s/ Z[.0-9]+/ Z$z/;
             $new_gcode .= "$line\n";
-        } elsif ($cmd eq 'G1' && !exists $args->{Z} && $info->{dist_XY}) {
+        } elsif ($cmd eq 'G1' && !exists($args->{Z}) && $info->{dist_XY}) {
             # horizontal move
             my $line = $info->{raw};
             if ($info->{extruding}) {
                 $z += $info->{dist_XY} * $layer_height / $total_layer_length;
                 $line =~ s/^G1 /sprintf 'G1 Z%.3f ', $z/e;
                 $new_gcode .= "$line\n";
-            } else {
-                $new_gcode .= "$line\n";
             }
+            # skip travel moves: the move to first perimeter point will
+            # cause a visible seam when loops are not aligned in XY; by skipping
+            # it we blend the first loop move in the XY plane (although the smoothness
+            # of such blend depend on how long the first segment is; maybe we should
+            # enforce some minimum length?)
         } else {
             $new_gcode .= "$info->{raw}\n";
         }

xs/src/ExtrusionEntity.cpp

@@ -221,7 +221,7 @@ ExtrusionLoop::length() const
 }
 
 void
-ExtrusionLoop::split_at(const Point &point)
+ExtrusionLoop::split_at_vertex(const Point &point)
 {
     for (ExtrusionPaths::iterator path = this->paths.begin(); path != this->paths.end(); ++path) {
         int idx = path->polyline.find_point(point);
@@ -239,7 +239,7 @@ ExtrusionLoop::split_at(const Point &point)
                 {
                     ExtrusionPath p = *path;
                     p.polyline.points.erase(p.polyline.points.begin(), p.polyline.points.begin() + idx);
-                    if (!p.polyline.points.empty()) new_paths.push_back(p);
+                    if (p.polyline.is_valid()) new_paths.push_back(p);
                 }
             
                 // then we add all paths until the end of current path list
@@ -252,7 +252,7 @@ ExtrusionLoop::split_at(const Point &point)
                 {
                     ExtrusionPath p = *path;
                     p.polyline.points.erase(p.polyline.points.begin() + idx + 1, p.polyline.points.end());
-                    if (!p.polyline.points.empty()) new_paths.push_back(p);
+                    if (p.polyline.is_valid()) new_paths.push_back(p);
                 }
                 // we can now override the old path list with the new one and stop looping
                 this->paths = new_paths;
@@ -263,6 +263,39 @@ ExtrusionLoop::split_at(const Point &point)
     CONFESS("Point not found");
 }
 
+void
+ExtrusionLoop::split_at(const Point &point)
+{
+    if (this->paths.empty()) return;
+    
+    // find the closest path and closest point
+    size_t path_idx = 0;
+    Point p = this->paths.front().first_point();
+    double min = point.distance_to(p);
+    for (ExtrusionPaths::const_iterator path = this->paths.begin(); path != this->paths.end(); ++path) {
+        Point p_tmp = point.projection_onto(path->polyline);
+        double dist = point.distance_to(p_tmp);
+        if (dist < min) {
+            p = p_tmp;
+            min = dist;
+            path_idx = path - this->paths.begin();
+        }
+    }
+    
+    // now split path_idx in two parts
+    ExtrusionPath p1 = this->paths[path_idx];
+    ExtrusionPath p2 = p1;
+    this->paths[path_idx].polyline.split_at(p, &p1.polyline, &p2.polyline);
+    
+    // install the two paths
+    this->paths.erase(this->paths.begin() + path_idx);
+    if (p2.polyline.is_valid()) this->paths.insert(this->paths.begin() + path_idx, p2);
+    if (p1.polyline.is_valid()) this->paths.insert(this->paths.begin() + path_idx, p1);
+    
+    // split at the new vertex
+    this->split_at_vertex(p);
+}
+
 void
 ExtrusionLoop::clip_end(double distance, ExtrusionPaths* paths) const
 {

xs/src/ExtrusionEntity.hpp

@@ -93,6 +93,7 @@ class ExtrusionLoop : public ExtrusionEntity
     Point last_point() const;
     void polygon(Polygon* polygon) const;
     double length() const;
+    void split_at_vertex(const Point &point);
     void split_at(const Point &point);
     void clip_end(double distance, ExtrusionPaths* paths) const;
     bool has_overhang_point(const Point &point) const;

xs/src/Point.cpp

@@ -6,6 +6,12 @@
 
 namespace Slic3r {
 
+Point::Point(double x, double y)
+{
+    this->x = lrint(x);
+    this->y = lrint(y);
+}
+
 bool
 Point::operator==(const Point& rhs) const
 {

xs/src/Point.hpp

@@ -24,6 +24,7 @@ class Point
     coord_t x;
     coord_t y;
     explicit Point(coord_t _x = 0, coord_t _y = 0): x(_x), y(_y) {};
+    Point(double x, double y);
     bool operator==(const Point& rhs) const;
     std::string wkt() const;
     void scale(double factor);

xs/src/Polygon.cpp

@@ -56,7 +56,7 @@ Polygon::lines(Lines* lines) const
 }
 
 void
-Polygon::split_at(const Point &point, Polyline* polyline) const
+Polygon::split_at_vertex(const Point &point, Polyline* polyline) const
 {
     // find index of point
     for (Points::const_iterator it = this->points.begin(); it != this->points.end(); ++it) {

xs/src/Polygon.hpp

@@ -21,7 +21,7 @@ class Polygon : public MultiPoint {
     Point last_point() const;
     Lines lines() const;
     void lines(Lines* lines) const;
-    void split_at(const Point &point, Polyline* polyline) const;
+    void split_at_vertex(const Point &point, Polyline* polyline) const;
     void split_at_index(int index, Polyline* polyline) const;
     void split_at_first_point(Polyline* polyline) const;
     void equally_spaced_points(double distance, Points* points) const;

xs/src/Polyline.cpp

@@ -117,6 +117,42 @@ Polyline::simplify(double tolerance)
     this->points = MultiPoint::_douglas_peucker(this->points, tolerance);
 }
 
+void
+Polyline::split_at(const Point &point, Polyline* p1, Polyline* p2) const
+{
+    if (this->points.empty()) return;
+    
+    // find the line to split at
+    size_t line_idx = 0;
+    Point p = this->first_point();
+    double min = point.distance_to(p);
+    Lines lines = this->lines();
+    for (Lines::const_iterator line = lines.begin(); line != lines.end(); ++line) {
+        Point p_tmp = point.projection_onto(*line);
+        if (point.distance_to(p_tmp) < min) {
+	        p = p_tmp;
+	        min = point.distance_to(p);
+	        line_idx = line - lines.begin();
+        }
+    }
+    
+    // create first half
+    p1->points.clear();
+    for (Lines::const_iterator line = lines.begin(); line != lines.begin() + line_idx + 1; ++line) {
+        if (!line->a.coincides_with(p)) p1->points.push_back(line->a);
+    }
+    // we add point instead of p because they might differ because of numerical issues
+    // and caller might want to rely on point belonging to result polylines
+    p1->points.push_back(point);
+    
+    // create second half
+    p2->points.clear();
+    p2->points.push_back(point);
+    for (Lines::const_iterator line = lines.begin() + line_idx; line != lines.end(); ++line) {
+        if (!line->b.coincides_with(p)) p2->points.push_back(line->b);
+    }
+}
+
 
 #ifdef SLIC3RXS
 REGISTER_CLASS(Polyline, "Polyline");

xs/src/Polyline.hpp

@@ -21,6 +21,7 @@ class Polyline : public MultiPoint {
     void extend_start(double distance);
     void equally_spaced_points(double distance, Points* points) const;
     void simplify(double tolerance);
+    void split_at(const Point &point, Polyline* p1, Polyline* p2) const;
     
     #ifdef SLIC3RXS
     void from_SV_check(SV* poly_sv);

xs/t/03_point.t

@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use Slic3r::XS;
-use Test::More tests => 13;
+use Test::More tests => 15;
 
 my $point = Slic3r::Point->new(10, 15);
 is_deeply [ @$point ], [10, 15], 'point roundtrip';
@@ -56,6 +56,12 @@ ok !$point->coincides_with($point2), 'coincides_with';
     
     $point = Slic3r::Point->new(25, 15);
     is_deeply $point->projection_onto_line($line)->pp, [20,10], 'project_onto_line';
+    
+    $point = Slic3r::Point->new(10,10);
+    is_deeply $point->projection_onto_line($line)->pp, [10,10], 'project_onto_line';
+    
+    $point = Slic3r::Point->new(12, 10);
+    is_deeply $point->projection_onto_line($line)->pp, [12,10], 'project_onto_line';
 }
 
 __END__

xs/t/06_polygon.t

@@ -36,7 +36,7 @@ is_deeply [ map $_->pp, @$lines ], [
 
 is_deeply $polygon->split_at_first_point->pp, [ @$square[0,1,2,3,0] ], 'split_at_first_point';
 is_deeply $polygon->split_at_index(2)->pp, [ @$square[2,3,0,1,2] ], 'split_at_index';
-is_deeply $polygon->split_at(Slic3r::Point->new(@{$square->[2]}))->pp, [ @$square[2,3,0,1,2] ], 'split_at';
+is_deeply $polygon->split_at_vertex(Slic3r::Point->new(@{$square->[2]}))->pp, [ @$square[2,3,0,1,2] ], 'split_at';
 is $polygon->area, 100*100, 'area';
 
 ok $polygon->is_counter_clockwise, 'is_counter_clockwise';

xs/t/08_extrusionloop.t

@@ -5,7 +5,7 @@ use warnings;
 
 use List::Util qw(sum);
 use Slic3r::XS;
-use Test::More tests => 30;
+use Test::More tests => 45;
 
 {
     my $square = [
@@ -39,7 +39,7 @@ use Test::More tests => 30;
         is $path->role, Slic3r::ExtrusionPath::EXTR_ROLE_FILL, 'modify role';
     }
 
-    $loop->split_at($square_p->[2]);
+    $loop->split_at_vertex($square_p->[2]);
     is scalar(@$loop), 1, 'splitting a single-path loop results in a single path';
     is scalar(@{$loop->[0]->polyline}), 5, 'path has correct number of points';
     ok $loop->[0]->polyline->[0]->coincides_with($square_p->[2]), 'expected point order';
@@ -65,24 +65,58 @@ use Test::More tests => 30;
             mm3_per_mm  => 1,
         ),
     );
-    is $loop->length, sum($polyline1->length, $polyline2->length), 'length';
+    my $tot_len = sum($polyline1->length, $polyline2->length);
+    is $loop->length, $tot_len, 'length';
     is scalar(@$loop), 2, 'loop contains two paths';
-    $loop->split_at($polyline1->[1]);
-    is $loop->length, sum($polyline1->length, $polyline2->length), 'length after splitting';
-    is scalar(@$loop), 3, 'loop contains three paths after splitting';
-    ok $loop->[0]->polyline->[0]->coincides_with($polyline1->[1]), 'expected starting point';
-    ok $loop->[-1]->polyline->[-1]->coincides_with($polyline1->[1]), 'expected ending point';
-    ok $loop->[0]->polyline->[-1]->coincides_with($loop->[1]->polyline->[0]), 'paths have common point';
-    ok $loop->[1]->polyline->[-1]->coincides_with($loop->[2]->polyline->[0]), 'paths have common point';
-    is $loop->[0]->role, Slic3r::ExtrusionPath::EXTR_ROLE_EXTERNAL_PERIMETER, 'expected order after splitting';
-    is $loop->[1]->role, Slic3r::ExtrusionPath::EXTR_ROLE_OVERHANG_PERIMETER, 'expected order after splitting';
-    is $loop->[2]->role, Slic3r::ExtrusionPath::EXTR_ROLE_EXTERNAL_PERIMETER, 'expected order after splitting';
-    is scalar(@{$loop->[0]->polyline}), 2, 'path has correct number of points';
-    is scalar(@{$loop->[1]->polyline}), 3, 'path has correct number of points';
-    is scalar(@{$loop->[2]->polyline}), 2, 'path has correct number of points';
     
-    my @paths = @{$loop->clip_end(3)};
+    {
-    is sum(map $_->length, @paths), $loop->length - 3, 'returned paths have expected length';
+        # check splitting at intermediate point
+        my $loop2 = $loop->clone;
+        isa_ok $loop2, 'Slic3r::ExtrusionLoop';
+        $loop2->split_at_vertex($polyline1->[1]);
+        is $loop2->length, $tot_len, 'length after splitting is unchanged';
+        is scalar(@$loop2), 3, 'loop contains three paths after splitting';
+        ok $loop2->[0]->polyline->[0]->coincides_with($polyline1->[1]), 'expected starting point';
+        ok $loop2->[-1]->polyline->[-1]->coincides_with($polyline1->[1]), 'expected ending point';
+        ok $loop2->[0]->polyline->[-1]->coincides_with($loop2->[1]->polyline->[0]), 'paths have common point';
+        ok $loop2->[1]->polyline->[-1]->coincides_with($loop2->[2]->polyline->[0]), 'paths have common point';
+        is $loop2->[0]->role, Slic3r::ExtrusionPath::EXTR_ROLE_EXTERNAL_PERIMETER, 'expected order after splitting';
+        is $loop2->[1]->role, Slic3r::ExtrusionPath::EXTR_ROLE_OVERHANG_PERIMETER, 'expected order after splitting';
+        is $loop2->[2]->role, Slic3r::ExtrusionPath::EXTR_ROLE_EXTERNAL_PERIMETER, 'expected order after splitting';
+        is scalar(@{$loop2->[0]->polyline}), 2, 'path has correct number of points';
+        is scalar(@{$loop2->[1]->polyline}), 3, 'path has correct number of points';
+        is scalar(@{$loop2->[2]->polyline}), 2, 'path has correct number of points';
+    
+        my @paths = @{$loop2->clip_end(3)};
+        is sum(map $_->length, @paths), $loop2->length - 3, 'returned paths have expected length';
+    }
+    
+    {
+        # check splitting at endpoint
+        my $loop2 = $loop->clone;
+        $loop2->split_at_vertex($polyline2->[0]);
+        is $loop2->length, $tot_len, 'length after splitting is unchanged';
+        is scalar(@$loop2), 2, 'loop contains two paths after splitting';
+        ok $loop2->[0]->polyline->[0]->coincides_with($polyline2->[0]), 'expected starting point';
+        ok $loop2->[-1]->polyline->[-1]->coincides_with($polyline2->[0]), 'expected ending point';
+        ok $loop2->[0]->polyline->[-1]->coincides_with($loop2->[1]->polyline->[0]), 'paths have common point';
+        ok $loop2->[1]->polyline->[-1]->coincides_with($loop2->[0]->polyline->[0]), 'paths have common point';
+        is $loop2->[0]->role, Slic3r::ExtrusionPath::EXTR_ROLE_OVERHANG_PERIMETER, 'expected order after splitting';
+        is $loop2->[1]->role, Slic3r::ExtrusionPath::EXTR_ROLE_EXTERNAL_PERIMETER, 'expected order after splitting';
+        is scalar(@{$loop2->[0]->polyline}), 3, 'path has correct number of points';
+        is scalar(@{$loop2->[1]->polyline}), 3, 'path has correct number of points';
+    }
+    
+    {
+        my $loop2 = $loop->clone;
+        my $point = Slic3r::Point->new(250,150);
+        $loop2->split_at($point);
+        is $loop2->length, $tot_len, 'length after splitting is unchanged';
+        is scalar(@$loop2), 3, 'loop contains three paths after splitting';
+        my $expected_start_point = Slic3r::Point->new(200,150);
+        ok $loop2->[0]->polyline->[0]->coincides_with($expected_start_point), 'expected starting point';
+        ok $loop2->[-1]->polyline->[-1]->coincides_with($expected_start_point), 'expected ending point';
+    }
 }
 
 __END__

xs/t/09_polyline.t

@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use Slic3r::XS;
-use Test::More tests => 10;
+use Test::More tests => 14;
 
 my $points = [
     [100, 100],
@@ -51,4 +51,16 @@ is_deeply $polyline->pp, [ @$points, @$points ], 'append_polyline';
     is $polyline->length, 100*2 + 50 + 50, 'extend_start';
 }
 
+{
+    my $polyline = Slic3r::Polyline->new(@$points);
+    my $p1 = Slic3r::Polyline->new;
+    my $p2 = Slic3r::Polyline->new;
+    my $point = Slic3r::Point->new(150, 100);
+    $polyline->split_at($point, $p1, $p2);
+    is scalar(@$p1), 2, 'split_at';
+    is scalar(@$p2), 3, 'split_at';
+    ok $p1->last_point->coincides_with($point), 'split_at';
+    ok $p2->first_point->coincides_with($point), 'split_at';
+}
+
 __END__

xs/xsp/ExtrusionLoop.xsp

@@ -20,6 +20,8 @@
     void append(ExtrusionPath* path)
         %code{% THIS->paths.push_back(*path); %};
     double length();
+    void split_at_vertex(Point* point)
+        %code{% THIS->split_at_vertex(*point); %};
     void split_at(Point* point)
         %code{% THIS->split_at(*point); %};
     ExtrusionPaths clip_end(double distance)

xs/xsp/Polygon.xsp

@@ -17,8 +17,8 @@
     void translate(double x, double y);
     void reverse();
     Lines lines();
-    Polyline* split_at(Point* point)
+    Polyline* split_at_vertex(Point* point)
-        %code{% RETVAL = new Polyline(); THIS->split_at(*point, RETVAL); %};
+        %code{% RETVAL = new Polyline(); THIS->split_at_vertex(*point, RETVAL); %};
     Polyline* split_at_index(int index)
         %code{% RETVAL = new Polyline(); THIS->split_at_index(index, RETVAL); %};
     Polyline* split_at_first_point()

xs/xsp/Polyline.xsp

@@ -31,6 +31,8 @@
     void extend_end(double distance);
     void extend_start(double distance);
     void simplify(double tolerance);
+    void split_at(Point* point, Polyline* p1, Polyline* p2)
+        %code{% THIS->split_at(*point, p1, p2); %};
 %{
 
 Polyline*