Many changes and fixes to remove leaks and return objects by reference

lib/Slic3r/GCode.pm

@@ -8,7 +8,7 @@ use Slic3r::Geometry::Clipper qw(union_ex);
 use Slic3r::Surface ':types';
 
 has 'config'             => (is => 'ro', required => 1);
-has 'extruders'          => (is => 'ro', default => sub {0}, required => 1);
+has 'extruders'          => (is => 'ro', required => 1);
 has 'multiple_extruders' => (is => 'lazy');
 has 'enable_loop_clipping' => (is => 'rw', default => sub {1});
 has 'enable_wipe'        => (is => 'lazy');   # at least one extruder has wipe enabled

lib/Slic3r/Layer/Region.pm

@@ -197,9 +197,10 @@ sub make_perimeters {
                 : $perimeter_spacing;
             
             my @offsets = @{offset2_ex(\@last, -1.5*$spacing,  +0.5*$spacing)};
-            my @contours_offsets    = map $_->contour, @offsets;
+            # clone polygons because these ExPolygons will go out of scope very soon
-            my @holes_offsets       = map @{$_->holes}, @offsets;
+            my @contours_offsets    = map $_->contour->clone, @offsets;
-            @offsets = (@contours_offsets, @holes_offsets);     # turn @offsets from ExPolygons to Polygons
+            my @holes_offsets       = map $_->clone, map @{$_->holes}, @offsets;
+            @offsets = map $_->clone, (@contours_offsets, @holes_offsets);     # turn @offsets from ExPolygons to Polygons
             
             # where offset2() collapses the expolygon, then there's no room for an inner loop
             # and we can extract the gap for later processing
@@ -555,13 +556,12 @@ sub _detect_bridge_direction {
         }
     } elsif (@edges) {
         # inset the bridge expolygon; we'll use this one to clip our test lines
-        my $inset = [ $expolygon->offset_ex($self->infill_flow->scaled_width) ];
+        my $inset = $expolygon->offset_ex($self->infill_flow->scaled_width);
         
         # detect anchors as intersection between our bridge expolygon and the lower slices
         my $anchors = intersection_ex(
             [ @$grown ],
             [ map @$_, @lower ],
-            undef,
             1,  # safety offset required to avoid Clipper from detecting empty intersection while Boost actually found some @edges
         );
         
@@ -584,7 +584,8 @@ sub _detect_bridge_direction {
             }
             
             # TODO: use a multi_polygon_multi_linestring_intersection() call
-            my @clipped_lines = map @{ Boost::Geometry::Utils::polygon_multi_linestring_intersection($_, \@lines) }, @$inset;
+            my @clipped_lines = map Slic3r::Line->new(@$_),
+                map @{ Boost::Geometry::Utils::polygon_multi_linestring_intersection($_->pp, \@lines) }, @$inset;
             
             # remove any line not having both endpoints within anchors
             @clipped_lines = grep {

lib/Slic3r/Polygon.pm

@@ -19,7 +19,7 @@ sub merge_continuous_lines {
     
     my $p = $self->pp;
     polygon_remove_parallel_continuous_edges($p);
-    return (ref $self)->new(@$p);
+    return __PACKAGE__->new(@$p);
 }
 
 sub remove_acute_vertices {

lib/Slic3r/Polyline.pm

@@ -32,7 +32,7 @@ sub simplify {
     my $tolerance = shift || 10;
     
     my $simplified = Boost::Geometry::Utils::linestring_simplify($self->pp, $tolerance);
-    return (ref $self)->new(@$simplified);
+    return __PACKAGE__->new(@$simplified);
 }
 
 sub length {
@@ -66,7 +66,7 @@ sub clip_with_expolygon {
     my ($expolygon) = @_;
     
     my $result = Boost::Geometry::Utils::polygon_multi_linestring_intersection($expolygon->pp, [$self->pp]);
-    return map { (ref $self)->new(@$_) } @$result;
+    return map { __PACKAGE__->new(@$_) } @$result;
 }
 
 sub bounding_box {
@@ -128,7 +128,7 @@ sub clip_start {
         $distance = 0;
     }
     
-    return (ref $self)->new($points);
+    return __PACKAGE__->new($points);
 }
 
 # this method returns a collection of points picked on the polygon contour

lib/Slic3r/Print/Object.pm

@@ -643,7 +643,7 @@ sub discover_horizontal_shells {
                                 # additional area in the next shell too
 
                                 # make sure our grown surfaces don't exceed the fill area
-                                my @grown = map @$_, @{intersection_ex(
+                                my @grown = @{intersection(
                                     offset([ map @$_, @$too_narrow ], +$margin),
                                     [ map $_->p, @neighbor_fill_surfaces ],
                                 )};

t/cooling.t

@@ -15,7 +15,7 @@ sub buffer {
     my $config = shift || Slic3r::Config->new_from_defaults;
     my $buffer = Slic3r::GCode::CoolingBuffer->new(
         config      => $config,
-        gcodegen    => Slic3r::GCode->new(config => $config, layer_count => 10),
+        gcodegen    => Slic3r::GCode->new(config => $config, layer_count => 10, extruders => []),
     );
     return $buffer;
 }

t/gcode.t

@@ -14,6 +14,7 @@ use Slic3r::Geometry qw(scale);
     my $gcodegen = Slic3r::GCode->new(
         config      => Slic3r::Config->new_from_defaults,
         layer_count => 1,
+        extruders   => [],
     );
     $gcodegen->set_shift(10, 10);
     is_deeply $gcodegen->last_pos->arrayref, [scale -10, scale -10], 'last_pos is shifted correctly';

xs/src/ExPolygon.cpp

@@ -73,14 +73,14 @@ ExPolygon::to_SV_ref() {
 }
 
 SV*
-ExPolygon::to_SV_clone_ref() {
+ExPolygon::to_SV_clone_ref() const {
     SV* sv = newSV(0);
     sv_setref_pv( sv, "Slic3r::ExPolygon", new ExPolygon(*this) );
     return sv;
 }
 
 SV*
-ExPolygon::to_SV_pureperl()
+ExPolygon::to_SV_pureperl() const
 {
     const unsigned int num_holes = this->holes.size();
     AV* av = newAV();

xs/src/ExPolygon.hpp

@@ -15,8 +15,8 @@ class ExPolygon
     void from_SV_check(SV* poly_sv);
     SV* to_SV();
     SV* to_SV_ref();
-    SV* to_SV_clone_ref();
+    SV* to_SV_clone_ref() const;
-    SV* to_SV_pureperl();
+    SV* to_SV_pureperl() const;
     void scale(double factor);
     void translate(double x, double y);
     void rotate(double angle, Point* center);

xs/src/Line.cpp

@@ -84,14 +84,14 @@ Line::to_SV_ref() {
 }
 
 SV*
-Line::to_SV_clone_ref() {
+Line::to_SV_clone_ref() const {
     SV* sv = newSV(0);
     sv_setref_pv( sv, "Slic3r::Line", new Line(*this) );
     return sv;
 }
 
 SV*
-Line::to_SV_pureperl() {
+Line::to_SV_pureperl() const {
     AV* av = newAV();
     av_extend(av, 1);
     av_store(av, 0, this->a.to_SV_pureperl());

xs/src/Line.hpp

@@ -17,8 +17,8 @@ class Line
     void from_SV_check(SV* line_sv);
     SV* to_SV();
     SV* to_SV_ref();
-    SV* to_SV_clone_ref();
+    SV* to_SV_clone_ref() const;
-    SV* to_SV_pureperl();
+    SV* to_SV_pureperl() const;
     void scale(double factor);
     void translate(double x, double y);
     void rotate(double angle, Point* center);

xs/src/MultiPoint.cpp

@@ -73,7 +73,7 @@ MultiPoint::to_SV() {
 }
 
 SV*
-MultiPoint::to_SV_pureperl() {
+MultiPoint::to_SV_pureperl() const {
     const unsigned int num_points = this->points.size();
     AV* av = newAV();
     av_extend(av, num_points-1);

xs/src/MultiPoint.hpp

@@ -14,7 +14,7 @@ class MultiPoint
     void from_SV(SV* poly_sv);
     void from_SV_check(SV* poly_sv);
     SV* to_SV();
-    SV* to_SV_pureperl();
+    SV* to_SV_pureperl() const;
     void scale(double factor);
     void translate(double x, double y);
     void rotate(double angle, Point* center);

xs/src/Point.cpp

@@ -73,7 +73,7 @@ Point::distance_to(const Point* point) const
 }
 
 SV*
-Point::to_SV_ref() const {
+Point::to_SV_ref() {
     SV* sv = newSV(0);
     sv_setref_pv( sv, "Slic3r::Point::Ref", (void*)this );
     return sv;

xs/src/Point.hpp

@@ -18,7 +18,7 @@ class Point
     explicit Point(long _x = 0, long _y = 0): x(_x), y(_y) {};
     void from_SV(SV* point_sv);
     void from_SV_check(SV* point_sv);
-    SV* to_SV_ref() const;
+    SV* to_SV_ref();
     SV* to_SV_clone_ref() const;
     SV* to_SV_pureperl() const;
     void scale(double factor);

xs/src/Polygon.cpp

@@ -12,7 +12,7 @@ Polygon::last_point() const
 }
 
 SV*
-Polygon::to_SV_ref() const {
+Polygon::to_SV_ref() {
     SV* sv = newSV(0);
     sv_setref_pv( sv, "Slic3r::Polygon::Ref", (void*)this );
     return sv;
@@ -26,7 +26,7 @@ Polygon::to_SV_clone_ref() const {
 }
 
 Lines
-Polygon::lines()
+Polygon::lines() const
 {
     Lines lines;
     for (int i = 0; i < this->points.size()-1; i++) {

xs/src/Polygon.hpp

@@ -12,9 +12,9 @@ namespace Slic3r {
 class Polygon : public MultiPoint {
     public:
     Point* last_point() const;
-    SV* to_SV_ref() const;
+    SV* to_SV_ref();
     SV* to_SV_clone_ref() const;
-    Lines lines();
+    Lines lines() const;
     Polyline* split_at(const Point* point);
     Polyline* split_at_index(int index);
     Polyline* split_at_first_point();

xs/src/Polyline.cpp

@@ -8,18 +8,17 @@ Polyline::last_point() const
     return new Point(this->points.back());
 }
 
-Lines
+void
-Polyline::lines()
+Polyline::lines(Lines &lines) const
 {
-    Lines lines;
+    lines.clear();
     for (int i = 0; i < this->points.size()-1; i++) {
         lines.push_back(Line(this->points[i], this->points[i+1]));
     }
-    return lines;
 }
 
 SV*
-Polyline::to_SV_ref() const
+Polyline::to_SV_ref()
 {
     SV* sv = newSV(0);
     sv_setref_pv( sv, "Slic3r::Polyline::Ref", (void*)this );

xs/src/Polyline.hpp

@@ -9,8 +9,8 @@ namespace Slic3r {
 class Polyline : public MultiPoint {
     public:
     Point* last_point() const;
-    Lines lines();
+    void lines(Lines &lines) const;
-    SV* to_SV_ref() const;
+    SV* to_SV_ref();
     SV* to_SV_clone_ref() const;
 };
 

xs/t/06_polygon.t

@@ -44,7 +44,7 @@ ok $polygon->is_counter_clockwise, 'is_counter_clockwise';
     ok $clone->is_counter_clockwise, 'make_counter_clockwise';
 }
 
-isa_ok $polygon->first_point, 'Slic3r::Point::Ref', 'first_point';
+ok ref($polygon->first_point) eq 'Slic3r::Point', 'first_point';
 
 # this is not a test: this just demonstrates bad usage, where $polygon->clone gets
 # DESTROY'ed before the derived object ($point), causing bad memory access

xs/t/09_polyline.t

@@ -13,6 +13,7 @@ my $points = [
 ];
 
 my $polyline = Slic3r::Polyline->new(@$points);
+
 is_deeply $polyline->pp, $points, 'polyline roundtrip';
 
 is ref($polyline->arrayref), 'ARRAY', 'polyline arrayref is unblessed';

xs/t/11_clipper.t

@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use Slic3r::XS;
-use Test::More tests => 4;
+use Test::More tests => 5;
 
 my $square = [  # ccw
     [200, 100],
@@ -20,6 +20,21 @@ my $hole_in_square = [  # cw
 ];
 my $expolygon = Slic3r::ExPolygon->new($square, $hole_in_square);
 
+{
+    my $result = Slic3r::Geometry::Clipper::offset([ $square, $hole_in_square ], 5);
+    is_deeply [ map $_->pp, @$result ], [ [
+        [205, 95],
+        [205, 205],
+        [95, 205],
+        [95, 95],
+    ], [
+        [155, 145],
+        [145, 145],
+        [145, 155],
+        [155, 155],
+    ] ], 'offset';
+}
+
 {
     my $result = Slic3r::Geometry::Clipper::offset_ex([ @$expolygon ], 5);
     is_deeply $result->[0]->pp, [ [

xs/xsp/ExtrusionPath.xsp

@@ -15,11 +15,11 @@
         %code{% THIS->polyline.points.pop_back(); %};
     void reverse();
     Lines lines()
-        %code{% RETVAL = THIS->polyline.lines(); %};
+        %code{% RETVAL = Lines(); THIS->polyline.lines(RETVAL); %};
     Point* first_point()
-        %code{% const char* CLASS = "Slic3r::Point::Ref"; RETVAL = THIS->first_point(); %};
+        %code{% const char* CLASS = "Slic3r::Point"; RETVAL = THIS->first_point(); %};
     Point* last_point()
-        %code{% const char* CLASS = "Slic3r::Point::Ref"; RETVAL = THIS->last_point(); %};
+        %code{% const char* CLASS = "Slic3r::Point"; RETVAL = THIS->last_point(); %};
 %{
 
 ExtrusionPath*

xs/xsp/Polygon.xsp

@@ -30,7 +30,7 @@
     bool make_clockwise();
     bool is_valid();
     Point* first_point()
-        %code{% const char* CLASS = "Slic3r::Point::Ref"; RETVAL = THIS->first_point(); %};
+        %code{% const char* CLASS = "Slic3r::Point"; RETVAL = THIS->first_point(); %};
 %{
 
 Polygon*

xs/xsp/Polyline.xsp

@@ -18,11 +18,12 @@
     void pop_back()
         %code{% THIS->points.pop_back(); %};
     void reverse();
-    Lines lines();
+    Lines lines()
+        %code{% RETVAL = Lines(); THIS->lines(RETVAL); %};
     Point* first_point()
-        %code{% const char* CLASS = "Slic3r::Point::Ref"; RETVAL = THIS->first_point(); %};
+        %code{% const char* CLASS = "Slic3r::Point"; RETVAL = THIS->first_point(); %};
     Point* last_point()
-        %code{% const char* CLASS = "Slic3r::Point::Ref"; RETVAL = THIS->last_point(); %};
+        %code{% const char* CLASS = "Slic3r::Point"; RETVAL = THIS->last_point(); %};
 %{
 
 Polyline*

xs/xsp/my.map

@@ -48,9 +48,10 @@ OUTPUT
 T_ARRAYREF
 	AV* av = newAV();
 	$arg = newRV_noinc((SV*)av);
+	sv_2mortal($arg);
 	av_extend(av, $var.size()-1);
 	int i = 0;
-    for (${type}::iterator it = $var.begin(); it != $var.end(); ++it) {
+    for (${type}::const_iterator it = $var.begin(); it != $var.end(); ++it) {
         av_store(av, i++, (*it).to_SV_clone_ref());
 	}
 	$var.clear();
@@ -58,6 +59,7 @@ T_ARRAYREF
 T_ARRAYREF_POLYGONS_PTR
     AV* av = newAV();
 	$arg = newRV_noinc((SV*)av);
+	sv_2mortal($arg);
 	av_extend(av, $var->size()-1);
 	int i = 0;
     for (Polygons::iterator it = $var->begin(); it != $var->end(); ++it) {

xs/xsp/typemap.xspt

@@ -15,6 +15,7 @@
 %typemap{Polygons};
 %typemap{ExPolygons};
 %typemap{Polygons*};
+%typemap{Lines*};
 
 %typemap{SurfaceType}{parsed}{
   %cpp_type{SurfaceType};