Extended (and fixed) unit testing to track down (and fix) an issue caused by floating point math that reversed some holes into contours when they should actually be ignored

lib/Slic3r/Layer.pm

@@ -105,8 +105,28 @@ sub make_surfaces {
     my ($loops) = @_;
     
     {
+        # merge everything
         my $expolygons = union_ex($loops);
         
+        # sometimes the magic of floating point values produces holes outside of any contour;
+        # we need to ignore such holes, but Clipper will convert them to contours.
+        # so we identify them and remove them manually.
+        
+        # get expolygons without holes (candidate for reverse holes detection)
+        my @expolygons_without_holes = grep { @$_ == 1 } @$expolygons;
+        
+        # remove all holes from such expolygons
+        my $diff = diff_ex(
+            [ map @$_, @expolygons_without_holes ],
+            [ map [ reverse @$_ ], grep !is_counter_clockwise($_), @$loops ],
+        );
+        
+        # merge resulting holes (true holes) and other expolygons
+        $expolygons = [
+            (grep { @$_ > 1 } @$expolygons),
+            @$diff,
+        ];
+        
         Slic3r::debugf "  %d surface(s) having %d holes detected from %d polylines\n",
             scalar(@$expolygons), scalar(map $_->holes, @$expolygons), scalar(@$loops);
         

lib/Slic3r/TriangleMesh.pm

@@ -71,14 +71,6 @@ sub make_loops {
     my $sparse_lines = [ map $_->line, grep $_, @lines ];
     
     # detect closed loops
-    if (0) {
-        printf "Layer was sliced at z = %f\n", $self->slice_z * $Slic3r::resolution;
-        require "Slic3r/SVG.pm";
-        Slic3r::SVG::output(undef, "lines.svg",
-            lines       => [ grep !$_->isa('Slic3r::Line::FacetEdge'), @lines ],
-            red_lines   => [ grep  $_->isa('Slic3r::Line::FacetEdge'), @lines ],
-        );
-    }
     
     my (@polygons, %visited_lines, @discarded_lines, @discarded_polylines) = ();
     
@@ -332,7 +324,7 @@ sub _facet {
     }
     Slic3r::debugf "z: min = %.0f, max = %.0f\n", $min_z, $max_z;
     
-    if ($min_z == $max_z) {
+    if (abs($max_z - $min_z) < epsilon) {
         Slic3r::debugf "Facet is horizontal; ignoring\n";
         return;
     }
@@ -375,8 +367,8 @@ sub intersect_facet {
         
         if (abs($a->[Z] - $b->[Z]) < epsilon && abs($a->[Z] - $z) < epsilon) {
             # edge is horizontal and belongs to the current layer
-            my $edge_type = (grep $_->[Z] > $z, @$vertices) ? 'bottom' : 'top';
+            my $edge_type = (grep $_->[Z] < $z - epsilon, @$vertices) ? 'top' : 'bottom';
-            ($a, $b) = ($b, $a) if $edge_type eq 'bottom';
+            ($a, $b) = ($b, $a) if $edge_type eq 'top';
             push @lines, Slic3r::TriangleMesh::IntersectionLine->new(
                 a           => [$a->[X], $a->[Y]],
                 b           => [$b->[X], $b->[Y]],
@@ -429,13 +421,13 @@ sub intersect_facet {
         
         # connect points:
         return Slic3r::TriangleMesh::IntersectionLine->new(
-            a      => [$points[A][X], $points[A][Y]],
+            a      => [$points[B][X], $points[B][Y]],
-            b      => [$points[B][X], $points[B][Y]],
+            b      => [$points[A][X], $points[A][Y]],
-            a_id   => $points[A][2],
+            a_id   => $points[B][2],
-            b_id   => $points[B][2],
+            b_id   => $points[A][2],
             facet_index => $facet_index,
-            prev_facet_index => ($points[A][3] ? +(grep $_ != $facet_index, @{$self->edge_facets->{$points[A][3]}})[0] || undef : undef),
+            prev_facet_index => ($points[B][3] ? +(grep $_ != $facet_index, @{$self->edge_facets->{$points[B][3]}})[0] || undef : undef),
-            next_facet_index => ($points[B][3] ? +(grep $_ != $facet_index, @{$self->edge_facets->{$points[B][3]}})[0] || undef : undef),
+            next_facet_index => ($points[A][3] ? +(grep $_ != $facet_index, @{$self->edge_facets->{$points[A][3]}})[0] || undef : undef),
         );
         #printf "  intersection points at z = %f: %f,%f - %f,%f\n", $z, map @$_, @intersection_points;
     }

t/stl.t

@@ -2,7 +2,7 @@ use Test::More;
 use strict;
 use warnings;
 
-plan tests => 9;
+plan tests => 17;
 
 BEGIN {
     use FindBin;
@@ -10,14 +10,14 @@ BEGIN {
 }
 
 use Slic3r;
-use Slic3r::Geometry qw(X Y Z);
+use Slic3r::Geometry qw(X Y Z A B);
 use XXX;
 
 my $mesh = Slic3r::TriangleMesh->new;
 
 my @lines;
 my $z = 20;
-my @points = ([3, 4], [8, 5], [1, 9]);
+my @points = ([3, 4], [8, 5], [1, 9]);  # XY coordinates of the facet vertices
 
 is_deeply lines(20, 20, 20), [
     [ $points[0], $points[1] ],
@@ -25,12 +25,76 @@ is_deeply lines(20, 20, 20), [
     [ $points[2], $points[0] ],
 ], 'horizontal';
 
-is_deeply lines(22, 20, 20), [ [ $points[2], $points[1] ] ], 'lower edge on layer';
+is_deeply lines(22, 20, 20), [ [ $points[1], $points[2] ] ], 'lower edge on layer';
-is_deeply lines(20, 20, 10), [ [ $points[0], $points[1] ] ], 'upper edge on layer';
+is_deeply lines(20, 20, 22), [ [ $points[0], $points[1] ] ], 'lower edge on layer';
+is_deeply lines(20, 22, 20), [ [ $points[2], $points[0] ] ], 'lower edge on layer';
+
+is_deeply lines(20, 20, 10), [ [ $points[1], $points[0] ] ], 'upper edge on layer';
+is_deeply lines(10, 20, 20), [ [ $points[2], $points[1] ] ], 'upper edge on layer';
+is_deeply lines(20, 10, 20), [ [ $points[0], $points[2] ] ], 'upper edge on layer';
+
 is_deeply lines(20, 15, 10), [                            ], 'upper vertex on layer';
 is_deeply lines(28, 20, 30), [                            ], 'lower vertex on layer';
-is_deeply lines(24, 10, 16), [ [ [4, 4],     [2, 6]     ] ], 'two edges intersect';
+
-is_deeply lines(24, 10, 20), [ [ [4, 4],     [1, 9]     ] ], 'one vertex on plane and one edge intersects';
+{
+    my @z = (24, 10, 16);
+    is_deeply lines(@z), [
+        [
+            line_plane_intersection([ vertices(@z)->[2], vertices(@z)->[0] ]),
+            line_plane_intersection([ vertices(@z)->[0], vertices(@z)->[1] ]),
+        ]
+    ], 'two edges intersect';
+}
+
+{
+    my @z = (16, 24, 10);
+    is_deeply lines(@z), [
+        [
+            line_plane_intersection([ vertices(@z)->[1], vertices(@z)->[2] ]),
+            line_plane_intersection([ vertices(@z)->[0], vertices(@z)->[1] ]),
+        ]
+    ], 'two edges intersect';
+}
+
+{
+    my @z = (10, 16, 24);
+    is_deeply lines(@z), [
+        [
+            line_plane_intersection([ vertices(@z)->[2], vertices(@z)->[0] ]),
+            line_plane_intersection([ vertices(@z)->[1], vertices(@z)->[2] ]),
+        ]
+    ], 'two edges intersect';
+}
+
+{
+    my @z = (24, 10, 20);
+    is_deeply lines(@z), [
+        [
+            $points[2],
+            line_plane_intersection([ vertices(@z)->[0], vertices(@z)->[1] ]),
+        ]
+    ], 'one vertex on plane and one edge intersects';
+}
+
+{
+    my @z = (10, 20, 24);
+    is_deeply lines(@z), [
+        [
+            line_plane_intersection([ vertices(@z)->[2], vertices(@z)->[0] ]),
+            $points[1],
+        ]
+    ], 'one vertex on plane and one edge intersects';
+}
+
+{
+    my @z = (20, 24, 10);
+    is_deeply lines(@z), [
+        [
+            line_plane_intersection([ vertices(@z)->[1], vertices(@z)->[2] ]),
+            $points[0],
+        ]
+    ], 'one vertex on plane and one edge intersects';
+}
 
 my @lower = $mesh->intersect_facet(0, vertices(22, 20, 20), $z);
 my @upper = $mesh->intersect_facet(0, vertices(20, 20, 10), $z);
@@ -38,7 +102,7 @@ is $lower[0]->facet_edge, 'bottom', 'bottom edge on layer';
 is $upper[0]->facet_edge, 'top', 'upper edge on layer';
 
 sub vertices {
-    [ map [ @{$points[$_]}, $_[$_] ], X,Y,Z ]
+    [ map [ @{$points[$_]}, $_[$_] ], 0..2 ]
 }
 
 sub lines {
@@ -49,3 +113,15 @@ sub lines {
     $_->b->[Y] = sprintf('%.0f', $_->b->[Y]) for @lines;
     return [ map $_->points, @lines ];
 }
+
+sub line_plane_intersection {
+    my ($line) = @_;
+    
+    return [
+        map sprintf('%.0f', $_),
+            map +($line->[B][$_] + ($line->[A][$_] - $line->[B][$_]) * ($z - $line->[B][Z]) / ($line->[A][Z] - $line->[B][Z])),
+                (X,Y)
+    ];
+}
+
+__END__