Fixes to Config and plater; also refactored the arrange() code

lib/Slic3r/Config.pm

@@ -78,8 +78,7 @@ sub load {
     my $ini = __PACKAGE__->read_ini($file);
     my $config = __PACKAGE__->new;
     foreach my $opt_key (keys %{$ini->{_}}) {
-        print "key: $opt_key\n";
-        ($opt_key, my $value) = handle_legacy($opt_key, $ini->{_}{$opt_key});
+        ($opt_key, my $value) = _handle_legacy($opt_key, $ini->{_}{$opt_key});
         next if !defined $opt_key;
         $config->set_deserialize($opt_key, $value);
     }
@@ -103,8 +102,7 @@ sub get_value {
         : $self->get($opt_key);
 }
 
-sub handle_legacy {
-    my $self = shift;
+sub _handle_legacy {
     my ($opt_key, $value) = @_;
     
     # handle legacy options

lib/Slic3r/GUI/Plater.pm

@@ -591,10 +591,17 @@ sub changescale {
 sub arrange {
     my $self = shift;
     
+    # get the bounding box of the model area shown in the viewport
+    my $bb = Slic3r::Geometry::BoundingBox->new_from_points([
+        Slic3r::Point->new(@{ $self->point_to_model_units([0,0]) }),
+        Slic3r::Point->new(@{ $self->point_to_model_units(CANVAS_SIZE) }),
+    ]);
+    
     eval {
-        $self->{model}->arrange_objects($self->skeinpanel->config);
+        $self->{model}->arrange_objects($self->skeinpanel->config->min_object_distance, $bb);
     };
-    # ignore arrange warnings on purpose
+    # ignore arrange failures on purpose: user has visual feedback and we don't need to warn him
+    # when parts don't fit in print bed
     
     $self->update(1);
     $self->{canvas}->Refresh;

lib/Slic3r/GUI/SkeinPanel.pm

@@ -142,7 +142,7 @@ sub quick_slice {
             foreach my $object (@{$model->objects}) {
                 $object->add_instance(offset => [0,0]) if !defined $object->instances;
             }
-            $model->arrange_objects($config);
+            $model->arrange_objects($config->min_object_distance);
         }
         $model->center_instances_around_point($config->print_center);
         

lib/Slic3r/Geometry.pm

@@ -660,7 +660,7 @@ sub douglas_peucker2 {
 }
 
 sub arrange {
-    my ($total_parts, $partx, $party, $areax, $areay, $dist, $Config) = @_;
+    my ($total_parts, $partx, $party, $dist, $bb) = @_;
     
     my $linint = sub {
         my ($value, $oldmin, $oldmax, $newmin, $newmax) = @_;
@@ -671,22 +671,19 @@ sub arrange {
     $partx += $dist;
     $party += $dist;
     
-    # margin needed for the skirt
-    my $skirt_margin;		
-    if ($Config->skirts > 0) {
-        my $flow = Slic3r::Flow->new(
-            layer_height    => $Config->get_value('first_layer_height'),
-            nozzle_diameter => $Config->nozzle_diameter->[0],  # TODO: actually look for the extruder used for skirt
-            width           => $Config->get_value('first_layer_extrusion_width'),
-        );
-        $skirt_margin = ($flow->spacing * $Config->skirts + $Config->skirt_distance) * 2;
+    my ($areax, $areay);
+    if (defined $bb) {
+        my $size = $bb->size;
+        ($areax, $areay) = @$size[X,Y];
     } else {
-        $skirt_margin = 0;		
+        # bogus area size, large enough not to trigger the error below
+        $areax = $partx * $total_parts;
+        $areay = $party * $total_parts;
     }
     
     # this is how many cells we have available into which to put parts
-    my $cellw = int(($areax - $skirt_margin + $dist) / $partx);
-    my $cellh = int(($areay - $skirt_margin + $dist) / $party);
+    my $cellw = int(($areax + $dist) / $partx);
+    my $cellh = int(($areay + $dist) / $party);
     
     die "$total_parts parts won't fit in your print area!\n" if $total_parts > ($cellw * $cellh);
     
@@ -769,6 +766,11 @@ sub arrange {
 
         push @positions, [$cx * $partx, $cy * $party];
     }
+    
+    if (defined $bb) {
+        $_->[X] += $bb->x_min for @positions;
+        $_->[Y] += $bb->y_min for @positions;
+    }
     return @positions;
 }
 

lib/Slic3r/Model.pm

@@ -119,7 +119,7 @@ sub duplicate_objects_grid {
 # this will append more instances to each object
 # and then automatically rearrange everything
 sub duplicate_objects {
-    my ($self, $config, $copies_num) = @_;
+    my ($self, $copies_num, $distance, $bb) = @_;
     
     foreach my $object (@{$self->objects}) {
         my @instances = @{$object->instances};
@@ -133,13 +133,13 @@ sub duplicate_objects {
         }
     }
     
-    $self->arrange_objects($config);
+    $self->arrange_objects($distance, $bb);
 }
 
 # arrange objects preserving their instance count
 # but altering their instance positions
 sub arrange_objects {
-    my ($self, $config) = @_;
+    my ($self, $distance, $bb) = @_;
     
     # get the (transformed) size of each instance so that we take
     # into account their different transformations when packing
@@ -148,20 +148,20 @@ sub arrange_objects {
         push @instance_sizes, map $object->instance_bounding_box($_)->size, 0..$#{$object->instances};
     }
     
-    my @positions = $self->_arrange($config, \@instance_sizes);
+    my @positions = $self->_arrange(\@instance_sizes, $distance, $bb);
     
     foreach my $object (@{$self->objects}) {
-        $_->offset(shift @positions) for @{$object->instances};
+        $_->offset([ @{shift @positions} ]) for @{$object->instances};
         $object->update_bounding_box;
     }
 }
 
 # duplicate the entire model preserving instance relative positions
 sub duplicate {
-    my ($self, $config, $copies_num) = @_;
+    my ($self, $copies_num, $distance, $bb) = @_;
     
     my $model_size = $self->bounding_box->size;
-    my @positions = $self->_arrange($config, [ map $model_size, 2..$copies_num ]);
+    my @positions = $self->_arrange([ map $model_size, 2..$copies_num ], $distance, $bb);
     
     # note that this will leave the object count unaltered
     
@@ -181,13 +181,15 @@ sub duplicate {
 }
 
 sub _arrange {
-    my ($self, $config, $sizes) = @_;
+    my ($self, $sizes, $distance, $bb) = @_;
     
-    my $partx = max(map $_->[X], @$sizes);
-    my $party = max(map $_->[Y], @$sizes);
-    return Slic3r::Geometry::arrange
-        (scalar(@$sizes), $partx, $party, (map $_, @{$config->bed_size}),
-        $config->min_object_distance, $config);
+    return Slic3r::Geometry::arrange(
+        scalar(@$sizes),                # number of parts
+        max(map $_->[X], @$sizes),      # cell width
+        max(map $_->[Y], @$sizes),      # cell height
+        $distance,                      # distance between cells
+        $bb,                            # bounding box of the area to fill (can be undef)
+    );
 }
 
 sub has_objects_with_no_instances {

lib/Slic3r/Test.pm

@@ -108,7 +108,7 @@ sub init_print {
     $model_name = [$model_name] if ref($model_name) ne 'ARRAY';
     for my $model (map model($_, %params), @$model_name) {
         die "Unknown model in test" if !defined $model;
-        $model->arrange_objects($config);
+        $model->arrange_objects($config->min_object_distance);
         $model->center_instances_around_point($config->print_center);
         $print->add_model_object($_) for @{$model->objects};
     }

slic3r.pl

@@ -140,10 +140,10 @@ if (@ARGV) {  # slicing from command line
         if ($config->duplicate_grid->[X] > 1 || $config->duplicate_grid->[Y] > 1) {
             $model->duplicate_objects_grid($config->duplicate_grid, $config->duplicate_distance);
         } elsif ($need_arrange) {
-            $model->duplicate_objects($config, $config->duplicate);
+            $model->duplicate_objects($config->duplicate, $config->min_object_distance);
         } elsif ($config->duplicate > 1) {
             # if all input objects have defined position(s) apply duplication to the whole model
-            $model->duplicate($config, $config->duplicate);
+            $model->duplicate($config->duplicate, $config->min_object_distance);
         }
         $model->center_instances_around_point($config->print_center);
         

xs/src/Config.cpp

@@ -35,7 +35,7 @@ ConfigBase::serialize(const t_config_option_key opt_key) {
 
 void
 ConfigBase::set_deserialize(const t_config_option_key opt_key, std::string str) {
-    ConfigOption* opt = this->option(opt_key);
+    ConfigOption* opt = this->option(opt_key, true);
     assert(opt != NULL);
     opt->deserialize(str);
 }

xs/t/15_config.t

@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use Slic3r::XS;
-use Test::More tests => 76;
+use Test::More tests => 78;
 
 foreach my $config (Slic3r::Config->new, Slic3r::Config::Print->new) {
     $config->set('layer_height', 0.3);
@@ -56,11 +56,13 @@ foreach my $config (Slic3r::Config->new, Slic3r::Config::Print->new) {
     is_deeply $config->get('extruder_offset'), [[20,10]], 'deserialize points';
     
     # truncate ->get() to first decimal digit
-    $config->set('nozzle_diameter', [0.2,0.3]);
-    is_deeply [ map int($_*10)/10, @{$config->get('nozzle_diameter')} ], [0.2,0.3], 'set/get floats';
-    is $config->serialize('nozzle_diameter'), '0.2,0.3', 'serialize floats';
+    $config->set('nozzle_diameter', [0.2,3]);
+    is_deeply [ map int($_*10)/10, @{$config->get('nozzle_diameter')} ], [0.2,3], 'set/get floats';
+    is $config->serialize('nozzle_diameter'), '0.2,3', 'serialize floats';
     $config->set_deserialize('nozzle_diameter', '0.1,0.4');
     is_deeply [ map int($_*10)/10, @{$config->get('nozzle_diameter')} ], [0.1,0.4], 'deserialize floats';
+    $config->set_deserialize('nozzle_diameter', '3');
+    is_deeply [ map int($_*10)/10, @{$config->get('nozzle_diameter')} ], [3], 'deserialize a single float';
     
     $config->set('temperature', [180,210]);
     is_deeply $config->get('temperature'), [180,210], 'set/get ints';
@@ -87,6 +89,9 @@ foreach my $config (Slic3r::Config->new, Slic3r::Config::Print->new) {
     my $config = Slic3r::Config->new;
     $config->set('perimeters', 2);
     
+    # test that no crash happens when using set_deserialize() with a key that hasn't been set() yet
+    $config->set_deserialize('filament_diameter', '3');
+    
     my $config2 = Slic3r::Config::Print->new;
     $config2->apply_dynamic($config);
     is $config2->get('perimeters'), 2, 'apply_dynamic';