Include thin walls in general top-level perimeter sorting to get more efficient paths

lib/Slic3r/Layer/Region.pm

@@ -149,6 +149,30 @@ sub make_perimeters {
         );
     }
     
+    # process thin walls by collapsing slices to single passes
+    my @thin_wall_polylines = ();
+    if (@thin_walls) {
+        # the following offset2 ensures almost nothing in @thin_walls is narrower than $min_width
+        # (actually, something larger than that still may exist due to mitering or other causes)
+        my $min_width = $pwidth / 4;
+        @thin_walls = @{offset2_ex([ map @$_, @thin_walls ], -$min_width/2, +$min_width/2)};
+        
+        # the maximum thickness of our thin wall area is equal to the minimum thickness of a single loop
+        @thin_wall_polylines = map @{$_->medial_axis($pwidth + $pspacing, $min_width)}, @thin_walls;
+        Slic3r::debugf "  %d thin walls detected\n", scalar(@thin_wall_polylines) if $Slic3r::debug;
+        
+        if (0) {
+            require "Slic3r/SVG.pm";
+            Slic3r::SVG::output(
+                "medial_axis.svg",
+                no_arrows => 1,
+                expolygons      => \@thin_walls,
+                green_polylines => [ map $_->polygon->split_at_first_point, @{$self->perimeters} ],
+                red_polylines   => \@thin_wall_polylines,
+            );
+        }
+    }
+    
     # find nesting hierarchies separately for contours and holes
     my $contours_pt = union_pt(\@contours);
     my $holes_pt    = union_pt(\@holes);
@@ -160,36 +184,20 @@ sub make_perimeters {
     $traverse = sub {
         my ($polynodes, $depth, $is_contour) = @_;
         
-        # use a nearest neighbor search to order these children
+        # convert all polynodes to ExtrusionLoop objects
-        # TODO: supply second argument to chained_path() too?
+        my $collection = Slic3r::ExtrusionPath::Collection->new;
-        my @ordering_points = map { ($_->{outer} // $_->{hole})->first_point } @$polynodes;
+        my @children = ();
-        my @nodes = @$polynodes[@{chained_path(\@ordering_points)}];
+        foreach my $polynode (@$polynodes) {
-        
+            my $polygon = ($polynode->{outer} // $polynode->{hole})->clone;
-        my @loops = ();
-        
-        foreach my $polynode (@nodes) {
-            # if this is an external contour find all holes belonging to this contour(s)
-            # and prepend them
-            if ($is_contour && $depth == 0) {
-                # $polynode is the outermost loop of an island
-                my @holes = ();
-                for (my $i = 0; $i <= $#$holes_pt; $i++) {
-                    if ($polynode->{outer}->contains_point($holes_pt->[$i]{outer}->first_point)) {
-                        push @holes, splice @$holes_pt, $i, 1;  # remove from candidates to reduce complexity
-                        $i--;
-                    }
-                }
-                push @loops, reverse map $traverse->([$_], 0), @holes;
-            }
-            push @loops, $traverse->($polynode->{children}, $depth+1, $is_contour);
             
             # return ccw contours and cw holes
             # GCode.pm will convert all of them to ccw, but it needs to know
             # what the holes are in order to compute the correct inwards move
-            
+            if ($is_contour) {
-            my $polygon = ($polynode->{outer} // $polynode->{hole})->clone;
+                $polygon->make_counter_clockwise;
-            $polygon->reverse if defined $polynode->{hole};
+            } else {
-            $polygon->reverse if !$is_contour;
+                $polygon->make_clockwise;
+            }
             
             my $role = EXTR_ROLE_PERIMETER;
             if ($is_contour ? $depth == 0 : !@{ $polynode->{children} }) {
@@ -200,11 +208,58 @@ sub make_perimeters {
                 $role = EXTR_ROLE_CONTOUR_INTERNAL_PERIMETER;
             }
             
-            push @loops, Slic3r::ExtrusionLoop->new(
+            $collection->append(Slic3r::ExtrusionLoop->new(
                 polygon         => $polygon,
                 role            => $role,
                 mm3_per_mm      => $mm3_per_mm,
-            );
+            ));
+            
+            # save the children
+            push @children, $polynode->{children};
+        }
+
+        # if we're handling the top-level contours, add thin walls as candidates too
+        # in order to include them in the nearest-neighbor search
+        if ($is_contour && $depth == 0) {
+            foreach my $polyline (@thin_wall_polylines) {
+                $collection->append(Slic3r::ExtrusionPath->new(
+                    polyline        => $polyline,
+                    role            => EXTR_ROLE_EXTERNAL_PERIMETER,
+                    mm3_per_mm      => $mm3_per_mm,
+                ));
+            }
+        }
+        
+        # use a nearest neighbor search to order these children
+        # TODO: supply second argument to chained_path() too?
+        my $sorted_collection = $collection->chained_path(0);
+        my @orig_indices = @{$sorted_collection->orig_indices};
+        
+        my @loops = ();
+        foreach my $loop (@$sorted_collection) {
+            my $orig_index = shift @orig_indices;
+            
+            if ($loop->isa('Slic3r::ExtrusionPath')) {
+                push @loops, $loop->clone;
+            } else {
+                # if this is an external contour find all holes belonging to this contour(s)
+                # and prepend them
+                if ($is_contour && $depth == 0) {
+                    # $loop is the outermost loop of an island
+                    my @holes = ();
+                    for (my $i = 0; $i <= $#$holes_pt; $i++) {
+                        if ($loop->contains_point($holes_pt->[$i]{outer}->first_point)) {
+                            push @holes, splice @$holes_pt, $i, 1;  # remove from candidates to reduce complexity
+                            $i--;
+                        }
+                    }
+                    push @loops, reverse map $traverse->([$_], 0, 0), @holes;
+                }
+                
+                # traverse children and prepend them to this loop
+                push @loops, $traverse->($children[$orig_index], $depth+1, $is_contour);
+                push @loops, $loop->clone;
+            }
         }
         return @loops;
     };
@@ -222,43 +277,6 @@ sub make_perimeters {
     # append perimeters
     $self->perimeters->append(@loops);
     
-    # process thin walls by collapsing slices to single passes
-    # the following offset2 ensures almost nothing in @thin_walls is narrower than $min_width
-    # (actually, something larger than that still may exist due to mitering or other causes)
-    my $min_width = $pwidth / 4;
-    @thin_walls = @{offset2_ex([ map @$_, @thin_walls ], -$min_width/2, +$min_width/2)};
-    if (@thin_walls) {
-        # the maximum thickness of our thin wall area is equal to the minimum thickness of a single loop
-        my @p = map @{$_->medial_axis($pwidth + $pspacing, $min_width)}, @thin_walls;
-        
-        if (0) {
-            require "Slic3r/SVG.pm";
-            Slic3r::SVG::output(
-                "medial_axis.svg",
-                no_arrows => 1,
-                expolygons      => \@thin_walls,
-                green_polylines => [ map $_->polygon->split_at_first_point, @{$self->perimeters} ],
-                red_polylines   => \@p,
-            );
-        }
-        
-        my @paths = ();
-        for my $p (@p) {
-            my %params = (
-                role        => EXTR_ROLE_EXTERNAL_PERIMETER,
-                mm3_per_mm  => $mm3_per_mm,
-            );
-            push @paths, $p->isa('Slic3r::Polygon')
-                ? Slic3r::ExtrusionLoop->new(polygon  => $p, %params)
-                : Slic3r::ExtrusionPath->new(polyline => $p, %params);
-        }
-        
-        $self->perimeters->append(
-            map $_->clone, @{Slic3r::ExtrusionPath::Collection->new(@paths)->chained_path(0)}
-        );
-        Slic3r::debugf "  %d thin walls detected\n", scalar(@paths) if $Slic3r::debug;
-    }
-    
     $self->_fill_gaps(\@gaps);
 }
 

xs/lib/Slic3r/XS.pm

@@ -109,7 +109,7 @@ sub new {
 sub clone {
     my ($self, %args) = @_;
     
-    return (ref $self)->_new(
+    return __PACKAGE__->_new(
         $args{polygon}       // $self->polygon,
         $args{role}          // $self->role,
         $args{mm3_per_mm}    // $self->mm3_per_mm,
@@ -139,7 +139,7 @@ sub new {
 sub clone {
     my ($self, %args) = @_;
     
-    return (ref $self)->_new(
+    return __PACKAGE__->_new(
         $args{polyline}      // $self->polyline,
         $args{role}          // $self->role,
         $args{mm3_per_mm}    // $self->mm3_per_mm,

xs/src/ExtrusionEntity.cpp

@@ -54,22 +54,22 @@ ExtrusionPath::last_point() const
     return new Point(this->polyline.points.back());
 }
 
-ExtrusionEntityCollection*
+void
-ExtrusionPath::intersect_expolygons(ExPolygonCollection* collection) const
+ExtrusionPath::intersect_expolygons(const ExPolygonCollection &collection, ExtrusionEntityCollection* retval) const
 {
     // perform clipping
     Polylines clipped;
-    intersection(this->polyline, *collection, clipped);
+    intersection(this->polyline, collection, clipped);
-    return this->_inflate_collection(clipped);
+    return this->_inflate_collection(clipped, retval);
 }
 
-ExtrusionEntityCollection*
+void
-ExtrusionPath::subtract_expolygons(ExPolygonCollection* collection) const
+ExtrusionPath::subtract_expolygons(const ExPolygonCollection &collection, ExtrusionEntityCollection* retval) const
 {
     // perform clipping
     Polylines clipped;
-    diff(this->polyline, *collection, clipped);
+    diff(this->polyline, collection, clipped);
-    return this->_inflate_collection(clipped);
+    return this->_inflate_collection(clipped, retval);
 }
 
 void
@@ -90,16 +90,14 @@ ExtrusionPath::length() const
     return this->polyline.length();
 }
 
-ExtrusionEntityCollection*
+void
-ExtrusionPath::_inflate_collection(const Polylines &polylines) const
+ExtrusionPath::_inflate_collection(const Polylines &polylines, ExtrusionEntityCollection* collection) const
 {
-    ExtrusionEntityCollection* retval = new ExtrusionEntityCollection();
     for (Polylines::const_iterator it = polylines.begin(); it != polylines.end(); ++it) {
         ExtrusionPath* path = this->clone();
         path->polyline = *it;
-        retval->entities.push_back(path);
+        collection->entities.push_back(path);
     }
-    return retval;
 }
 
 ExtrusionLoop*

xs/src/ExtrusionEntity.hpp

@@ -50,13 +50,13 @@ class ExtrusionPath : public ExtrusionEntity
     void reverse();
     Point* first_point() const;
     Point* last_point() const;
-    ExtrusionEntityCollection* intersect_expolygons(ExPolygonCollection* collection) const;
+    void intersect_expolygons(const ExPolygonCollection &collection, ExtrusionEntityCollection* retval) const;
-    ExtrusionEntityCollection* subtract_expolygons(ExPolygonCollection* collection) const;
+    void subtract_expolygons(const ExPolygonCollection &collection, ExtrusionEntityCollection* retval) const;
     void clip_end(double distance);
     void simplify(double tolerance);
     double length() const;
     private:
-    ExtrusionEntityCollection* _inflate_collection(const Polylines &polylines) const;
+    void _inflate_collection(const Polylines &polylines, ExtrusionEntityCollection* collection) const;
 };
 
 class ExtrusionLoop : public ExtrusionEntity

xs/src/ExtrusionEntityCollection.cpp

@@ -47,6 +47,8 @@ ExtrusionEntityCollection::chained_path_from(Point* start_near, bool no_reverse)
 {
     if (this->no_sort) return this->clone();
     ExtrusionEntityCollection* retval = new ExtrusionEntityCollection;
+    retval->entities.reserve(this->entities.size());
+    retval->orig_indices.reserve(this->entities.size());
     
     ExtrusionEntitiesPtr my_paths;
     for (ExtrusionEntitiesPtr::const_iterator it = this->entities.begin(); it != this->entities.end(); ++it) {
@@ -71,6 +73,7 @@ ExtrusionEntityCollection::chained_path_from(Point* start_near, bool no_reverse)
             my_paths.at(path_index)->reverse();
         }
         retval->entities.push_back(my_paths.at(path_index));
+        retval->orig_indices.push_back(path_index);
         my_paths.erase(my_paths.begin() + path_index);
         endpoints.erase(endpoints.begin() + 2*path_index, endpoints.begin() + 2*path_index + 2);
         start_near = retval->entities.back()->last_point();

xs/src/ExtrusionEntityCollection.hpp

@@ -11,6 +11,7 @@ class ExtrusionEntityCollection : public ExtrusionEntity
     public:
     ExtrusionEntityCollection* clone() const;
     ExtrusionEntitiesPtr entities;
+    std::vector<int> orig_indices;
     bool no_sort;
     ExtrusionEntityCollection(): no_sort(false) {};
     ExtrusionEntityCollection* chained_path(bool no_reverse) const;

xs/t/04_expolygon.t

@@ -104,10 +104,4 @@ is $expolygon->area, 100*100-20*20, 'area';
     is_deeply $collection->[0]->clone->pp, $collection->[0]->pp, 'clone collection item';
 }
 
-{
-    my $expolygon = Slic3r::ExPolygon->new($square, $hole_in_square);
-    my $res = $expolygon->medial_axis(10);
-    use XXX; YYY $res;
-}
-
 __END__

xs/t/12_extrusionpathcollection.t

@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use Slic3r::XS;
-use Test::More tests => 14;
+use Test::More tests => 16;
 
 my $points = [
     [100, 100],
@@ -40,6 +40,8 @@ is scalar(@$collection), 4, 'append ExtrusionLoop';
 isa_ok $collection->[1], 'Slic3r::ExtrusionPath::Collection::Ref', 'correct object returned for collection';
 isa_ok $collection->[2], 'Slic3r::ExtrusionPath::Ref', 'correct object returned for path';
 isa_ok $collection->[3], 'Slic3r::ExtrusionLoop::Ref', 'correct object returned for loop';
+is ref($collection->[2]->clone), 'Slic3r::ExtrusionPath', 'correct object returned for cloned path';
+is ref($collection->[3]->clone), 'Slic3r::ExtrusionLoop', 'correct object returned for cloned loop';
 
 is scalar(@{$collection->[1]}), 1, 'appended collection was duplicated';
 

xs/xsp/ExtrusionEntityCollection.xsp

@@ -20,6 +20,8 @@
         %code{% const char* CLASS = "Slic3r::Point"; RETVAL = THIS->last_point(); %};
     int count()
         %code{% RETVAL = THIS->entities.size(); %};
+    std::vector<int> orig_indices()
+        %code{% RETVAL = THIS->orig_indices; %};
 %{
 
 void

xs/xsp/ExtrusionPath.xsp

@@ -3,6 +3,7 @@
 %{
 #include <myinit.h>
 #include "ExtrusionEntity.hpp"
+#include "ExtrusionEntityCollection.hpp"
 %}
 
 %name{Slic3r::ExtrusionPath} class ExtrusionPath {
@@ -20,10 +21,6 @@
         %code{% const char* CLASS = "Slic3r::Point"; RETVAL = THIS->first_point(); %};
     Point* last_point()
         %code{% const char* CLASS = "Slic3r::Point"; RETVAL = THIS->last_point(); %};
-    ExtrusionEntityCollection* intersect_expolygons(ExPolygonCollection* collection)
-        %code{% const char* CLASS = "Slic3r::ExtrusionPath::Collection"; RETVAL = THIS->intersect_expolygons(collection); %};
-    ExtrusionEntityCollection* subtract_expolygons(ExPolygonCollection* collection)
-        %code{% const char* CLASS = "Slic3r::ExtrusionPath::Collection"; RETVAL = THIS->subtract_expolygons(collection); %};
     void clip_end(double distance);
     void simplify(double tolerance);
     double length();
@@ -87,6 +84,26 @@ ExtrusionPath::append(...)
             THIS->polyline.points.push_back(p);
         }
 
+ExtrusionEntityCollection*
+ExtrusionPath::intersect_expolygons(ExPolygonCollection* collection)
+    PREINIT:
+        const char* CLASS = "Slic3r::ExtrusionPath::Collection";
+    CODE:
+        RETVAL = new ExtrusionEntityCollection ();
+        THIS->intersect_expolygons(*collection, RETVAL);
+    OUTPUT:
+        RETVAL
+
+ExtrusionEntityCollection*
+ExtrusionPath::subtract_expolygons(ExPolygonCollection* collection)
+    PREINIT:
+        const char* CLASS = "Slic3r::ExtrusionPath::Collection";
+    CODE:
+        RETVAL = new ExtrusionEntityCollection ();
+        THIS->subtract_expolygons(*collection, RETVAL);
+    OUTPUT:
+        RETVAL
+
 %}
 };
 

xs/xsp/typemap.xspt

@@ -1,6 +1,7 @@
 %typemap{bool}{simple};
 %typemap{std::string};
 %typemap{t_config_option_key};
+%typemap{std::vector<int>};
 %typemap{std::vector<unsigned int>*};
 %typemap{std::vector<double>};
 %typemap{std::vector<double>*};