From aef5c05c57db87d2c78fbdbfcf7c81e3cad5b472 Mon Sep 17 00:00:00 2001
From: Alessandro Ranellucci <aar@cpan.org>
Date: Mon, 16 Sep 2013 10:33:30 +0200
Subject: [PATCH] Update brim generation code. Includes regression test. #1440

---
 lib/Slic3r/GCode/Layer.pm            |  5 +++--
 lib/Slic3r/Print.pm                  | 24 ++++++++----------------
 t/skirt_brim.t                       | 27 ++++++++++++++++++++++++++-
 xs/xsp/ExtrusionEntityCollection.xsp |  1 +
 4 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/lib/Slic3r/GCode/Layer.pm b/lib/Slic3r/GCode/Layer.pm
index aa6af1d60..5e76fccdd 100644
--- a/lib/Slic3r/GCode/Layer.pm
+++ b/lib/Slic3r/GCode/Layer.pm
@@ -77,13 +77,14 @@ sub process_layer {
         # skip skirt if we have a large brim
         if ($layer->id < $Slic3r::Config->skirt_height) {
             # distribute skirt loops across all extruders
-            for my $i (0 .. $#{$self->print->skirt}) {
+            my @skirt_loops = @{$self->print->skirt};
+            for my $i (0 .. $#skirt_loops) {
                 # when printing layers > 0 ignore 'min_skirt_length' and 
                 # just use the 'skirts' setting; also just use the current extruder
                 last if ($layer->id > 0) && ($i >= $Slic3r::Config->skirts);
                 $gcode .= $self->gcodegen->set_extruder($self->extruders->[ ($i/@{$self->extruders}) % @{$self->extruders} ])
                     if $layer->id == 0;
-                $gcode .= $self->gcodegen->extrude_loop($self->print->skirt->[$i], 'skirt');
+                $gcode .= $self->gcodegen->extrude_loop($skirt_loops[$i], 'skirt');
             }
         }
         $self->skirt_done->{$layer->print_z} = 1;
diff --git a/lib/Slic3r/Print.pm b/lib/Slic3r/Print.pm
index a82354782..762931fe6 100644
--- a/lib/Slic3r/Print.pm
+++ b/lib/Slic3r/Print.pm
@@ -22,18 +22,10 @@ has 'first_layer_support_material_flow' => (is => 'rw');
 has 'has_support_material'   => (is => 'lazy');
 
 # ordered collection of extrusion paths to build skirt loops
-has 'skirt' => (
-    is      => 'rw',
-    #isa     => 'ArrayRef[Slic3r::ExtrusionLoop]',
-    default => sub { [] },
-);
+has 'skirt' => (is => 'rw', default => sub { Slic3r::ExtrusionPath::Collection->new });
 
 # ordered collection of extrusion paths to build a brim
-has 'brim' => (
-    is      => 'rw',
-    #isa     => 'ArrayRef[Slic3r::ExtrusionLoop]',
-    default => sub { [] },
-);
+has 'brim' => (is => 'rw', default => sub { Slic3r::ExtrusionPath::Collection->new });
 
 sub BUILD {
     my $self = shift;
@@ -609,11 +601,11 @@ sub make_skirt {
     for (my $i = $Slic3r::Config->skirts; $i > 0; $i--) {
         $distance += scale $spacing;
         my $loop = Slic3r::Geometry::Clipper::offset([$convex_hull], $distance, 0.0001, JT_ROUND)->[0];
-        push @{$self->skirt}, Slic3r::ExtrusionLoop->new(
+        $self->skirt->append(Slic3r::ExtrusionLoop->new(
             polygon         => Slic3r::Polygon->new(@$loop),
             role            => EXTR_ROLE_SKIRT,
             flow_spacing    => $spacing,
-        );
+        ));
         
         if ($Slic3r::Config->min_skirt_length > 0) {
             $extruded_length[$extruder_idx]     ||= 0;
@@ -629,7 +621,7 @@ sub make_skirt {
         }
     }
     
-    @{$self->skirt} = reverse @{$self->skirt};
+    $self->skirt->reverse;
 }
 
 sub make_brim {
@@ -657,7 +649,7 @@ sub make_brim {
                 if $support_layer0->support_interface_fills;
         }
         foreach my $copy (@{$object->copies}) {
-            push @islands, map $_->clone->translate(@$copy), @object_islands;
+            push @islands, map { $_->translate(@$copy); $_ } map $_->clone, @object_islands;
         }
     }
     
@@ -677,11 +669,11 @@ sub make_brim {
         push @loops, @{offset2(\@islands, ($i + 0.5) * $flow->scaled_spacing, -1.0 * $flow->scaled_spacing, 100000, JT_SQUARE)};
     }
     
-    @{$self->brim} = map Slic3r::ExtrusionLoop->new(
+    $self->brim->append(map Slic3r::ExtrusionLoop->new(
         polygon         => Slic3r::Polygon->new(@$_),
         role            => EXTR_ROLE_SKIRT,
         flow_spacing    => $flow->spacing,
-    ), reverse traverse_pt( union_pt(\@loops) );
+    ), reverse traverse_pt( union_pt(\@loops) ));
 }
 
 sub write_gcode {
diff --git a/t/skirt_brim.t b/t/skirt_brim.t
index e73ccc620..8c616d98a 100644
--- a/t/skirt_brim.t
+++ b/t/skirt_brim.t
@@ -1,4 +1,4 @@
-use Test::More tests => 1;
+use Test::More tests => 2;
 use strict;
 use warnings;
 
@@ -43,4 +43,29 @@ use Slic3r::Test;
     ok $test->(), "skirt_height is honored when printing multiple objects too";
 }
 
+{
+    my $config = Slic3r::Config->new_from_defaults;
+    $config->set('skirts', 0);
+    $config->set('perimeters', 0);
+    $config->set('top_solid_layers', 0);            # to prevent solid shells and their speeds
+    $config->set('bottom_solid_layers', 0);         # to prevent solid shells and their speeds
+    $config->set('brim_width', 5);
+    $config->set('cooling', 0);                     # to prevent speeds to be altered
+    $config->set('first_layer_speed', '100%');      # to prevent speeds to be altered
+    
+    my $print = Slic3r::Test::init_print('20mm_cube', config => $config);
+    
+    my %layers_with_brim = ();  # Z => $count
+    Slic3r::GCode::Reader->new->parse(Slic3r::Test::gcode($print), sub {
+        my ($self, $cmd, $args, $info) = @_;
+        
+        if (defined $self->Z) {
+            $layers_with_brim{$self->Z} //= 0;
+            $layers_with_brim{$self->Z} = 1
+                if $info->{extruding} && $info->{dist_XY} > 0 && ($args->{F} // $self->F) != $config->infill_speed*60;
+        }
+    });
+    is scalar(grep $_, values %layers_with_brim), 1, "brim is generated";
+}
+
 __END__
diff --git a/xs/xsp/ExtrusionEntityCollection.xsp b/xs/xsp/ExtrusionEntityCollection.xsp
index e5a8230fa..5870b3c80 100644
--- a/xs/xsp/ExtrusionEntityCollection.xsp
+++ b/xs/xsp/ExtrusionEntityCollection.xsp
@@ -7,6 +7,7 @@
 
 %name{Slic3r::ExtrusionPath::Collection} class ExtrusionEntityCollection {
     %name{_new} ExtrusionEntityCollection();
+    void reverse();
     void clear()
         %code{% THIS->entities.clear(); %};
     ExtrusionEntityCollection* chained_path(bool no_reverse)