Refactored Perimeter code with new Slic3r::Polygon and Slic3r::ExPolygon objects

Large refactoring. Speed gains. Removed convex hull for bridges.

lib/Slic3r.pm

@@ -9,6 +9,7 @@ sub debugf {
 }
 
 use Slic3r::Config;
+use Slic3r::ExPolygon;
 use Slic3r::Extruder;
 use Slic3r::ExtrusionLoop;
 use Slic3r::ExtrusionPath;
@@ -20,6 +21,7 @@ use Slic3r::Line;
 use Slic3r::Line::FacetEdge;
 use Slic3r::Perimeter;
 use Slic3r::Point;
+use Slic3r::Polygon;
 use Slic3r::Polyline;
 use Slic3r::Polyline::Closed;
 use Slic3r::Print;
@@ -27,7 +29,6 @@ use Slic3r::Skein;
 use Slic3r::STL;
 use Slic3r::Surface;
 use Slic3r::Surface::Bridge;
-use Slic3r::Surface::Collection;
 
 # printer options
 our $nozzle_diameter    = 0.5;

lib/Slic3r/ExPolygon.pm

@@ -0,0 +1,66 @@
+package Slic3r::ExPolygon;
+use strict;
+use warnings;
+
+# an ExPolygon is a polygon with holes
+
+use Math::Clipper qw(CT_UNION PFT_NONZERO JT_MITER);
+use Slic3r::Geometry::Clipper qw(union_ex);
+
+# the constructor accepts an array of polygons 
+# or a Math::Clipper ExPolygon (hashref)
+sub new {
+    my $class = shift;
+    my $self;
+    if (@_ == 1 && ref $_[0] eq 'HASH') {
+        $self = [
+            Slic3r::Polygon->new($_[0]{outer}),
+            map Slic3r::Polygon->new($_), @{$_[0]{holes}},
+        ];
+    } else {
+        $self = [@_];
+    }
+    bless $self, $class;
+    $self;
+}
+
+# this class method accepts an array of polygons and returns
+# an array of expolygons with the right holes applied to the 
+# right contours
+sub make {
+    my $class = shift;
+    return map $class->new($_), @{ union_ex(\@_) };
+}
+
+sub contour {
+    my $self = shift;
+    return $self->[0];
+}
+
+sub holes {
+    my $self = shift;
+    return @$self[1..$#$self];
+}
+
+sub clipper_expolygon {
+    my $self = shift;
+    return {
+        outer => $self->contour,
+        holes => [ $self->holes ],
+    };
+}
+
+sub offset {
+    my $self = shift;
+    my ($distance, $scale, $joinType, $miterLimit) = @_;
+    $scale      ||= $Slic3r::resolution * 1000000;
+    $joinType   = JT_MITER if !defined $joinType;
+    $miterLimit ||= 2;
+    
+    my $offsets = Math::Clipper::offset($self, $distance, $scale, $joinType, $miterLimit);
+    
+    # apply holes to the right contours
+    return (ref $self)->make(@$offsets);
+}
+
+1;

lib/Slic3r/ExtrusionPath.pm

@@ -55,14 +55,14 @@ sub split_at_acute_angles {
             # if the angle between $p[-2], $p[-1], $p3 is too acute
             # then consider $p3 only as a starting point of a new
             # path and stop the current one as it is
-            push @paths, __PACKAGE__->cast([@p]);
+            push @paths, (ref $self)->cast([@p]);
             @p = ($p3);
             push @p, grep $_, shift @points or last;
         } else {
             push @p, $p3;
         }
     }
-    push @paths, __PACKAGE__->cast([@p]) if @p > 1;
+    push @paths, (ref $self)->cast([@p]) if @p > 1;
     return @paths;
 }
 

lib/Slic3r/Fill.pm

@@ -32,10 +32,10 @@ sub make_fill {
     }
     
     printf "Filling layer %d:\n", $layer->id;
-    foreach my $surface_collection (@{ $layer->fill_surfaces }) {
+    foreach my $surfaces (@{ $layer->fill_surfaces }) {
         my @path_collection = ();
         
-        SURFACE: foreach my $surface (@{ $surface_collection->surfaces }) {
+        SURFACE: foreach my $surface (@$surfaces) {
             Slic3r::debugf " Processing surface %s:\n", $surface->id;
             
             my $filler = $Slic3r::fill_type;

lib/Slic3r/Layer.pm

@@ -2,7 +2,6 @@ package Slic3r::Layer;
 use Moo;
 
 use Math::Clipper ':all';
-use Math::ConvexHull qw(convex_hull);
 use Slic3r::Geometry qw(polygon_lines points_coincide angle3points polyline_lines nearest_point
     line_length);
 use Slic3r::Geometry::Clipper qw(union_ex);
@@ -63,10 +62,10 @@ has 'skirts' => (
 );
 
 # collection of surfaces generated by offsetting the innermost perimeter(s)
-# they represent boundaries of areas to fill
+# they represent boundaries of areas to fill (grouped by original objects)
 has 'fill_surfaces' => (
     is      => 'rw',
-    #isa     => 'ArrayRef[Slic3r::Surface::Collection]',
+    #isa     => 'ArrayRef[ArrayRef[Slic3r::Surface]]',
     default => sub { [] },
 );
 
@@ -156,7 +155,7 @@ sub make_surfaces {
     }
     
     my $n = 0;
-    my @polylines = ();
+    my @polygons = ();
     while (my $first_line = shift @lines) {
         my @points = @$first_line;
         my %seen_points = map { $get_point_id->($points[$_]) => $_ } 0..1;
@@ -219,14 +218,15 @@ sub make_surfaces {
         }
         
         pop @points;
-        Slic3r::debugf "Discovered polyline of %d points\n", scalar(@points);
+        Slic3r::debugf "Discovered polygon of %d points\n", scalar(@points);
-        push @polylines, [@points];
+        push @polygons, Slic3r::Polygon->new(@points);
+        $polygons[-1]->cleanup;
     }
     
     {
-        my $expolygons = union_ex([ @polylines ]);
+        my $expolygons = union_ex([ @polygons ]);
         Slic3r::debugf "  %d surface(s) detected from %d polylines\n",
-            scalar(@$expolygons), scalar(@polylines);
+            scalar(@$expolygons), scalar(@polygons);
         
         push @{$self->surfaces}, map Slic3r::Surface->cast_from_expolygon($_, surface_type => 'internal'), @$expolygons;
     }
@@ -279,7 +279,7 @@ sub process_bridges {
     SURFACE: foreach my $surface (@bottom_surfaces) {
         # since we can't print concave bridges, we transform the surface
         # in a convex polygon; this will print thin membranes eventually
-        my $surface_p = convex_hull($surface->contour->p);
+        my $surface_p = $surface->contour->p;
         
         # offset the surface a bit to avoid approximation issues when doing the
         # intersection below (this is to make sure we overlap with supporting
@@ -407,9 +407,9 @@ sub split_bridges_fills {
     my $self = shift;
     
     my $clipper = Math::Clipper->new;
-    foreach my $surf_coll (@{$self->fill_surfaces}) {
+    foreach my $surfaces (@{$self->fill_surfaces}) {
-        my @surfaces = @{$surf_coll->surfaces};
+        my @surfaces = @$surfaces;
-        @{$surf_coll->surfaces} = ();
+        @$surfaces = ();
         
         # intersect fill_surfaces with bridges to get actual bridges
         foreach my $bridge (@{$self->bridges}) {
@@ -417,7 +417,7 @@ sub split_bridges_fills {
             $clipper->add_subject_polygons([ map $_->p, @surfaces ]);
             $clipper->add_clip_polygon($bridge->contour->p);
             my $intersection = $clipper->ex_execute(CT_INTERSECTION, PFT_NONZERO, PFT_NONZERO);
-            push @{$surf_coll->surfaces}, map Slic3r::Surface::Bridge->cast_from_expolygon($_,
+            push @$surfaces, map Slic3r::Surface::Bridge->cast_from_expolygon($_,
                 surface_type => 'bottom',
                 bridge_angle => $bridge->bridge_angle,
             ), @$intersection;
@@ -429,7 +429,7 @@ sub split_bridges_fills {
             $clipper->add_subject_polygons([ $surface->p ]);
             $clipper->add_clip_polygons([ map $_->contour->p, @{$self->bridges} ]);
             my $difference = $clipper->ex_execute(CT_DIFFERENCE, PFT_NONZERO, PFT_NONZERO);
-            push @{$surf_coll->surfaces}, map Slic3r::Surface->cast_from_expolygon($_,
+            push @$surfaces, map Slic3r::Surface->cast_from_expolygon($_,
                 surface_type => $surface->surface_type), @$difference;
         }
     }

lib/Slic3r/Line.pm

@@ -65,4 +65,9 @@ sub parallel_to {
     return Slic3r::Geometry::lines_parallel($self, $line);
 }
 
+sub length {
+    my $self = shift;
+    return Slic3r::Geometry::line_length($self);
+}
+
 1;

lib/Slic3r/Perimeter.pm

@@ -14,63 +14,64 @@ sub make_perimeter {
     printf "Making perimeter for layer %d:\n", $layer->id;
     
     # at least one perimeter is required
-    die "Can't extrude object without any perimeter!\n"
+    die "Can't slice object with no perimeters!\n"
         if $Slic3r::perimeter_offsets == 0;
     
-    my (%contours, %holes) = ();
+    # this array will hold one arrayref per original surface;
+    # each item of this arrayref is an arrayref representing a depth (from inner
+    # perimeters to outer); each item of this arrayref is an ExPolygon:
+    # @perimeters = (
+    #    [ # first object (identified by a single surface before offsetting)
+    #        [ Slic3r::ExPolygon, Slic3r::ExPolygon... ],  #item 0: outer loop
+    #        [ Slic3r::ExPolygon, Slic3r::ExPolygon... ],  #item 1: inner loop
+    #    ],
+    #    [ # second object
+    #        ...
+    #    ]
+    # )
+    my @perimeters = ();  # one item per depth; each item
+    
     foreach my $surface (@{ $layer->perimeter_surfaces }) {
-        $contours{$surface} = [];
+        # the outer loop must be offsetted by half extrusion width inwards
-        $holes{$surface} = [];
+        my @last_offsets = ($surface->expolygon);
-        my @last_offsets = ();
+        my $distance = $Slic3r::flow_width / 2 / $Slic3r::resolution;
-        
-        # first perimeter
-        {
-            my $polygon = $surface->clipper_polygon;
-            my ($contour_p, @holes_p) = ($polygon->{outer}, @{$polygon->{holes}});
-            push @{ $contours{$surface} }, $contour_p;
-            push @{ $holes{$surface} }, @holes_p;
-            @last_offsets = ($polygon);
-        }
         
         # create other offsets
-        for (my $loop = 1; $loop < $Slic3r::perimeter_offsets; $loop++) {
+        push @perimeters, [];
-            
+        for (my $loop = 0; $loop < $Slic3r::perimeter_offsets; $loop++) {
             # offsetting a polygon can result in one or many offset polygons
-            @last_offsets = map $self->offset_polygon($_), @last_offsets;
+            @last_offsets = map $_->offset(-$distance), @last_offsets;
+            push @{ $perimeters[-1] }, [@last_offsets];
             
-            foreach my $offset_polygon (@last_offsets) {
+            # offset distance for inner loops
-                my ($contour_p, @holes_p) = ($offset_polygon->{outer}, @{$offset_polygon->{holes}});
+            $distance = $Slic3r::flow_width / $Slic3r::resolution;
-                
-                push @{ $contours{$surface} }, $contour_p;
-                push @{ $holes{$surface} }, @holes_p;
-            }
         }
         
         # create one more offset to be used as boundary for fill
         {
-            my @fill_surfaces = map Slic3r::Surface->cast_from_expolygon(
+            my @fill_surfaces = map Slic3r::Surface->cast_from_expolygon
-                $_,
+                ($_, surface_type => $surface->surface_type),
-                surface_type => $surface->surface_type,
+                map $_->offset(-$distance), @last_offsets;
-            ), map $self->offset_polygon($_), @last_offsets;
             
-            push @{ $layer->fill_surfaces }, Slic3r::Surface::Collection->new(
+            push @{ $layer->fill_surfaces }, [@fill_surfaces] if @fill_surfaces;
-                surfaces => [@fill_surfaces],
-            ) if @fill_surfaces;
         }
     }
     
-    # generate paths for holes:
+    # first generate paths for all holes, starting from external (innermost) perimeters
-    # we start from innermost loops (that is, external ones), do them
+    foreach my $i (1..$Slic3r::perimeter_offsets) {
-    # for all holes, than go on with inner loop and do that for all
+        foreach my $hole (map $_->holes, map @{$_->[$i-1]}, @perimeters) {
-    # holes and so on;
+            push @{ $layer->perimeters }, Slic3r::ExtrusionLoop->cast($hole);
-    # then we generate paths for contours:
+        }
+    }
+    
+    # then generate paths for contours
     # this time we do something different: we do contour loops for one
     # shape (that is, one original surface) at a time: we start from the
     # innermost loop (that is, internal one), then without interrupting 
     # our path we go onto the outer loop and continue; this should ensure
     # good surface quality
-    foreach my $p (map @$_, values %holes, values %contours) {
+    foreach my $contour (map $_->contour, map @$_, map @$_, @perimeters) {
-        push @{ $layer->perimeters }, Slic3r::ExtrusionLoop->cast($p);
+        push @{ $layer->perimeters }, Slic3r::ExtrusionLoop->cast($contour);
     }
     
     # generate skirt on bottom layer
@@ -87,32 +88,4 @@ sub make_perimeter {
     }
 }
 
-sub offset_polygon {
-    my $self = shift;
-    my ($polygon) = @_;
-    # $polygon holds a Math::Clipper ExPolygon hashref representing 
-    # a polygon and its holes
-    
-    # generate offsets
-    my $distance = $Slic3r::flow_width / $Slic3r::resolution;
-    my $offsets = offset([ $polygon->{outer}, @{$polygon->{holes}} ], -$distance, 
-        $Slic3r::resolution * 100000, JT_MITER, 2);
-    
-    # defensive programming
-    my (@contour_offsets, @hole_offsets) = ();
-    for (@$offsets) {
-        if (is_counter_clockwise($_)) {
-            push @contour_offsets, $_;
-        } else {
-            push @hole_offsets, $_;
-        }
-    }
-    
-    # apply holes to the right contours
-    my $clipper = Math::Clipper->new;
-    $clipper->add_subject_polygons($offsets);
-    my $results = $clipper->ex_execute(CT_UNION, PFT_NONZERO, PFT_NONZERO);
-    return @$results;
-}
-
 1;

lib/Slic3r/Polygon.pm

@@ -0,0 +1,30 @@
+package Slic3r::Polygon;
+use strict;
+use warnings;
+
+# a polygon is a closed polyline.
+# if you're asking why there's a Slic3r::Polygon as well
+# as a Slic3r::Polyline::Closed you're right. I plan to
+# ditch the latter and port everything to this class.
+
+use Slic3r::Geometry qw(polygon_remove_parallel_continuous_edges);
+
+# the constructor accepts an array(ref) of points
+sub new {
+    my $class = shift;
+    my $self;
+    if (@_ == 1) {
+        $self = [ @{$_[0]} ];
+    } else {
+        $self = [ @_ ];
+    }
+    bless $self, $class;
+    $self;
+}
+
+sub cleanup {
+    my $self = shift;
+    polygon_remove_parallel_continuous_edges($self);
+}
+
+1;

lib/Slic3r/Print.pm

@@ -202,7 +202,7 @@ sub discover_horizontal_shells {
         my $layer = $self->layers->[$i];
         foreach my $type (qw(top bottom)) {
             # find surfaces of current type for current layer
-            my @surfaces = grep $_->surface_type eq $type, map @{$_->surfaces}, @{$layer->fill_surfaces} or next;
+            my @surfaces = grep $_->surface_type eq $type, map @$_, @{$layer->fill_surfaces} or next;
             Slic3r::debugf "Layer %d has %d surfaces of type '%s'\n",
                 $i, scalar(@surfaces), $type;
             
@@ -213,8 +213,8 @@ sub discover_horizontal_shells {
                 next if $n < 0 || $n >= $self->layer_count;
                 Slic3r::debugf "  looking for neighbors on layer %d...\n", $n;
                 
-                foreach my $surf_coll (@{$self->layers->[$n]->fill_surfaces}) {
+                foreach my $surfaces (@{$self->layers->[$n]->fill_surfaces}) {
-                    my $neighbor_polygons = [ map $_->p, grep $_->surface_type =~ /internal/, @{$surf_coll->surfaces} ];
+                    my $neighbor_polygons = [ map $_->p, grep $_->surface_type =~ /internal/, @$surfaces ];
                     
                     # find intersection between @surfaces and current layer's surfaces
                     $clipper->add_subject_polygons([ map $_->p, @surfaces ]);
@@ -237,9 +237,9 @@ sub discover_horizontal_shells {
                     # polygons as $internal_polygons; they will be removed by removed_small_features()
                     
                     # assign resulting inner surfaces to layer
-                    $surf_coll->surfaces([]);
+                    @$surfaces = ();
                     foreach my $p (@$internal_polygons) {
-                        push @{$surf_coll->surfaces}, Slic3r::Surface->new(
+                        push @$surfaces, Slic3r::Surface->new(
                             surface_type => 'internal',
                             contour => Slic3r::Polyline::Closed->cast($p->{outer}),
                             holes   => [
@@ -250,7 +250,7 @@ sub discover_horizontal_shells {
                     
                     # assign new internal-solid surfaces to layer
                     foreach my $p (@$intersections) {
-                        push @{$surf_coll->surfaces}, Slic3r::Surface->new(
+                        push @$surfaces, Slic3r::Surface->new(
                             surface_type => 'internal-solid',
                             contour => Slic3r::Polyline::Closed->cast($p->{outer}),
                             holes   => [

lib/Slic3r/Surface.pm

@@ -33,6 +33,10 @@ sub cast_from_expolygon {
     my $class = shift;
     my ($expolygon, %args) = @_;
     
+    if (ref $expolygon ne 'HASH') {
+        $expolygon = $expolygon->clipper_expolygon;
+    }
+    
     return $class->new(
         contour      => Slic3r::Polyline::Closed->cast($expolygon->{outer}),
         holes        => [
@@ -79,6 +83,11 @@ sub p {
     return ($self->contour->p, map $_->p, @{$self->holes});
 }
 
+sub expolygon {
+    my $self = shift;
+    return Slic3r::ExPolygon->new($self->contour->p, map $_->p, @{$self->holes});
+}
+
 sub lines {
     my $self = shift;
     return @{ $self->contour->lines }, map @{ $_->lines }, @{ $self->holes };

lib/Slic3r/Surface/Collection.pm

@@ -1,10 +0,0 @@
-package Slic3r::Surface::Collection;
-use Moo;
-
-has 'surfaces' => (
-    is      => 'rw',
-    #isa     => 'ArrayRef[Slic3r::Surface]',
-    default => sub { [] },
-);
-
-1;