Bugfix: concave starting points were not correctly detected for slice holes. Includes regression test. #2857

lib/Slic3r/GCode.pm

@@ -129,9 +129,16 @@ sub extrude_loop {
         $loop->split_at($last_pos);
     } elsif ($self->config->seam_position eq 'nearest' || $self->config->seam_position eq 'aligned') {
         # simplify polygon in order to skip false positives in concave/convex detection
+        # ($loop is always ccw as $polygon->simplify only works on ccw polygons)
         my $polygon = $loop->polygon;
         my @simplified = @{$polygon->simplify(scale $self->config->get_at('nozzle_diameter', $self->writer->extruder->id)/2)};
         
+        # restore original winding order so that concave and convex detection always happens
+        # on the right/outer side of the polygon
+        if ($was_clockwise) {
+            $_->reverse for @simplified;
+        }
+        
         # concave vertices have priority
         my @candidates = map @{$_->concave_points(PI*4/3)}, @simplified;
         

lib/Slic3r/Test.pm

@@ -34,6 +34,13 @@ sub mesh {
         $facets = [
             [0,1,2],[2,1,3],[1,0,4],[5,1,4],[6,7,4],[8,2,9],[0,2,8],[10,8,9],[0,8,6],[0,6,4],[4,7,9],[7,10,9],[2,3,9],[9,3,11],[12,1,5],[13,3,12],[14,12,5],[3,1,12],[11,3,13],[11,15,5],[11,13,15],[15,14,5],[5,4,9],[11,5,9],[8,13,12],[6,8,12],[10,15,13],[8,10,13],[15,10,14],[14,10,7],[14,7,12],[12,7,6]
         ],
+    } elsif ($name eq 'cube_with_concave_hole') {
+        $vertices = [
+            [-10,-10,-5],[-10,-10,5],[-10,10,-5],[-10,10,5],[10,-10,-5],[10,-10,5],[-5,-5,-5],[5,-5,-5],[5,5,-5],[5,10,-5],[-5,5,-5],[3.06161699911402e-16,5,-5],[5,0,-5],[0,0,-5],[10,5,-5],[5,10,5],[-5,-5,5],[5,0,5],[5,-5,5],[-5,5,5],[10,5,5],[5,5,5],[3.06161699911402e-16,5,5],[0,0,5]
+        ];
+        $facets = [
+            [0,1,2],[2,1,3],[1,0,4],[5,1,4],[6,7,4],[8,2,9],[10,2,11],[11,12,13],[0,2,10],[0,10,6],[0,6,4],[11,2,8],[4,7,12],[4,12,8],[12,11,8],[14,4,8],[2,3,9],[9,3,15],[16,1,5],[17,18,5],[19,3,16],[20,21,5],[18,16,5],[3,1,16],[22,3,19],[21,3,22],[21,17,5],[21,22,17],[21,15,3],[23,17,22],[5,4,14],[20,5,14],[20,14,21],[21,14,8],[9,15,21],[8,9,21],[10,19,16],[6,10,16],[11,22,19],[10,11,19],[13,23,11],[11,23,22],[23,13,12],[17,23,12],[17,12,18],[18,12,7],[18,7,16],[16,7,6]
+        ],
     } elsif ($name eq 'V') {
         $vertices = [
             [-14,0,20],[-14,15,20],[0,0,0],[0,15,0],[-4,0,20],[-4,15,20],[5,0,7.14286],[10,0,0],[24,0,20],[14,0,20],[10,15,0],[5,15,7.14286],[14,15,20],[24,15,20]

t/geometry.t

@@ -2,7 +2,7 @@ use Test::More;
 use strict;
 use warnings;
 
-plan tests => 38;
+plan tests => 42;
 
 BEGIN {
     use FindBin;
@@ -190,6 +190,21 @@ my $polygons = [
 
 #==========================================================
 
+{
+    my $square = Slic3r::Polygon->new_scale(
+        [100,100],
+        [200,100],
+        [200,200],
+        [100,200],
+    );
+    is scalar(@{$square->concave_points(PI*4/3)}), 0, 'no concave vertices detected in ccw square';
+    is scalar(@{$square->convex_points(PI*2/3)}), 4, 'four convex vertices detected in ccw square';
+    
+    $square->make_clockwise;
+    is scalar(@{$square->concave_points(PI*4/3)}), 4, 'fuor concave vertices detected in cw square';
+    is scalar(@{$square->convex_points(PI*2/3)}), 0, 'no convex vertices detected in cw square';
+}
+
 {
     my $square = Slic3r::Polygon->new_scale(
         [150,100],

t/perimeters.t

@@ -1,4 +1,4 @@
-use Test::More tests => 29;
+use Test::More tests => 33;
 use strict;
 use warnings;
 
@@ -47,14 +47,14 @@ use Slic3r::Test;
         ok !$has_cw_loops, 'all perimeters extruded ccw';
     }
     
-    {
+    foreach my $model (qw(cube_with_hole cube_with_concave_hole)) {
         $config->set('external_perimeter_speed', 68);
         my $print = Slic3r::Test::init_print(
-            'cube_with_hole',
+            $model,
             config      => $config,
             duplicate   => 2,  # we test two copies to make sure ExtrusionLoop objects are not modified in-place (the second object would not detect cw loops and thus would calculate wrong inwards moves)
         );
-        my $has_cw_loops = my $has_outwards_move = 0;
+        my $has_cw_loops = my $has_outwards_move = my $starts_on_convex_point = 0;
         my $cur_loop;
         my %external_loops = ();  # print_z => count of external loops
         Slic3r::GCode::Reader->new->parse(Slic3r::Test::gcode($print), sub {
@@ -74,10 +74,22 @@ use Slic3r::Test;
                             if defined($external_loops{$self->Z}) && $external_loops{$self->Z} == 2;
                         
                         $external_loops{$self->Z}++;
-                        my $loop_contains_point = Slic3r::Polygon->new_scale(@$cur_loop)->contains_point($move_dest);
+                        my $is_contour  = $external_loops{$self->Z} == 2;
+                        my $is_hole     = $external_loops{$self->Z} == 1;
+                        
+                        my $loop = Slic3r::Polygon->new_scale(@$cur_loop);
+                        my $loop_contains_point = $loop->contains_point($move_dest);
                         $has_outwards_move = 1
-                            if (!$loop_contains_point && $external_loops{$self->Z} == 2)  # contour should include destination
+                            if (!$loop_contains_point && $is_contour)  # contour should include destination
-                             || ($loop_contains_point && $external_loops{$self->Z} == 1); # hole should not
+                             || ($loop_contains_point && $is_hole);    # hole should not
+                        
+                        if ($model eq 'cube_with_concave_hole') {
+                            # check that loop starts at a concave vertex
+                            my $ccw_angle = $loop->first_point->ccw_angle(@$loop[-2,1]);
+                            my $convex = ($ccw_angle > PI);  # whether the angle on the *right* side is convex
+                            $starts_on_convex_point = 1
+                                if ($convex && $is_contour) || (!$convex && $is_hole);
+                        }
                     }
                     $cur_loop = undef;
                 }
@@ -85,6 +97,7 @@ use Slic3r::Test;
         });
         ok !$has_cw_loops, 'all perimeters extruded ccw';
         ok !$has_outwards_move, 'move inwards after completing external loop';
+        ok !$starts_on_convex_point, 'loops start on concave point if any';
     }
     
     {

xs/src/libslic3r/Polygon.cpp

@@ -218,7 +218,8 @@ Polygon::wkt() const
     return wkt.str();
 }
 
-// find all concave vertices (i.e. having an internal angle greater than the supplied angle) */
+// find all concave vertices (i.e. having an internal angle greater than the supplied angle)
+// (external = right side, thus we consider ccw orientation)
 Points
 Polygon::concave_points(double angle) const
 {
@@ -241,7 +242,8 @@ Polygon::concave_points(double angle) const
     return points;
 }
 
-// find all convex vertices (i.e. having an internal angle smaller than the supplied angle) */
+// find all convex vertices (i.e. having an internal angle smaller than the supplied angle)
+// (external = right side, thus we consider ccw orientation)
 Points
 Polygon::convex_points(double angle) const
 {