Refactored the BridgeDetector class to expose a cleaner API and make it stateful

lib/Slic3r/Layer/BridgeDetector.pm

@@ -5,45 +5,61 @@ use List::Util qw(first sum max);
 use Slic3r::Geometry qw(PI unscale scaled_epsilon rad2deg epsilon);
 use Slic3r::Geometry::Clipper qw(intersection_pl intersection_ex);
 
+has 'expolygon'         => (is => 'ro', required => 1);
 has 'lower_slices'      => (is => 'rw', required => 1);  # ExPolygons or ExPolygonCollection
-has 'infill_flow'       => (is => 'rw', required => 1);
+has 'extrusion_width'   => (is => 'rw', required => 1);  # scaled
 has 'resolution'        => (is => 'rw', default => sub { PI/36 });
 
-sub detect_angle {
-    my ($self, $expolygon) = @_;
-    
-    my $anchors_offset = $self->infill_flow->scaled_width;
+has '_edges'            => (is => 'rw'); # Polylines representing the supporting edges
+has '_anchors'          => (is => 'rw'); # ExPolygons
+has 'angle'             => (is => 'rw');
+
+sub BUILD {
+    my ($self) = @_;
     
-    my $grown = $expolygon->offset(+$anchors_offset);
-    my @lower = @{$self->lower_slices};       # expolygons
+    # outset our bridge by an arbitrary amout; we'll use this outer margin
+    # for detecting anchors
+    my $grown = $self->expolygon->offset(+$self->extrusion_width);
     
     # detect what edges lie on lower slices
-    my @edges = (); # polylines
-    foreach my $lower (@lower) {
+    $self->_edges(my $edges = []);
+    foreach my $lower (@{$self->lower_slices}) {
         # turn bridge contour and holes into polylines and then clip them
         # with each lower slice's contour
-        push @edges, map @{$_->clip_as_polyline([$lower->contour])}, @$grown;
+        push @$edges, map @{$_->clip_as_polyline([$lower->contour])}, @$grown;
     }
+    Slic3r::debugf "  bridge has %d support(s)\n", scalar(@$edges);
     
-    Slic3r::debugf "  bridge has %d support(s)\n", scalar(@edges);
-    return undef if !@edges;
-    
-    my $bridge_angle = undef;
+    # detect anchors as intersection between our bridge expolygon and the lower slices
+    $self->_anchors(intersection_ex(
+        $grown,
+        [ map @$_, @{$self->lower_slices} ],
+        1,  # safety offset required to avoid Clipper from detecting empty intersection while Boost actually found some @edges
+    ));
     
     if (0) {
         require "Slic3r/SVG.pm";
         Slic3r::SVG::output("bridge.svg",
-            expolygons      => [ $expolygon ],
-            red_expolygons  => [ @lower ],
-            polylines       => [ @edges ],
+            expolygons      => [ $self->expolygon ],
+            red_expolygons  => $self->lower_slices,
+            polylines       => $self->_edges,
         );
     }
+}
+
+sub detect_angle {
+    my ($self) = @_;
+    
+    return undef if !@{$self->_edges};
+    
+    my @edges = @{$self->_edges};
+    my $anchors = $self->_anchors;
     
     if (@edges == 2) {
         my @chords = map Slic3r::Line->new($_->[0], $_->[-1]), @edges;
         my @midpoints = map $_->midpoint, @chords;
         my $line_between_midpoints = Slic3r::Line->new(@midpoints);
-        $bridge_angle = $line_between_midpoints->direction;
+        $self->angle($line_between_midpoints->direction);
     } elsif (@edges == 1 && !$edges[0][0]->coincides_with($edges[0][-1])) {
         # Don't use this logic if $edges[0] is actually a closed loop
         # TODO: this case includes both U-shaped bridges and plain overhangs;
@@ -53,20 +69,13 @@ sub detect_angle {
         # our supporting edge is a straight line
         if (@{$edges[0]} > 2) {
             my $line = Slic3r::Line->new($edges[0]->[0], $edges[0]->[-1]);
-            $bridge_angle = $line->direction;
+            $self->angle($line->direction);
         }
     } elsif (@edges) {
         # Outset the bridge expolygon by half the amount we used for detecting anchors;
         # we'll use this one to clip our test lines and be sure that their endpoints
         # are inside the anchors and not on their contours leading to false negatives.
-        my $clip_area = $expolygon->offset_ex(+$anchors_offset/2);
-        
-        # detect anchors as intersection between our bridge expolygon and the lower slices
-        my $anchors = intersection_ex(
-            $grown,
-            [ map @$_, @lower ],
-            1,  # safety offset required to avoid Clipper from detecting empty intersection while Boost actually found some @edges
-        );
+        my $clip_area = $self->expolygon->offset_ex(+$self->extrusion_width/2);
         
         if (@$anchors) {
             # we'll now try several directions using a rudimentary visibility check:
@@ -74,7 +83,7 @@ sub detect_angle {
             # endpoints within anchors
             my %directions_coverage     = ();  # angle => score
             my %directions_avg_length   = ();  # angle => score
-            my $line_increment = $self->infill_flow->scaled_width;
+            my $line_increment = $self->extrusion_width;
             for (my $angle = 0; $angle < PI; $angle += $self->resolution) {
                 my $my_clip_area    = [ map $_->clone, @$clip_area ];
                 my $my_anchors      = [ map $_->clone, @$anchors ];
@@ -113,12 +122,15 @@ sub detect_angle {
                 $directions_avg_length{$angle} = @lengths ? (max(@lengths)) : -1;
             }
             
+            # if no direction produced coverage, then there's no bridge direction
+            return undef if !defined first { $_ > 0 } values %directions_coverage;
+            
             # the best direction is the one causing most lines to be bridged (thus most coverage)
             # and shortest max line length
             my @sorted_directions = sort {
                 my $cmp;
                 my $coverage_diff = $directions_coverage{$a} - $directions_coverage{$b};
-                if (abs($coverage_diff) < $self->infill_flow->scaled_width) {
+                if (abs($coverage_diff) < $self->extrusion_width) {
                     $cmp = $directions_avg_length{$b} <=> $directions_avg_length{$a};
                 } else {
                     $cmp = ($coverage_diff > 0) ? 1 : -1;
@@ -126,20 +138,19 @@ sub detect_angle {
                 $cmp;
             } keys %directions_coverage;
             
-            $bridge_angle = $sorted_directions[-1];
+            $self->angle($sorted_directions[-1]);
         }
     }
     
-    if (defined $bridge_angle) {
-        if ($bridge_angle >= PI - epsilon) {
-            $bridge_angle -= PI;
+    if (defined $self->angle) {
+        if ($self->angle >= PI - epsilon) {
+            $self->angle($self->angle - PI);
         }
         
-        Slic3r::debugf "  Optimal infill angle is %d degrees\n", rad2deg($bridge_angle)
-            if defined $bridge_angle;
+        Slic3r::debugf "  Optimal infill angle is %d degrees\n", rad2deg($self->angle);
     }
     
-    return $bridge_angle;
+    return $self->angle;
 }
 
 1;

lib/Slic3r/Layer/Region.pm

@@ -387,7 +387,6 @@ sub process_external_surfaces {
     my $margin = scale &Slic3r::EXTERNAL_INFILL_MARGIN;
     
     my @bottom = ();
-    my $bridge_detector;
     foreach my $surface (grep $_->is_bottom, @surfaces) {
         my $grown = $surface->expolygon->offset_ex(+$margin);
         
@@ -397,12 +396,13 @@ sub process_external_surfaces {
         # of very thin (but still working) anchors, the grown expolygon would go beyond them
         my $angle;
         if ($lower_layer) {
-            $bridge_detector //= Slic3r::Layer::BridgeDetector->new(
+            my $bridge_detector = Slic3r::Layer::BridgeDetector->new(
+                expolygon       => $surface->expolygon,
                 lower_slices    => $lower_layer->slices,
-                infill_flow     => $self->flow(FLOW_ROLE_INFILL),
+                extrusion_width => $self->flow(FLOW_ROLE_INFILL, $self->height, 1)->scaled_width,
             );
             Slic3r::debugf "Processing bridge at layer %d:\n", $self->id;
-            $angle = $bridge_detector->detect_angle($surface->expolygon);
+            $angle = $bridge_detector->detect_angle;
         }
         
         push @bottom, map $surface->clone(expolygon => $_, bridge_angle => $angle), @$grown;

t/bridges.t

@@ -1,4 +1,4 @@
-use Test::More tests => 12;
+use Test::More tests => 6;
 use strict;
 use warnings;
 
@@ -12,85 +12,72 @@ use Slic3r;
 use Slic3r::Geometry qw(scale epsilon deg2rad rad2deg PI);
 use Slic3r::Test;
 
-my $full_test = sub {
-    my ($bd) = @_;
-    {
-        my $test = sub {
-            my ($bridge_size, $rotate, $expected_angle, $tolerance) = @_;
-        
-            my ($x, $y) = @$bridge_size;
-            my $lower = Slic3r::ExPolygon->new(
-                Slic3r::Polygon->new_scale([-2,-2], [$x+2,-2], [$x+2,$y+2], [-2,$y+2]),
-                Slic3r::Polygon->new_scale([0,0], [0,$y], [$x,$y], [$x,0]),
-            );
-            $lower->translate(scale 20, scale 20); # avoid negative coordinates for easier SVG preview
-            $lower->rotate(deg2rad($rotate), [$x/2,$y/2]);
-            my $bridge = $lower->[1]->clone;
-            $bridge->reverse;
-            $bridge = Slic3r::ExPolygon->new($bridge);
-            $bd->lower_slices([$lower]);
-            
-            ok check_angle($bd, $bridge, $expected_angle, $tolerance), 'correct bridge angle for O-shaped overhang';
-        };
+{
+    my $test = sub {
+        my ($bridge_size, $rotate, $expected_angle, $tolerance) = @_;
     
-        $test->([20,10], 0, 0);
-        $test->([10,20], 0, 90);
-        $test->([20,10], 45, 135, 20);
-        $test->([20,10], 135, 45, 20);
-    }
-
-    {
-        my $bridge = Slic3r::ExPolygon->new(
-            Slic3r::Polygon->new_scale([0,0], [20,0], [20,10], [0,10]),
+        my ($x, $y) = @$bridge_size;
+        my $lower = Slic3r::ExPolygon->new(
+            Slic3r::Polygon->new_scale([-2,-2], [$x+2,-2], [$x+2,$y+2], [-2,$y+2]),
+            Slic3r::Polygon->new_scale([0,0], [0,$y], [$x,$y], [$x,0]),
         );
-        my $lower = [
-            Slic3r::ExPolygon->new(
-                Slic3r::Polygon->new_scale([-2,0], [0,0], [0,10], [-2,10]),
-            ),
-        ];
-        $_->translate(scale 20, scale 20) for $bridge, @$lower; # avoid negative coordinates for easier SVG preview
-    
-        $lower->[1] = $lower->[0]->clone;
-        $lower->[1]->translate(scale 22, 0);
-    
-        $bd->lower_slices($lower);
-        ok check_angle($bd, $bridge, 0), 'correct bridge angle for two-sided bridge';
-    }
-
-    {
-        my $bridge = Slic3r::ExPolygon->new(
-            Slic3r::Polygon->new_scale([0,0], [20,0], [10,10], [0,10]),
-        );
-        my $lower = [
-            Slic3r::ExPolygon->new(
-                Slic3r::Polygon->new_scale([0,0], [0,10], [10,10], [10,12], [-2,12], [-2,-2], [22,-2], [22,0]),
-            ),
-        ];
-        $_->translate(scale 20, scale 20) for $bridge, @$lower; # avoid negative coordinates for easier SVG preview
-    
-        $bd->lower_slices($lower);
-        ok check_angle($bd, $bridge, 135), 'correct bridge angle for C-shaped overhang';
-    }
-};
+        $lower->translate(scale 20, scale 20); # avoid negative coordinates for easier SVG preview
+        $lower->rotate(deg2rad($rotate), [$x/2,$y/2]);
+        my $bridge = $lower->[1]->clone;
+        $bridge->reverse;
+        $bridge = Slic3r::ExPolygon->new($bridge);
+        
+        ok check_angle([$lower], $bridge, $expected_angle, $tolerance), 'correct bridge angle for O-shaped overhang';
+    };
 
-my $flow = Slic3r::Flow->new(width => 0.5, spacing => 0.45, nozzle_diameter => 0.5);
-my $bd = Slic3r::Layer::BridgeDetector->new(
-    lower_slices    => [],
-    infill_flow     => $flow,
-);
+    $test->([20,10], 0, 0);
+    $test->([10,20], 0, 90);
+    $test->([20,10], 45, 135, 20);
+    $test->([20,10], 135, 45, 20);
+}
 
-$full_test->($bd);
+{
+    my $bridge = Slic3r::ExPolygon->new(
+        Slic3r::Polygon->new_scale([0,0], [20,0], [20,10], [0,10]),
+    );
+    my $lower = [
+        Slic3r::ExPolygon->new(
+            Slic3r::Polygon->new_scale([-2,0], [0,0], [0,10], [-2,10]),
+        ),
+    ];
+    $_->translate(scale 20, scale 20) for $bridge, @$lower; # avoid negative coordinates for easier SVG preview
 
-# infill flow larger than perimeter flow
-$bd->infill_flow(Slic3r::Flow->new(width => 0.9, spacing => 0.85, nozzle_diameter => 0.5));
-$full_test->($bd);
+    $lower->[1] = $lower->[0]->clone;
+    $lower->[1]->translate(scale 22, 0);
+    
+    ok check_angle($lower, $bridge, 0), 'correct bridge angle for two-sided bridge';
+}
 
+{
+    my $bridge = Slic3r::ExPolygon->new(
+        Slic3r::Polygon->new_scale([0,0], [20,0], [10,10], [0,10]),
+    );
+    my $lower = [
+        Slic3r::ExPolygon->new(
+            Slic3r::Polygon->new_scale([0,0], [0,10], [10,10], [10,12], [-2,12], [-2,-2], [22,-2], [22,0]),
+        ),
+    ];
+    $_->translate(scale 20, scale 20) for $bridge, @$lower; # avoid negative coordinates for easier SVG preview
+    
+    ok check_angle($lower, $bridge, 135), 'correct bridge angle for C-shaped overhang';
+}
 
 sub check_angle {
-    my ($bd, $bridge, $expected, $tolerance) = @_;
+    my ($lower, $bridge, $expected, $tolerance) = @_;
+    
+    my $bd = Slic3r::Layer::BridgeDetector->new(
+        expolygon       => $bridge,
+        lower_slices    => $lower,
+        extrusion_width => scale 0.5,
+    );
     
     $tolerance //= rad2deg($bd->resolution) + epsilon;
-    my $result = $bd->detect_angle($bridge);
+    my $result = $bd->detect_angle;
     
     # our epsilon is equal to the steps used by the bridge detection algorithm
     ###use XXX; YYY [ rad2deg($result), $expected ];