diff --git a/lib/Slic3r/GCode.pm b/lib/Slic3r/GCode.pm index 0249b5be3..c3bc2dfb4 100644 --- a/lib/Slic3r/GCode.pm +++ b/lib/Slic3r/GCode.pm @@ -207,8 +207,11 @@ sub extrude_loop { my $last_path_polyline = $paths[-1]->polyline; # detect angle between last and first segment # the side depends on the original winding order of the polygon (left for contours, right for holes) - my @points = $was_clockwise ? (-2, 1) : (1, -2); - my $angle = Slic3r::Geometry::angle3points(@$last_path_polyline[0, @points]) / 3; + my @points = ($paths[0][1], $paths[-1][-2]); + @points = reverse @points if $was_clockwise; + my $angle = $paths[0]->first_point->ccw_angle(@points) / 3; + + # turn left if contour, turn right if hole $angle *= -1 if $was_clockwise; # create the destination point along the first segment and rotate it diff --git a/lib/Slic3r/Layer/PerimeterGenerator.pm b/lib/Slic3r/Layer/PerimeterGenerator.pm index 2feffb7df..90893d535 100644 --- a/lib/Slic3r/Layer/PerimeterGenerator.pm +++ b/lib/Slic3r/Layer/PerimeterGenerator.pm @@ -8,7 +8,7 @@ use Slic3r::Geometry::Clipper qw(union_ex diff diff_ex intersection_ex offset of offset_ex offset2_ex union_pt intersection_ppl diff_ppl); use Slic3r::Surface ':types'; -has 'slices' => (is => 'ro', required => 1); +has 'slices' => (is => 'ro', required => 1); # SurfaceCollection has 'lower_slices' => (is => 'ro', required => 0); has 'layer_height' => (is => 'ro', required => 1); has 'layer_id' => (is => 'ro', required => 0, default => sub { -1 }); @@ -16,9 +16,9 @@ has 'perimeter_flow' => (is => 'ro', required => 1); has 'ext_perimeter_flow' => (is => 'ro', required => 1); has 'overhang_flow' => (is => 'ro', required => 1); has 'solid_infill_flow' => (is => 'ro', required => 1); -has 'config' => (is => 'ro', default => sub { Slic3r::Config::Region->new }); +has 'config' => (is => 'ro', default => sub { Slic3r::Config::PrintRegion->new }); has 'print_config' => (is => 'ro', default => sub { Slic3r::Config::Print->new }); -has '_lower_slices_p' => (is => 'rw'); +has '_lower_slices_p' => (is => 'rw', default => sub { [] }); has '_holes_pt' => (is => 'rw'); has '_ext_mm3_per_mm' => (is => 'rw'); has '_mm3_per_mm' => (is => 'rw'); @@ -34,6 +34,19 @@ has 'gap_fill' => (is => 'ro', default => sub { Slic3r::ExtrusionPath::Coll # generated fill surfaces will be put here has 'fill_surfaces' => (is => 'ro', default => sub { Slic3r::Surface::Collection->new }); +sub BUILDARGS { + my ($class, %args) = @_; + + if (my $flow = delete $args{flow}) { + $args{perimeter_flow} //= $flow; + $args{ext_perimeter_flow} //= $flow; + $args{overhang_flow} //= $flow; + $args{solid_infill_flow} //= $flow; + } + + return { %args }; +} + sub process { my ($self) = @_; @@ -62,13 +75,25 @@ sub process { my $min_spacing = $pspacing * (1 - &Slic3r::INSET_OVERLAP_TOLERANCE); my $ext_min_spacing = $ext_pspacing * (1 - &Slic3r::INSET_OVERLAP_TOLERANCE); - my @contours = (); # array of Polygons with ccw orientation - my @holes = (); # array of Polygons with cw orientation - my @thin_walls = (); # array of ExPolygons + # prepare grown lower layer slices for overhang detection + if ($self->lower_slices && $self->config->overhangs) { + # We consider overhang any part where the entire nozzle diameter is not supported by the + # lower layer, so we take lower slices and offset them by half the nozzle diameter used + # in the current layer + my $nozzle_diameter = $self->print_config->get_at('nozzle_diameter', $self->config->perimeter_extruder-1); + + $self->_lower_slices_p( + offset([ map @$_, @{$self->lower_slices} ], scale +$nozzle_diameter/2) + ); + } # we need to process each island separately because we might have different # extra perimeters for each one foreach my $surface (@{$self->slices}) { + my @contours = (); # array of Polygons with ccw orientation + my @holes = (); # array of Polygons with cw orientation + my @thin_walls = (); # array of ExPolygons + # detect how many perimeters must be generated for this island my $loop_number = $self->config->perimeters + ($surface->extra_perimeters || 0); @@ -194,60 +219,48 @@ sub process { -($pspacing/2 + $min_perimeter_infill_spacing/2), +$min_perimeter_infill_spacing/2, )}; - } - # process thin walls by collapsing slices to single passes - if (@thin_walls) { - # the following offset2 ensures almost nothing in @thin_walls is narrower than $min_width - # (actually, something larger than that still may exist due to mitering or other causes) - my $min_width = $pwidth / 4; - @thin_walls = @{offset2_ex([ map @$_, @thin_walls ], -$min_width/2, +$min_width/2)}; + # process thin walls by collapsing slices to single passes + if (@thin_walls) { + # the following offset2 ensures almost nothing in @thin_walls is narrower than $min_width + # (actually, something larger than that still may exist due to mitering or other causes) + my $min_width = $pwidth / 4; + @thin_walls = @{offset2_ex([ map @$_, @thin_walls ], -$min_width/2, +$min_width/2)}; - # the maximum thickness of our thin wall area is equal to the minimum thickness of a single loop - $self->_thin_wall_polylines([ map @{$_->medial_axis($pwidth + $pspacing, $min_width)}, @thin_walls ]); - Slic3r::debugf " %d thin walls detected\n", scalar(@{$self->_thin_wall_polylines}) if $Slic3r::debug; + # the maximum thickness of our thin wall area is equal to the minimum thickness of a single loop + $self->_thin_wall_polylines([ map @{$_->medial_axis($pwidth + $pspacing, $min_width)}, @thin_walls ]); + Slic3r::debugf " %d thin walls detected\n", scalar(@{$self->_thin_wall_polylines}) if $Slic3r::debug; - if (0) { - require "Slic3r/SVG.pm"; - Slic3r::SVG::output( - "medial_axis.svg", - no_arrows => 1, - expolygons => \@thin_walls, - green_polylines => [ map $_->polygon->split_at_first_point, @{$self->perimeters} ], - red_polylines => $self->_thin_wall_polylines, - ); + if (0) { + require "Slic3r/SVG.pm"; + Slic3r::SVG::output( + "medial_axis.svg", + no_arrows => 1, + expolygons => \@thin_walls, + green_polylines => [ map $_->polygon->split_at_first_point, @{$self->perimeters} ], + red_polylines => $self->_thin_wall_polylines, + ); + } } + + # find nesting hierarchies separately for contours and holes + my $contours_pt = union_pt(\@contours); + $self->_holes_pt(union_pt(\@holes)); + + # order loops from inner to outer (in terms of object slices) + my @loops = $self->_traverse_pt($contours_pt, 0, 1); + + # if brim will be printed, reverse the order of perimeters so that + # we continue inwards after having finished the brim + # TODO: add test for perimeter order + @loops = reverse @loops + if $self->config->external_perimeters_first + || ($self->layer_id == 0 && $self->print_config->brim_width > 0); + + # append perimeters for this slice as a collection + $self->loops->append(Slic3r::ExtrusionPath::Collection->new(@loops)); } - - # find nesting hierarchies separately for contours and holes - my $contours_pt = union_pt(\@contours); - $self->_holes_pt(union_pt(\@holes)); - - # prepare grown lower layer slices for overhang detection - my $lower_slices = Slic3r::ExPolygon::Collection->new; - if ($self->lower_slices && $self->config->overhangs) { - # We consider overhang any part where the entire nozzle diameter is not supported by the - # lower layer, so we take lower slices and offset them by half the nozzle diameter used - # in the current layer - my $nozzle_diameter = $self->print_config->get_at('nozzle_diameter', $self->config->perimeter_extruder-1); - $lower_slices->append($_) - for @{offset_ex([ map @$_, @{$self->lower_slices} ], scale +$nozzle_diameter/2)}; - } - $self->_lower_slices_p($lower_slices->polygons); - - # order loops from inner to outer (in terms of object slices) - my @loops = $self->_traverse_pt($contours_pt, 0, 1); - - # if brim will be printed, reverse the order of perimeters so that - # we continue inwards after having finished the brim - # TODO: add test for perimeter order - @loops = reverse @loops - if $self->config->external_perimeters_first - || ($self->layer_id == 0 && $self->print_config->brim_width > 0); - - # append perimeters - $self->loops->append($_) for @loops; } sub _traverse_pt { @@ -328,9 +341,8 @@ sub _traverse_pt { # return ccw contours and cw holes # GCode.pm will convert all of them to ccw, but it needs to know # what the holes are in order to compute the correct inwards move - # We do this on the final Loop object instead of the polygon because - # overhang clipping might have reversed its order since Clipper does - # not preserve polyline orientation. + # We do this on the final Loop object because overhang clipping + # does not keep orientation. if ($is_contour) { $loop->make_counter_clockwise; } else { @@ -358,10 +370,13 @@ sub _traverse_pt { # use a nearest neighbor search to order these children # TODO: supply second argument to chained_path() too? - # (We used to skip this chiained_path() when $is_contour && + # (We used to skip this chained_path() when $is_contour && # $depth == 0 because slices are ordered at G_code export # time, but multiple top-level perimeters might belong to # the same slice actually, so that was a broken optimization.) + # We supply no_reverse = false because we want to permit reversal + # of thin walls, but we rely on the fact that loops will never + # be reversed anyway. my $sorted_collection = $collection->chained_path_indices(0); my @orig_indices = @{$sorted_collection->orig_indices}; @@ -407,7 +422,6 @@ sub _fill_gaps { 1, ); my @polylines = map @{$_->medial_axis($max, $min/2)}, @$this; - return if !@polylines; Slic3r::debugf " %d gaps filled with extrusion width = %s\n", scalar @$this, $w diff --git a/lib/Slic3r/Print/GCode.pm b/lib/Slic3r/Print/GCode.pm index da4d51416..fa9c919ef 100644 --- a/lib/Slic3r/Print/GCode.pm +++ b/lib/Slic3r/Print/GCode.pm @@ -424,17 +424,17 @@ sub process_layer { # process perimeters { my $extruder_id = $region->config->perimeter_extruder-1; - foreach my $perimeter (@{$layerm->perimeters}) { + foreach my $perimeter_coll (@{$layerm->perimeters}) { # init by_extruder item only if we actually use the extruder $by_extruder{$extruder_id} //= []; - # $perimeter is an ExtrusionLoop or ExtrusionPath object + # $perimeter_coll is an ExtrusionPath::Collection object representing a single slice for my $i (0 .. $#{$layer->slices}) { if ($i == $#{$layer->slices} - || $layer->slices->[$i]->contour->contains_point($perimeter->first_point)) { + || $layer->slices->[$i]->contour->contains_point($perimeter_coll->first_point)) { $by_extruder{$extruder_id}[$i] //= { perimeters => {} }; $by_extruder{$extruder_id}[$i]{perimeters}{$region_id} //= []; - push @{ $by_extruder{$extruder_id}[$i]{perimeters}{$region_id} }, $perimeter; + push @{ $by_extruder{$extruder_id}[$i]{perimeters}{$region_id} }, @$perimeter_coll; last; } } diff --git a/lib/Slic3r/Print/SupportMaterial.pm b/lib/Slic3r/Print/SupportMaterial.pm index 7e23a75c7..e431f3fb5 100644 --- a/lib/Slic3r/Print/SupportMaterial.pm +++ b/lib/Slic3r/Print/SupportMaterial.pm @@ -178,7 +178,7 @@ sub contact_area { # TODO: split_at_first_point() could split a bridge mid-way my @overhang_perimeters = map { $_->isa('Slic3r::ExtrusionLoop') ? $_->polygon->split_at_first_point : $_->polyline->clone } - @{$layerm->perimeters}; + map @$_, @{$layerm->perimeters}; # workaround for Clipper bug, see Slic3r::Polygon::clip_as_polyline() $_->[0]->translate(1,0) for @overhang_perimeters; diff --git a/t/perimeters.t b/t/perimeters.t index 49217fd82..e2a5bc344 100644 --- a/t/perimeters.t +++ b/t/perimeters.t @@ -1,4 +1,4 @@ -use Test::More tests => 11; +use Test::More tests => 29; use strict; use warnings; @@ -7,6 +7,8 @@ BEGIN { use lib "$FindBin::Bin/../lib"; } +use Slic3r::ExtrusionLoop ':roles'; +use Slic3r::ExtrusionPath ':roles'; use List::Util qw(first); use Slic3r; use Slic3r::Flow ':roles'; @@ -188,7 +190,7 @@ use Slic3r::Test; my $pflow = $layerm->flow(FLOW_ROLE_PERIMETER); my $iflow = $layerm->flow(FLOW_ROLE_INFILL); my $covered_by_perimeters = union_ex([ - (map @{$_->polygon->split_at_first_point->grow($pflow->scaled_width/2)}, @{$layerm->perimeters}), + (map @{$_->polygon->split_at_first_point->grow($pflow->scaled_width/2)}, map @$_, @{$layerm->perimeters}), ]); my $covered_by_infill = union_ex([ (map $_->p, @{$layerm->fill_surfaces}), @@ -285,4 +287,89 @@ use Slic3r::Test; $test->('small_dorito'); } +{ + my $flow = Slic3r::Flow->new( + width => 1, + height => 1, + nozzle_diameter => 1, + ); + + my $config = Slic3r::Config->new; + my $test = sub { + my ($expolygons, %expected) = @_; + + my $slices = Slic3r::Surface::Collection->new; + $slices->append(Slic3r::Surface->new( + surface_type => S_TYPE_INTERNAL, + expolygon => $_, + )) for @$expolygons; + + my $g = Slic3r::Layer::PerimeterGenerator->new( + # input: + layer_height => 1, + slices => $slices, + flow => $flow, + ); + $g->config->apply_dynamic($config); + $g->process; + + is scalar(@{$g->loops}), + scalar(@$expolygons), 'expected number of collections'; + ok !defined(first { !$_->isa('Slic3r::ExtrusionPath::Collection') } @{$g->loops}), + 'everything is returned as collections'; + is scalar(map @$_, @{$g->loops}), + $expected{total}, 'expected number of loops'; + is scalar(grep $_->role == EXTR_ROLE_EXTERNAL_PERIMETER, map @$_, map @$_, @{$g->loops}), + $expected{external}, 'expected number of external loops'; + is scalar(grep $_->role == EXTRL_ROLE_CONTOUR_INTERNAL_PERIMETER, map @$_, @{$g->loops}), + $expected{cinternal}, 'expected number of internal contour loops'; + is scalar(grep $_->polygon->is_counter_clockwise, map @$_, @{$g->loops}), + $expected{ccw}, 'expected number of ccw loops'; + + return $g; + }; + + $config->set('perimeters', 3); + $test->( + [ + Slic3r::ExPolygon->new( + Slic3r::Polygon->new_scale([0,0], [100,0], [100,100], [0,100]), + ), + ], + total => 3, + external => 1, + cinternal => 1, + ccw => 3, + ); + $test->( + [ + Slic3r::ExPolygon->new( + Slic3r::Polygon->new_scale([0,0], [100,0], [100,100], [0,100]), + Slic3r::Polygon->new_scale([40,40], [40,60], [60,60], [60,40]), + ), + ], + total => 6, + external => 2, + cinternal => 1, + ccw => 3, + ); + $test->( + [ + Slic3r::ExPolygon->new( + Slic3r::Polygon->new_scale([0,0], [200,0], [200,200], [0,200]), + Slic3r::Polygon->new_scale([20,20], [20,180], [180,180], [180,20]), + ), + # nested: + Slic3r::ExPolygon->new( + Slic3r::Polygon->new_scale([50,50], [150,50], [150,150], [50,150]), + Slic3r::Polygon->new_scale([80,80], [80,120], [120,120], [120,80]), + ), + ], + total => 4*3, + external => 4, + cinternal => 2, + ccw => 2*3, + ); +} + __END__ diff --git a/xs/src/libslic3r/ExtrusionEntity.hpp b/xs/src/libslic3r/ExtrusionEntity.hpp index 65a1aba59..c40291190 100644 --- a/xs/src/libslic3r/ExtrusionEntity.hpp +++ b/xs/src/libslic3r/ExtrusionEntity.hpp @@ -35,6 +35,9 @@ enum ExtrusionLoopRole { class ExtrusionEntity { public: + virtual bool is_loop() const { + return false; + }; virtual ExtrusionEntity* clone() const = 0; virtual ~ExtrusionEntity() {}; virtual void reverse() = 0; @@ -84,6 +87,9 @@ class ExtrusionLoop : public ExtrusionEntity ExtrusionLoopRole role; ExtrusionLoop(ExtrusionLoopRole role = elrDefault) : role(role) {}; + bool is_loop() const { + return true; + }; operator Polygon() const; ExtrusionLoop* clone() const; bool make_clockwise(); diff --git a/xs/src/libslic3r/ExtrusionEntityCollection.cpp b/xs/src/libslic3r/ExtrusionEntityCollection.cpp index a958e53cf..4e3c596cf 100644 --- a/xs/src/libslic3r/ExtrusionEntityCollection.cpp +++ b/xs/src/libslic3r/ExtrusionEntityCollection.cpp @@ -37,7 +37,9 @@ void ExtrusionEntityCollection::reverse() { for (ExtrusionEntitiesPtr::iterator it = this->entities.begin(); it != this->entities.end(); ++it) { - (*it)->reverse(); + // Don't reverse it if it's a loop, as it doesn't change anything in terms of elements ordering + // and caller might rely on winding order + if (!(*it)->is_loop()) (*it)->reverse(); } std::reverse(this->entities.begin(), this->entities.end()); } @@ -96,7 +98,8 @@ ExtrusionEntityCollection::chained_path_from(Point start_near, ExtrusionEntityCo int start_index = start_near.nearest_point_index(endpoints); int path_index = start_index/2; ExtrusionEntity* entity = my_paths.at(path_index); - if (start_index % 2 && !no_reverse) { + // never reverse loops, since it's pointless for chained path and callers might depend on orientation + if (start_index % 2 && !no_reverse && !entity->is_loop()) { entity->reverse(); } retval->entities.push_back(my_paths.at(path_index));