Make combine_infill() completely idempotent. Includes unit testing

lib/Slic3r/Print/Object.pm

@@ -678,7 +678,8 @@ sub detect_surfaces_type {
             
             # Note: this method should be idempotent, but fill_surfaces gets modified 
             # in place. However we're now only using its boundaries (which are invariant)
-            # so we're safe
+            # so we're safe. This guarantees idempotence of prepare_infill() also in case
+            # that combine_infill() turns some fill_surface into VOID surfaces.
             my $fill_boundaries = [ map $_->clone->p, @{$layerm->fill_surfaces} ];
             $layerm->fill_surfaces->clear;
             foreach my $surface (@{$layerm->slices}) {
@@ -988,12 +989,19 @@ sub discover_horizontal_shells {
 }
 
 # combine fill surfaces across layers
+# Idempotence of this method is guaranteed by the fact that we don't remove things from
+# fill_surfaces but we only turn them into VOID surfaces, thus preserving the boundaries.
 sub combine_infill {
     my $self = shift;
     
+    # define the type used for voids
+    my %voidtype = (
+        &S_TYPE_INTERNAL() => S_TYPE_INTERNALVOID,
+    );
+    
     # work on each region separately
     for my $region_id (0 .. ($self->print->region_count-1)) {
-        my $region = $self->print->regions->[$region_id];
+        my $region = $self->print->get_region($region_id);
         my $every = $region->config->infill_every_layers;
         next unless $every > 1 && $region->config->fill_density > 0;
         
@@ -1030,7 +1038,7 @@ sub combine_infill {
             next unless $combine{$layer_idx} > 1;
             
             # get all the LayerRegion objects to be combined
-            my @layerms = map $self->get_layer($_)->regions->[$region_id],
+            my @layerms = map $self->get_layer($_)->get_region($region_id),
                 ($layer_idx - ($combine{$layer_idx}-1) .. $layer_idx);
             
             # only combine internal infill
@@ -1092,8 +1100,8 @@ sub combine_infill {
                             @$intersection;
                     } else {
                         # save void surfaces
-                        push @this_type,
+                        push @new_this_type,
-                            map Slic3r::Surface->new(expolygon => $_, surface_type => S_TYPE_INTERNALVOID),
+                            map Slic3r::Surface->new(expolygon => $_, surface_type => $voidtype{$type}),
                             @{intersection_ex(
                                 [ map @{$_->expolygon}, @this_type ],
                                 [ @intersection_with_clearance ],

t/combineinfill.t

@@ -9,9 +9,10 @@ BEGIN {
 
 use List::Util qw(first);
 use Slic3r;
+use Slic3r::Surface ':types';
 use Slic3r::Test;
 
-plan tests => 6;
+plan tests => 8;
 
 {
     my $test = sub {
@@ -53,7 +54,6 @@ plan tests => 6;
     };
     
     my $config = Slic3r::Config->new_from_defaults;
-    $config->set('gcode_comments', 1);
     $config->set('layer_height', 0.2);
     $config->set('first_layer_height', 0.2);
     $config->set('nozzle_diameter', [0.5]);
@@ -71,6 +71,30 @@ plan tests => 6;
     $test->($config);
 }
 
+{
+    my $config = Slic3r::Config->new_from_defaults;
+    $config->set('layer_height', 0.2);
+    $config->set('first_layer_height', 0.2);
+    $config->set('nozzle_diameter', [0.5]);
+    $config->set('infill_every_layers', 2);
+    
+    my $print = Slic3r::Test::init_print('20mm_cube', config => $config);
+    $print->process;
+    
+    ok defined(first { @{$_->get_region(0)->fill_surfaces->filter_by_type(S_TYPE_INTERNALVOID)} > 0 }
+        @{$print->print->get_object(0)->layers}),
+        'infill combination produces internal void surfaces';
+    
+    # we disable combination after infill has been generated
+    $config->set('infill_every_layers', 1);
+    $print->apply_config($config);
+    $print->process;
+    
+    ok !(defined first { @{$_->get_region(0)->fill_surfaces} == 0 }
+        @{$print->print->get_object(0)->layers}),
+            'infill combination is idempotent';
+}
+
 # the following needs to be adapted to the new API
 if (0) {
     my $config = Slic3r::Config->new_from_defaults;

xs/src/libslic3r/PrintObject.cpp

@@ -249,6 +249,8 @@ PrintObject::invalidate_state_by_config_options(const std::vector<t_config_optio
             steps.insert(posSupportMaterial);
         } else if (*opt_key == "interface_shells"
             || *opt_key == "infill_only_where_needed"
+            || *opt_key == "infill_every_layers"
+            || *opt_key == "solid_infill_every_layers"
             || *opt_key == "bottom_solid_layers"
             || *opt_key == "top_solid_layers"
             || *opt_key == "solid_infill_below_area"
@@ -258,8 +260,6 @@ PrintObject::invalidate_state_by_config_options(const std::vector<t_config_optio
         } else if (*opt_key == "external_fill_pattern"
             || *opt_key == "fill_angle"
             || *opt_key == "fill_pattern"
-            || *opt_key == "infill_every_layers"
-            || *opt_key == "solid_infill_every_layers"
             || *opt_key == "top_infill_extrusion_width"
             || *opt_key == "first_layer_extrusion_width") {
             steps.insert(posInfill);