From 5c02bfd31018e55519b838e51903c3f3764a925e Mon Sep 17 00:00:00 2001 From: Alessandro Ranellucci Date: Sat, 11 Jan 2014 17:24:56 +0100 Subject: [PATCH] Bugfix: ambiguous semantics of the layers_count() method caused M73 to go beyond 100%. #1670 --- lib/Slic3r/GCode.pm | 4 +++- lib/Slic3r/Print.pm | 49 +++++++++++++++++++++----------------- lib/Slic3r/Print/Object.pm | 35 ++++++++++++++------------- t/gcode.t | 17 +++++++++++++ 4 files changed, 66 insertions(+), 39 deletions(-) diff --git a/lib/Slic3r/GCode.pm b/lib/Slic3r/GCode.pm index 4faf45a0..5bace196 100644 --- a/lib/Slic3r/GCode.pm +++ b/lib/Slic3r/GCode.pm @@ -15,6 +15,7 @@ has 'standby_points' => (is => 'rw'); has 'enable_loop_clipping' => (is => 'rw', default => sub {1}); has 'enable_wipe' => (is => 'lazy'); # at least one extruder has wipe enabled has 'layer_count' => (is => 'ro', required => 1 ); +has '_layer_index' => (is => 'rw', default => sub {-1}); # just a counter has 'layer' => (is => 'rw'); has '_layer_islands' => (is => 'rw'); has '_upper_layer_islands' => (is => 'rw'); @@ -92,6 +93,7 @@ sub change_layer { my ($self, $layer) = @_; $self->layer($layer); + $self->_layer_index($self->_layer_index + 1); # avoid computing islands and overhangs if they're not needed $self->_layer_islands($layer->islands); @@ -111,7 +113,7 @@ sub change_layer { my $gcode = ""; if ($self->config->gcode_flavor =~ /^(?:makerware|sailfish)$/) { $gcode .= sprintf "M73 P%s%s\n", - int(99 * ($layer->id / ($self->layer_count - 1))), + int(99 * ($self->_layer_index / ($self->layer_count - 1))), ($self->config->gcode_comments ? ' ; update progress' : ''); } if ($self->config->first_layer_acceleration) { diff --git a/lib/Slic3r/Print.pm b/lib/Slic3r/Print.pm index 94fcabb0..2b413360 100644 --- a/lib/Slic3r/Print.pm +++ b/lib/Slic3r/Print.pm @@ -283,9 +283,11 @@ sub init_extruders { } } +# this value is not supposed to be compared with $layer->id +# since they have different semantics sub layer_count { my $self = shift; - return max(map { scalar @{$_->layers} } @{$self->objects}); + return max(map $_->layer_count, @{$self->objects}); } sub regions_count { @@ -398,8 +400,9 @@ sub export_gcode { items => sub { my @items = (); # [obj_idx, layer_id] for my $obj_idx (0 .. $#{$self->objects}) { - for my $region_id (0 .. ($self->regions_count-1)) { - push @items, map [$obj_idx, $_, $region_id], 0..($self->objects->[$obj_idx]->layer_count-1); + my $object = $self->objects->[$obj_idx]; + for my $region_id (0 .. $#{$self->regions}) { + push @items, map [$obj_idx, $_, $region_id], 0..$#{$object->layers}; } } @items; @@ -407,9 +410,9 @@ sub export_gcode { thread_cb => sub { my $q = shift; while (defined (my $obj_layer = $q->dequeue)) { - my ($obj_idx, $layer_id, $region_id) = @$obj_layer; + my ($obj_idx, $layer_i, $region_id) = @$obj_layer; my $object = $self->objects->[$obj_idx]; - my $layerm = $object->layers->[$layer_id]->regions->[$region_id]; + my $layerm = $object->layers->[$layer_i]->regions->[$region_id]; $layerm->fills->append( $object->fill_maker->make_fill($layerm) ); } }, @@ -517,29 +520,31 @@ EOF ($type eq 'contour' ? 'white' : 'black'); }; + my @layers = sort { $a->print_z <=> $b->print_z } + map { @{$_->layers}, @{$_->support_layers} } + @{$self->objects}; + + my $layer_id = -1; my @previous_layer_slices = (); - for my $layer_id (0..$self->layer_count-1) { - my @layers = map $_->layers->[$layer_id], @{$self->objects}; - printf $fh qq{ \n}, $layer_id, unscale +(grep defined $_, @layers)[0]->slice_z; + for my $layer (@layers) { + $layer_id++; + # TODO: remove slic3r:z for raft layers + printf $fh qq{ \n}, $layer_id, unscale($layer->slice_z); my @current_layer_slices = (); - for my $obj_idx (0 .. $#{$self->objects}) { - my $layer = $self->objects->[$obj_idx]->layers->[$layer_id] or next; - - # sort slices so that the outermost ones come first - my @slices = sort { $a->contour->encloses_point($b->contour->[0]) ? 0 : 1 } @{$layer->slices}; - foreach my $copy (@{$self->objects->[$obj_idx]->copies}) { - foreach my $slice (@slices) { - my $expolygon = $slice->clone; - $expolygon->translate(@$copy); - $print_polygon->($expolygon->contour, 'contour'); - $print_polygon->($_, 'hole') for @{$expolygon->holes}; - push @current_layer_slices, $expolygon; - } + # sort slices so that the outermost ones come first + my @slices = sort { $a->contour->encloses_point($b->contour->[0]) ? 0 : 1 } @{$layer->slices}; + foreach my $copy (@{$layer->object->copies}) { + foreach my $slice (@slices) { + my $expolygon = $slice->clone; + $expolygon->translate(@$copy); + $print_polygon->($expolygon->contour, 'contour'); + $print_polygon->($_, 'hole') for @{$expolygon->holes}; + push @current_layer_slices, $expolygon; } } # generate support material - if ($self->has_support_material && $layer_id > 0) { + if ($self->has_support_material && $layer->id > 0) { my (@supported_slices, @unsupported_slices) = (); foreach my $expolygon (@current_layer_slices) { my $intersection = intersection_ex( diff --git a/lib/Slic3r/Print/Object.pm b/lib/Slic3r/Print/Object.pm index 35149667..7c0f41f3 100644 --- a/lib/Slic3r/Print/Object.pm +++ b/lib/Slic3r/Print/Object.pm @@ -93,9 +93,12 @@ sub init_config { $self->config(Slic3r::Config->merge($self->print->config, $self->config_overrides)); } +# this is the *total* layer count +# this value is not supposed to be compared with $layer->id +# since they have different semantics sub layer_count { my $self = shift; - return scalar @{ $self->layers }; + return scalar @{ $self->layers } + scalar @{ $self->support_layers }; } sub bounding_box { @@ -219,9 +222,10 @@ sub make_perimeters { # hollow objects if ($self->config->extra_perimeters && $self->config->perimeters > 0 && $self->config->fill_density > 0) { for my $region_id (0 .. ($self->print->regions_count-1)) { - for my $layer_id (0 .. $self->layer_count-2) { - my $layerm = $self->layers->[$layer_id]->regions->[$region_id]; - my $upper_layerm = $self->layers->[$layer_id+1]->regions->[$region_id]; + for my $i (0 .. $#{$self->layers}-1) { + # remember that $i != $layer->id + my $layerm = $self->layers->[$i]->regions->[$region_id]; + my $upper_layerm = $self->layers->[$i+1]->regions->[$region_id]; my $perimeter_spacing = $layerm->perimeter_flow->scaled_spacing; my $overlap = $perimeter_spacing; # one perimeter @@ -258,7 +262,7 @@ sub make_perimeters { # only add the perimeter if there's an intersection with the collapsed area last CYCLE if !@{ intersection($diff, $hypothetical_perimeter) }; - Slic3r::debugf " adding one more perimeter at layer %d\n", $layer_id; + Slic3r::debugf " adding one more perimeter at layer %d\n", $layerm->id; $slice->extra_perimeters($extra_perimeters); } } @@ -267,11 +271,11 @@ sub make_perimeters { } Slic3r::parallelize( - items => sub { 0 .. ($self->layer_count-1) }, + items => sub { 0 .. $#{$self->layers} }, thread_cb => sub { my $q = shift; - while (defined (my $layer_id = $q->dequeue)) { - $self->layers->[$layer_id]->make_perimeters; + while (defined (my $i = $q->dequeue)) { + $self->layers->[$i]->make_perimeters; } }, collect_cb => sub {}, @@ -286,7 +290,7 @@ sub detect_surfaces_type { Slic3r::debugf "Detecting solid surfaces...\n"; for my $region_id (0 .. ($self->print->regions_count-1)) { - for my $i (0 .. ($self->layer_count-1)) { + for my $i (0 .. $#{$self->layers}) { my $layerm = $self->layers->[$i]->regions->[$region_id]; # prepare a reusable subroutine to make surface differences @@ -516,8 +520,8 @@ sub process_external_surfaces { for my $region_id (0 .. ($self->print->regions_count-1)) { $self->layers->[0]->regions->[$region_id]->process_external_surfaces(undef); - for my $layer_id (1 .. ($self->layer_count-1)) { - $self->layers->[$layer_id]->regions->[$region_id]->process_external_surfaces($self->layers->[$layer_id-1]); + for my $i (1 .. $#{$self->layers}) { + $self->layers->[$i]->regions->[$region_id]->process_external_surfaces($self->layers->[$i-1]); } } } @@ -528,7 +532,7 @@ sub discover_horizontal_shells { Slic3r::debugf "==> DISCOVERING HORIZONTAL SHELLS\n"; for my $region_id (0 .. ($self->print->regions_count-1)) { - for (my $i = 0; $i < $self->layer_count; $i++) { + for (my $i = 0; $i <= $#{$self->layers}; $i++) { my $layerm = $self->layers->[$i]->regions->[$region_id]; if ($self->config->solid_infill_every_layers && $self->config->fill_density > 0 @@ -560,7 +564,7 @@ sub discover_horizontal_shells { abs($n - $i) <= $solid_layers-1; ($type == S_TYPE_TOP) ? $n-- : $n++) { - next if $n < 0 || $n >= $self->layer_count; + next if $n < 0 || $n > $#{$self->layers}; Slic3r::debugf " looking for neighbors on layer %d...\n", $n; my $neighbor_fill_surfaces = $self->layers->[$n]->regions->[$region_id]->fill_surfaces; @@ -666,8 +670,7 @@ sub combine_infill { return unless $self->config->infill_every_layers > 1 && $self->config->fill_density > 0; my $every = $self->config->infill_every_layers; - my $layer_count = $self->layer_count; - my @layer_heights = map $self->layers->[$_]->height, 0 .. $layer_count-1; + my @layer_heights = map $_->height, @{$self->layers}; for my $region_id (0 .. ($self->print->regions_count-1)) { # limit the number of combined layers to the maximum height allowed by this regions' nozzle @@ -774,7 +777,7 @@ sub combine_infill { sub generate_support_material { my $self = shift; return unless ($self->config->support_material || $self->config->raft_layers > 0) - && $self->layer_count >= 2; + && scalar(@{$self->layers}) >= 2; Slic3r::Print::SupportMaterial ->new(config => $self->config, flow => $self->print->support_material_flow) diff --git a/t/gcode.t b/t/gcode.t index 4d0ed149..e68b899a 100644 --- a/t/gcode.t +++ b/t/gcode.t @@ -83,4 +83,21 @@ use Slic3r::Test; ok $print->extruders->[0]->absolute_E + $config->retract_length->[0] > 0, 'total filament length is positive'; } +{ + my $config = Slic3r::Config->new_from_defaults; + $config->set('gcode_flavor', 'sailfish'); + $config->set('raft_layers', 3); + my $print = Slic3r::Test::init_print('20mm_cube', config => $config); + my @percent = (); + Slic3r::GCode::Reader->new->parse(Slic3r::Test::gcode($print), sub { + my ($self, $cmd, $args, $info) = @_; + + if ($cmd eq 'M73') { + push @percent, $args->{P}; + } + }); + # the extruder heater is turned off when M73 P100 is reached + ok !(defined first { $_ > 100 } @percent), 'M73 is never given more than 100%'; +} + __END__