From e02ae0d18a89dd83d2bced50b06ffd89eae4cf5f Mon Sep 17 00:00:00 2001 From: Alessandro Ranellucci Date: Fri, 6 Sep 2013 18:36:38 +0200 Subject: [PATCH] Return Surface objects by reference from SurfaceCollection objects and fix a bug in XS code causing some shell options to be ignored --- lib/Slic3r/Fill.pm | 11 +++-------- lib/Slic3r/Layer/Region.pm | 32 ++++++++++++-------------------- lib/Slic3r/Print/Object.pm | 24 ++++++++++-------------- xs/lib/Slic3r/XS.pm | 5 +++++ xs/src/Surface.cpp | 18 ++++++++++++++++++ xs/src/Surface.hpp | 2 ++ xs/t/05_surface.t | 5 +++-- xs/xsp/Surface.xsp | 1 + xs/xsp/SurfaceCollection.xsp | 17 ++++++++++++++--- 9 files changed, 68 insertions(+), 47 deletions(-) create mode 100644 xs/src/Surface.cpp diff --git a/lib/Slic3r/Fill.pm b/lib/Slic3r/Fill.pm index 2e3503880..a26646aac 100644 --- a/lib/Slic3r/Fill.pm +++ b/lib/Slic3r/Fill.pm @@ -54,20 +54,15 @@ sub make_fill { my @surfaces = (); - # if hollow object is requested, remove internal surfaces - # (this needs to be done after internal-solid shells are created) - if ($fill_density == 0) { - @surfaces = grep $_->surface_type != S_TYPE_INTERNAL, @surfaces; - } - # merge adjacent surfaces # in case of bridge surfaces, the ones with defined angle will be attached to the ones # without any angle (shouldn't this logic be moved to process_external_surfaces()?) { - my @surfaces_with_bridge_angle = grep defined $_->bridge_angle, @{$layerm->fill_surfaces}; + my @fill_surfaces = @{$layerm->fill_surfaces}; + my @surfaces_with_bridge_angle = grep defined $_->bridge_angle, @fill_surfaces; # give priority to bridges - my @groups = Slic3r::Surface->group({merge_solid => 1}, @{$layerm->fill_surfaces}); + my @groups = Slic3r::Surface->group({merge_solid => 1}, @fill_surfaces); @groups = sort { defined $a->[0]->bridge_angle ? -1 : 0 } @groups; foreach my $group (@groups) { diff --git a/lib/Slic3r/Layer/Region.pm b/lib/Slic3r/Layer/Region.pm index 263eeca73..75b62f486 100644 --- a/lib/Slic3r/Layer/Region.pm +++ b/lib/Slic3r/Layer/Region.pm @@ -412,39 +412,30 @@ sub _fill_gaps { sub prepare_fill_surfaces { my $self = shift; - my @surfaces = @{$self->fill_surfaces}; - # if no solid layers are requested, turn top/bottom surfaces to internal if ($self->config->top_solid_layers == 0) { - for my $i (0..$#surfaces) { - next unless $surfaces[$i]->surface_type == S_TYPE_TOP; - $self->fill_surfaces->set_surface_type($i, S_TYPE_INTERNAL); - } + $_->surface_type(S_TYPE_INTERNAL) for @{$self->fill_surfaces->filter_by_type(S_TYPE_TOP)}; } if ($self->config->bottom_solid_layers == 0) { - for my $i (0..$#surfaces) { - next unless $surfaces[$i]->surface_type == S_TYPE_BOTTOM; - $self->fill_surfaces->set_surface_type($i, S_TYPE_INTERNAL); - } + $_->surface_type(S_TYPE_INTERNAL) for @{$self->fill_surfaces->filter_by_type(S_TYPE_BOTTOM)}; } # turn too small internal regions into solid regions according to the user setting if ($self->config->fill_density > 0) { my $min_area = scale scale $self->config->solid_infill_below_area; # scaling an area requires two calls! - for my $i (0..$#surfaces) { - next unless $surfaces[$i]->surface_type == S_TYPE_INTERNAL && $surfaces[$i]->expolygon->contour->area <= $min_area; - $self->fill_surfaces->set_surface_type($i, S_TYPE_INTERNALSOLID); - } + $_->surface_type(S_TYPE_INTERNALSOLID) + for grep { $_->area <= $min_area } @{$self->fill_surfaces->filter_by_type(S_TYPE_INTERNAL)}; } } sub process_external_surfaces { my $self = shift; + my @surfaces = @{$self->fill_surfaces}; my $margin = scale &Slic3r::EXTERNAL_INFILL_MARGIN; my @bottom = (); - foreach my $surface (grep $_->surface_type == S_TYPE_BOTTOM, @{$self->fill_surfaces}) { + foreach my $surface (grep $_->surface_type == S_TYPE_BOTTOM, @surfaces) { my $grown = $surface->expolygon->offset_ex(+$margin); # detect bridge direction before merging grown surfaces otherwise adjacent bridges @@ -459,7 +450,7 @@ sub process_external_surfaces { } my @top = (); - foreach my $surface (grep $_->surface_type == S_TYPE_TOP, @{$self->fill_surfaces}) { + foreach my $surface (grep $_->surface_type == S_TYPE_TOP, @surfaces) { # give priority to bottom surfaces my $grown = diff_ex( $surface->expolygon->offset(+$margin), @@ -471,8 +462,8 @@ sub process_external_surfaces { # if we're slicing with no infill, we can't extend external surfaces # over non-existent infill my @fill_boundaries = $self->object->config->fill_density > 0 - ? @{$self->fill_surfaces} - : grep $_->surface_type != S_TYPE_INTERNAL, @{$self->fill_surfaces}; + ? @surfaces + : grep $_->surface_type != S_TYPE_INTERNAL, @surfaces; # intersect the grown surfaces with the actual fill boundaries my @new_surfaces = (); @@ -487,14 +478,15 @@ sub process_external_surfaces { } # subtract the new top surfaces from the other non-top surfaces and re-add them - my @other = grep $_->surface_type != S_TYPE_TOP && $_->surface_type != S_TYPE_BOTTOM, @{$self->fill_surfaces}; + my @other = grep $_->surface_type != S_TYPE_TOP && $_->surface_type != S_TYPE_BOTTOM, @surfaces; foreach my $group (Slic3r::Surface->group(@other)) { push @new_surfaces, map $group->[0]->clone(expolygon => $_), @{diff_ex( [ map $_->p, @$group ], [ map $_->p, @new_surfaces ], )}; } - @{$self->fill_surfaces} = @new_surfaces; + $self->fill_surfaces->clear; + $self->fill_surfaces->append(@new_surfaces); } sub _detect_bridge_direction { diff --git a/lib/Slic3r/Print/Object.pm b/lib/Slic3r/Print/Object.pm index de3c2d87a..c27d90457 100644 --- a/lib/Slic3r/Print/Object.pm +++ b/lib/Slic3r/Print/Object.pm @@ -464,11 +464,11 @@ sub clip_fill_surfaces { ), @{intersection_ex( [ map @$_, @overhangs ], - [ map @{$_->expolygon}, grep $_->surface_type == S_TYPE_INTERNAL, @{$layerm->fill_surfaces} ], + [ map $_->p, @{$layerm->fill_surfaces->filter_by_type(S_TYPE_INTERNAL)} ], )}; my @new_surfaces = ( @new_internal, - (map $_->clone, grep $_->surface_type != S_TYPE_INTERNAL, @{$layerm->fill_surfaces}), + (map $_->clone, @{$layerm->fill_surfaces->filter_by_type(S_TYPE_INTERNAL)}), ); $layerm->fill_surfaces->clear; $layerm->fill_surfaces->append(@new_surfaces); @@ -502,8 +502,8 @@ sub bridge_over_infill { foreach my $layerm (@{$layer->regions}) { # compute the areas needing bridge math - my @internal_solid = grep $_->surface_type == S_TYPE_INTERNALSOLID, @{$layerm->fill_surfaces}; - my @lower_internal = grep $_->surface_type == S_TYPE_INTERNAL, map @{$_->fill_surfaces}, @{$lower_layer->regions}; + my @internal_solid = @{$layerm->fill_surfaces->filter_by_type(S_TYPE_INTERNALSOLID)}; + my @lower_internal = map @{$_->fill_surfaces->filter_by_type(S_TYPE_INTERNAL)}, @{$lower_layer->regions}; my $to_bridge = intersection_ex( [ map $_->p, @internal_solid ], [ map $_->p, @lower_internal ], @@ -578,11 +578,7 @@ sub discover_horizontal_shells { if ($self->config->solid_infill_every_layers && $self->config->fill_density > 0 && ($i % $self->config->solid_infill_every_layers) == 0) { - my @surfaces = @{$layerm->fill_surfaces}; - for my $i (0..$#surfaces) { - next unless $surfaces[$i]->surface_type == S_TYPE_INTERNAL; - $layerm->fill_surfaces->set_surface_type($i, S_TYPE_INTERNALSOLID); - } + $_->surface_type(S_TYPE_INTERNALSOLID) for @{$layerm->fill_surfaces->filter_by_type(S_TYPE_INTERNAL)}; } EXTERNAL: foreach my $type (S_TYPE_TOP, S_TYPE_BOTTOM) { @@ -593,7 +589,7 @@ sub discover_horizontal_shells { # not work in some situations, as there won't be any grown region in the perimeter # area (this was seen in a model where the top layer had one extra perimeter, thus # its fill_surfaces was thinner than the lower layer's infill) - my $solid = offset_ex([ map $_->p, grep $_->surface_type == $type, @{$layerm->slices} ], $margin); + my $solid = offset_ex([ map $_->p, @{$layerm->slices->filter_by_type($type)} ], $margin); next if !@$solid; Slic3r::debugf "Layer %d has %s surfaces\n", $i, ($type == S_TYPE_TOP) ? 'top' : 'bottom'; @@ -745,13 +741,13 @@ sub combine_infill { # we need to perform a multi-layer intersection, so let's split it in pairs # initialize the intersection with the candidates of the lowest layer - my $intersection = [ map $_->expolygon, grep $_->surface_type == $type, @{$layerms[0]->fill_surfaces} ]; + my $intersection = [ map $_->expolygon, @{$layerms[0]->fill_surfaces->filter_by_type($type)} ]; # start looping from the second layer and intersect the current intersection with it for my $layerm (@layerms[1 .. $#layerms]) { $intersection = intersection_ex( [ map @$_, @$intersection ], - [ map @{$_->expolygon}, grep $_->surface_type == $type, @{$layerm->fill_surfaces} ], + [ map @{$_->expolygon}, @{$layerm->fill_surfaces->filter_by_type($type)} ], ); } @@ -778,12 +774,12 @@ sub combine_infill { foreach my $layerm (@layerms) { - my @this_type = grep $_->surface_type == $type, @{$layerm->fill_surfaces}; + my @this_type = @{$layerm->fill_surfaces->filter_by_type($type)}; my @other_types = map $_->clone, grep $_->surface_type != $type, @{$layerm->fill_surfaces}; my @new_this_type = map Slic3r::Surface->new(expolygon => $_, surface_type => $type), @{diff_ex( - [ map @{$_->expolygon}, @this_type ], + [ map $_->p, @this_type ], [ @intersection_with_clearance ], )}; diff --git a/xs/lib/Slic3r/XS.pm b/xs/lib/Slic3r/XS.pm index c363d5a8b..5cac751b4 100644 --- a/xs/lib/Slic3r/XS.pm +++ b/xs/lib/Slic3r/XS.pm @@ -181,6 +181,11 @@ sub clone { ); } +package Slic3r::Surface::Ref; +our @ISA = 'Slic3r::Surface'; + +sub DESTROY {} + package Slic3r::Surface::Collection; use overload '@{}' => sub { $_[0]->arrayref }, diff --git a/xs/src/Surface.cpp b/xs/src/Surface.cpp new file mode 100644 index 000000000..f5b2c865c --- /dev/null +++ b/xs/src/Surface.cpp @@ -0,0 +1,18 @@ +#include "Surface.hpp" + +namespace Slic3r { + +SV* +Surface::to_SV_ref() { + SV* sv = newSV(0); + sv_setref_pv( sv, "Slic3r::Surface::Ref", (void*)this ); + return sv; +} + +double +Surface::area() const +{ + return this->expolygon.area(); +} + +} diff --git a/xs/src/Surface.hpp b/xs/src/Surface.hpp index 14450e1bb..b4325fb8d 100644 --- a/xs/src/Surface.hpp +++ b/xs/src/Surface.hpp @@ -16,6 +16,8 @@ class Surface unsigned short thickness_layers; // in layers double bridge_angle; unsigned short extra_perimeters; + SV* to_SV_ref(); + double area() const; }; typedef std::vector Surfaces; diff --git a/xs/t/05_surface.t b/xs/t/05_surface.t index dae46f9b0..4a3689bee 100644 --- a/xs/t/05_surface.t +++ b/xs/t/05_surface.t @@ -4,7 +4,7 @@ use strict; use warnings; use Slic3r::XS; -use Test::More tests => 13; +use Test::More tests => 14; my $square = [ # ccw [100, 100], @@ -58,8 +58,9 @@ is $surface->extra_perimeters, 2, 'extra_perimeters'; is scalar(@$collection), 1, 'append to collection'; my $item = $collection->[0]; + isa_ok $item, 'Slic3r::Surface::Ref'; $item->surface_type(Slic3r::Surface::S_TYPE_INTERNAL); - isnt $item->surface_type, $collection->[0]->surface_type, 'collection returns copies of items'; + is $item->surface_type, $collection->[0]->surface_type, 'collection returns items by reference'; } __END__ diff --git a/xs/xsp/Surface.xsp b/xs/xsp/Surface.xsp index 73bca3c3e..787db562f 100644 --- a/xs/xsp/Surface.xsp +++ b/xs/xsp/Surface.xsp @@ -13,6 +13,7 @@ %code{% RETVAL = THIS->thickness; %}; unsigned short thickness_layers() %code{% RETVAL = THIS->thickness_layers; %}; + double area(); %{ Surface* diff --git a/xs/xsp/SurfaceCollection.xsp b/xs/xsp/SurfaceCollection.xsp index 52131ec6d..512b06f09 100644 --- a/xs/xsp/SurfaceCollection.xsp +++ b/xs/xsp/SurfaceCollection.xsp @@ -31,9 +31,19 @@ SurfaceCollection::arrayref() av_fill(av, THIS->surfaces.size()-1); int i = 0; for (Surfaces::iterator it = THIS->surfaces.begin(); it != THIS->surfaces.end(); ++it) { - SV* sv = newSV(0); - sv_setref_pv( sv, "Slic3r::Surface", new Surface(*it) ); - av_store(av, i++, sv); + av_store(av, i++, (*it).to_SV_ref()); + } + RETVAL = newRV_noinc((SV*)av); + OUTPUT: + RETVAL + +SV* +SurfaceCollection::filter_by_type(surface_type) + SurfaceType surface_type; + CODE: + AV* av = newAV(); + for (Surfaces::iterator it = THIS->surfaces.begin(); it != THIS->surfaces.end(); ++it) { + if ((*it).surface_type == surface_type) av_push(av, (*it).to_SV_ref()); } RETVAL = newRV_noinc((SV*)av); OUTPUT: @@ -43,6 +53,7 @@ void SurfaceCollection::append(...) CODE: for (unsigned int i = 1; i < items; i++) { + // Note: a COPY of the input is stored THIS->surfaces.push_back(*(Surface *)SvIV((SV*)SvRV( ST(i) ))); }