Collect undetected lines (caused by dirty or non-manifold models), warn the user, post debug info. Includes some further ExPolygon refactoring.

lib/Slic3r/ExPolygon.pm

@@ -18,7 +18,7 @@ sub new {
             map Slic3r::Polygon->new($_), @{$_[0]{holes}},
         ];
     } else {
-        $self = [@_];
+        $self = [ map Slic3r::Polygon->new($_), @_ ];
     }
     bless $self, $class;
     $self;
@@ -29,7 +29,7 @@ sub new {
 # right contours
 sub make {
     my $class = shift;
-    return map $class->new($_), @{ union_ex(\@_) };
+    return @{ union_ex(\@_) };
 }
 
 sub contour {

lib/Slic3r/Geometry/Clipper.pm

@@ -5,7 +5,7 @@ use warnings;
 require Exporter;
 our @ISA = qw(Exporter);
 our @EXPORT_OK = qw(explode_expolygon explode_expolygons safety_offset
-    diff_ex diff union_ex intersection_ex);
+    diff_ex diff union_ex intersection_ex PFT_EVENODD);
 
 use Math::Clipper 1.02 ':all';
 our $clipper = Math::Clipper->new;
@@ -39,10 +39,14 @@ sub diff {
 }
 
 sub union_ex {
-    my ($polygons) = @_;
+    my ($polygons, $jointype) = @_;
+    $jointype = PFT_NONZERO unless defined $jointype;
     $clipper->clear;
     $clipper->add_subject_polygons($polygons);
-    return $clipper->ex_execute(CT_UNION, PFT_NONZERO, PFT_NONZERO);
+    return [
+        map Slic3r::ExPolygon->new($_),
+            @{ $clipper->ex_execute(CT_UNION, $jointype, $jointype) },
+    ];
 }
 
 sub intersection_ex {

lib/Slic3r/Layer.pm

@@ -4,7 +4,7 @@ use Moo;
 use Math::Clipper ':all';
 use Slic3r::Geometry qw(polygon_lines points_coincide angle3points polyline_lines nearest_point
     line_length);
-use Slic3r::Geometry::Clipper qw(safety_offset union_ex);
+use Slic3r::Geometry::Clipper qw(safety_offset union_ex PFT_EVENODD);
 use XXX;
 
 use constant PI => 4 * atan2(1, 1);
@@ -132,13 +132,6 @@ sub remove_surface {
 sub make_surfaces {
     my $self = shift;
     
-    # this algorithm can be further simplified:
-    # first remove all facetedges that are not connected to any other edge
-    # or that are connected to more than one edge: those are the edges
-    # tangent to our plane, that we don't care about;
-    # then we would have all points connecting two and only two lines,
-    # so a simple head-to-tail algorithm would work
-    
     my @lines = ();
     push @lines, @{$self->lines};
     #@lines = grep line_length($_) > xx, @lines;
@@ -162,9 +155,12 @@ sub make_surfaces {
     }
     
     my $n = 0;
-    my @polygons = ();
+    my (@polygons, %visited_lines, @discarded_lines, @discarded_polylines) = ();
     while (my $first_line = shift @lines) {
+        next if $visited_lines{ $first_line->id };
         my @points = @$first_line;
+        
+        my @seen_lines = ($first_line);
         my %seen_points = map { $get_point_id->($points[$_]) => $_ } 0..1;
         
         CYCLE: while (1) {
@@ -207,7 +203,7 @@ sub make_surfaces {
             #}
             
             my ($next_line) = splice @$next_lines, $ordered_next_lines[0], 1;
-            
+            push @seen_lines, $next_line;
             
             push @points, $next_line->[B];
             
@@ -221,21 +217,60 @@ sub make_surfaces {
         }
         
         if (@points < 4 || !points_coincide($points[0], $points[-1])) {
+            # discarding polyline
+            if (@points == 2) {
+                push @discarded_lines, [@points];
+            } else {
+                push @discarded_polylines, [@points];
+            }
             next;
         }
         
+        $visited_lines{ $_->id } = 1 for @seen_lines;
         pop @points;
         Slic3r::debugf "Discovered polygon of %d points\n", scalar(@points);
         push @polygons, Slic3r::Polygon->new(@points);
         $polygons[-1]->cleanup;
     }
     
-    {
+    # Now, if we got a clean and manifold model then @polygons would contain everything
-        my $expolygons = union_ex([ @polygons ]);
+    # we need to draw our layer. In real life, sadly, things are different and it is likely
-        Slic3r::debugf "  %d surface(s) detected from %d polylines\n",
+    # that the above algorithm wasn't able to detect every polygon. This may happen because
-            scalar(@$expolygons), scalar(@polygons);
+    # of non-manifoldness or because of many close lines, often overlapping; both situations
+    # make a head-to-tail search difficult.
+    # On the other hand, we can safely assume that every polygon we detected is correct, as 
+    # the above algorithm is quite strict. We can take a brute force approach to connect any
+    # other line.
     
-        push @{$self->surfaces}, map Slic3r::Surface->cast_from_expolygon($_, surface_type => 'internal'), @$expolygons;
+    # So, let's first check what lines were not detected as part of polygons.
+    if (@discarded_lines || @discarded_polylines) {
+        print "  Warning: errors while parsing this layer (dirty or non-manifold model)\n";
+        Slic3r::debugf "  %d lines out of %d were discarded and %d polylines were not closed\n",
+            scalar(@discarded_lines), scalar(@{$self->lines}), scalar(@discarded_polylines);
+        
+        if (0) {
+            require "Slic3r/SVG.pm";
+            Slic3r::SVG::output(undef, "layer" . $self->id . "_detected.svg",
+                white_polygons => \@polygons,
+            );
+            Slic3r::SVG::output(undef, "layer" . $self->id . "_discarded_lines.svg",
+                red_lines   => \@discarded_lines,
+            );
+            Slic3r::SVG::output(undef, "layer" . $self->id . "_discarded_polylines.svg",
+                polylines   => \@discarded_polylines,
+            );
+            exit;
+        }
+    }
+    
+    {
+        my $expolygons = union_ex([ @polygons ], PFT_EVENODD);
+        Slic3r::debugf "  %d surface(s) having %d holes detected from %d polylines\n",
+            scalar(@$expolygons), scalar(map $_->holes, @$expolygons), scalar(@polygons);
+        
+        push @{$self->surfaces},
+            map Slic3r::Surface->cast_from_expolygon($_, surface_type => 'internal'),
+                @$expolygons;
     }
     
     #use Slic3r::SVG;

lib/Slic3r/Line.pm

@@ -13,7 +13,7 @@ sub new {
     if (@_ == 2) {
         $self = [ map Slic3r::Point->new($_), @_ ];
     } elsif (ref $_[0] eq 'ARRAY') {
-        $self = [ map Slic3r::Point->new($_), @{$_[0]} ];
+        $self = [ map Slic3r::Point->new($_), $_[0][0], $_[0][1] ];
     } elsif ($_[0]->isa(__PACKAGE__)) {
         return $_[0];
     } else {

lib/Slic3r/Point.pm

@@ -4,22 +4,31 @@ use warnings;
 
 sub new {
     my $class = shift;
-    my $self;use XXX; ZZZ if !defined $_[0];
+    my $self;
     if (@_ == 2) {
         $self = [@_];
-    } elsif (ref $_[0] eq 'ARRAY') {
+    } elsif ((ref $_[0]) =~ 'ARRAY' || (ref $_[0]) =~ /Slic3r::Point/) {
         $self = [@{$_[0]}];
     } elsif ($_[0]->isa(__PACKAGE__)) {
         return $_[0];
     } else {
         use XXX;
-        ZZZ "test";
+        ZZZ \@_;
         die "Invalid arguments for ${class}->new";
     }
     bless $self, $class;
     return $self;
 }
 
+sub cast {
+    my $class = shift;
+    if (ref $_[0] eq 'Slic3r::Point') {
+        return $_[0];
+    } else {
+        return $class->new(@_);
+    }
+}
+
 sub id {
     my $self = shift;
     return join ',', @$self;

lib/Slic3r/Polygon.pm

@@ -7,7 +7,7 @@ use warnings;
 # as a Slic3r::Polyline::Closed you're right. I plan to
 # ditch the latter and port everything to this class.
 
-use Slic3r::Geometry qw(polygon_remove_parallel_continuous_edges);
+use Slic3r::Geometry qw(polygon_lines polygon_remove_parallel_continuous_edges);
 
 # the constructor accepts an array(ref) of points
 sub new {
@@ -18,10 +18,23 @@ sub new {
     } else {
         $self = [ @_ ];
     }
+    
+    @$self = map Slic3r::Point->cast($_), @$self;
     bless $self, $class;
     $self;
 }
 
+# legacy method, to be removed when we ditch Slic3r::Polyline::Closed
+sub closed_polyline {
+    my $self = shift;
+    return Slic3r::Polyline::Closed->cast($self);
+}
+
+sub lines {
+    my $self = shift;
+    return map Slic3r::Line->new($_), polygon_lines($self);
+}
+
 sub cleanup {
     my $self = shift;
     polygon_remove_parallel_continuous_edges($self);

lib/Slic3r/Polyline.pm

@@ -24,7 +24,7 @@ sub cast {
     my $class = shift;
     my ($points, %args) = @_;
     
-    $points = [ map { ref $_ eq 'ARRAY' ? Slic3r::Point->new($_) : $_ } @$points ];
+    $points = [ map Slic3r::Point->cast($_), @$points ];
     return $class->new(points => $points, %args);
 }
 

lib/Slic3r/Print.pm

@@ -2,7 +2,7 @@ package Slic3r::Print;
 use Moo;
 
 use Math::Clipper ':all';
-use Slic3r::Geometry::Clipper qw(explode_expolygons safety_offset diff_ex union_ex intersection_ex);
+use Slic3r::Geometry::Clipper qw(explode_expolygons safety_offset diff_ex intersection_ex);
 use XXX;
 
 use constant X => 0;

lib/Slic3r/Surface.pm

@@ -36,16 +36,13 @@ sub cast_from_expolygon {
     my $class = shift;
     my ($expolygon, %args) = @_;
     
-    if (ref $expolygon ne 'HASH') {
+    if (ref $expolygon eq 'HASH') {
-        use XXX; ZZZ $expolygon if ref $expolygon eq 'ARRAY';
+        $expolygon = Slic3r::ExPolygon->new($expolygon);
-        $expolygon = $expolygon->clipper_expolygon;
     }
     
     return $class->new(
-        contour      => Slic3r::Polyline::Closed->cast($expolygon->{outer}),
+        contour => $expolygon->contour->closed_polyline,
-        holes        => [
+        holes   => [ map $_->closed_polyline, $expolygon->holes ],
-            map Slic3r::Polyline::Closed->cast($_), @{$expolygon->{holes}}
-        ],
         %args,
     );
 }