Further cleanup of BridgeDetector and other minor things

t/bridges.t

@@ -82,8 +82,27 @@ use Slic3r::Test;
     ok check_angle($lower, $bridge, 45, undef, $bridge->area/2), 'correct bridge angle for square overhang with L-shaped anchors';
 }
 
+if (0) {
+    # GH #2477:
+    # This rectangle-shaped bridge is actually unsupported (i.e. the potential anchors are
+    # a bit far away from the contour of the bridge area) because perimeters are reducing
+    # its area.
+    my $bridge = Slic3r::ExPolygon->new(
+        Slic3r::Polygon->new([30023195,14023195],[1776805,14023195],[1776805,1776805],[30023195,1776805]),
+    );
+    my $lower = [
+        Slic3r::ExPolygon->new(
+            Slic3r::Polygon->new([31800000,15800000],[0,15800000],[0,0],[31800000,0]),
+            Slic3r::Polygon->new([1499999,1500000],[1499999,14300000],[30300000,14300000],[30300000,1500000]),
+        ),
+    ];
+    
+    ok check_angle($lower, $bridge, 90, undef, $bridge->area, 500000),
+        'correct bridge angle for rectangle';
+}
+
 sub check_angle {
-    my ($lower, $bridge, $expected, $tolerance, $expected_coverage) = @_;
+    my ($lower, $bridge, $expected, $tolerance, $expected_coverage, $extrusion_width) = @_;
     
     if (ref($lower) eq 'ARRAY') {
         $lower = Slic3r::ExPolygon::Collection->new(@$lower);
@@ -91,8 +110,9 @@ sub check_angle {
     
     $expected_coverage //= -1;
     $expected_coverage = $bridge->area if $expected_coverage == -1;
+    $extrusion_width //= scale 0.5;
     
-    my $bd = Slic3r::BridgeDetector->new($bridge, $lower, scale 0.5);
+    my $bd = Slic3r::BridgeDetector->new($bridge, $lower, $extrusion_width);
     
     $tolerance //= rad2deg($bd->resolution) + epsilon;
     $bd->detect_angle;

xs/src/libslic3r/BridgeDetector.cpp

@@ -26,16 +26,16 @@ BridgeDetector::BridgeDetector(const ExPolygon &_expolygon, const ExPolygonColle
     // safety offset required to avoid Clipper from detecting empty intersection while Boost actually found some edges
     this->_anchors = intersection_ex(grown, this->lower_slices, true);
     
-    /*
+    #if 0
-    if (0) {
+    {
-        require "Slic3r/SVG.pm";
+        SVG svg("bridge.svg");
-        Slic3r::SVG::output("bridge.svg",
+        svg.draw(this->expolygon);
-            expolygons      => [ $self->expolygon ],
+        svg.draw(this->lower_slices, "red");
-            red_expolygons  => $self->lower_slices,
+        svg.draw(this->_anchors, "yellow");
-            polylines       => $self->_edges,
+        //svg.draw(this->_edges, "black", scale_(0.2));
-        );
+        svg.Close();
     }
-    */
+    #endif
 }
 
 bool
@@ -48,7 +48,7 @@ BridgeDetector::detect_angle()
     /*  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. */
-    Polygons clip_area = offset(this->expolygon, +this->extrusion_width/2);
+    const Polygons clip_area = offset(this->expolygon, +this->extrusion_width/2);
     
     /*  we'll now try several directions using a rudimentary visibility check:
         bridge in several directions and then sum the length of lines having both
@@ -74,13 +74,13 @@ BridgeDetector::detect_angle()
     
         /*  we also test angles of each open supporting edge
             (this finds the optimal angle for C-shaped supports) */
-        for (Polylines::const_iterator edge = this->_edges.begin(); edge != this->_edges.end(); ++edge) {
+        for (const Polyline &edge : this->_edges) {
-            if (edge->first_point().coincides_with(edge->last_point())) continue;
+            if (edge.first_point().coincides_with(edge.last_point())) continue;
-            angles.push_back(Line(edge->first_point(), edge->last_point()).direction());
+            angles.push_back(Line(edge.first_point(), edge.last_point()).direction());
         }
     
         // remove duplicates
-        double min_resolution = PI/180.0;  // 1 degree
+        constexpr double min_resolution = PI/180.0;  // 1 degree
         std::sort(angles.begin(), angles.end());
         for (size_t i = 1; i < angles.size(); ++i) {
             if (Slic3r::Geometry::directions_parallel(angles[i], angles[i-1], min_resolution)) {
@@ -97,7 +97,7 @@ BridgeDetector::detect_angle()
             candidates.push_back(BridgeDirection(angle));
     }
     
-    double line_increment = this->extrusion_width;
+    const double line_increment = this->extrusion_width;
     bool have_coverage = false;
     for (BridgeDirection &candidate : candidates) {
         Polygons my_clip_area = clip_area;
@@ -112,27 +112,22 @@ BridgeDetector::detect_angle()
         // generate lines in this direction
         BoundingBox bb;
         for (const ExPolygon &e : my_anchors)
-            bb.merge((Points)e);
+            bb.merge(e.bounding_box());
         
         Lines lines;
         for (coord_t y = bb.min.y; y <= bb.max.y; y += line_increment)
             lines.push_back(Line(Point(bb.min.x, y), Point(bb.max.x, y)));
         
-        Lines clipped_lines = intersection_ln(lines, my_clip_area);
+        const Lines clipped_lines = intersection_ln(lines, my_clip_area);
-        
-        // remove any line not having both endpoints within anchors
-        for (size_t i = 0; i < clipped_lines.size(); ++i) {
-            Line &line = clipped_lines[i];
-            if (!Slic3r::Geometry::contains(my_anchors, line.a)
-                || !Slic3r::Geometry::contains(my_anchors, line.b)) {
-                clipped_lines.erase(clipped_lines.begin() + i);
-                --i;
-            }
-        }
         
         std::vector<double> lengths;
         double total_length = 0;
         for (const Line &line : clipped_lines) {
+            // skip any line not having both endpoints within anchors
+            if (!Slic3r::Geometry::contains(my_anchors, line.a)
+                || !Slic3r::Geometry::contains(my_anchors, line.b))
+                continue;
+            
             const double len = line.length();
             lengths.push_back(len);
             total_length += len;
@@ -220,13 +215,13 @@ BridgeDetector::coverage(double angle) const
     }
     
     // merge trapezoids and rotate them back
-    Polygons _coverage = union_(covered);
+    covered = union_(covered);
-    for (Polygons::iterator p = _coverage.begin(); p != _coverage.end(); ++p)
+    for (Polygon &p : covered)
-        p->rotate(-(PI/2.0 - angle), Point(0,0));
+        p.rotate(-(PI/2.0 - angle), Point(0,0));
     
     // intersect trapezoids with actual bridge area to remove extra margins
     // and append it to result
-    return intersection(_coverage, this->expolygon);
+    return intersection(covered, this->expolygon);
     
     /*
     if (0) {

xs/src/libslic3r/ExPolygon.hpp

@@ -2,6 +2,7 @@
 #define slic3r_ExPolygon_hpp_
 
 #include "libslic3r.h"
+#include "BoundingBox.hpp"
 #include "Polygon.hpp"
 #include "Polyline.hpp"
 #include <ostream>
@@ -30,6 +31,7 @@ class ExPolygon
     bool contains(const Point &point) const;
     bool contains_b(const Point &point) const;
     bool has_boundary_point(const Point &point) const;
+    BoundingBox bounding_box() const { return this->contour.bounding_box(); };
     void remove_vertical_collinear_points(coord_t tolerance);
     void simplify_p(double tolerance, Polygons* polygons) const;
     Polygons simplify_p(double tolerance) const;

xs/src/libslic3r/LayerRegion.cpp

@@ -71,13 +71,12 @@ void
 LayerRegion::process_external_surfaces()
 {
     const Surfaces &surfaces = this->fill_surfaces.surfaces;
-    const double margin = scale_(EXTERNAL_INFILL_MARGIN);
     
     SurfaceCollection bottom;
     for (Surfaces::const_iterator surface = surfaces.begin(); surface != surfaces.end(); ++surface) {
         if (!surface->is_bottom()) continue;
         
-        const ExPolygons grown = offset_ex(surface->expolygon, +margin);
+        const ExPolygons grown = offset_ex(surface->expolygon, +SCALED_EXTERNAL_INFILL_MARGIN);
         
         /*  detect bridge direction before merging grown surfaces otherwise adjacent bridges
             would get merged into a single one while they need different directions
@@ -92,7 +91,7 @@ LayerRegion::process_external_surfaces()
             );
             
             #ifdef SLIC3R_DEBUG
-            printf("Processing bridge at layer %zu:\n", this->layer()->id());
+            printf("Processing bridge at layer %zu (z = %f):\n", this->layer()->id(), this->layer()->print_z);
             #endif
             
             if (bd.detect_angle()) {
@@ -119,7 +118,7 @@ LayerRegion::process_external_surfaces()
         
         // give priority to bottom surfaces
         ExPolygons grown = diff_ex(
-            offset(surface->expolygon, +margin),
+            offset(surface->expolygon, +SCALED_EXTERNAL_INFILL_MARGIN),
             (Polygons)bottom
         );
         for (ExPolygons::const_iterator it = grown.begin(); it != grown.end(); ++it) {

xs/src/libslic3r/SVG.cpp

@@ -35,6 +35,11 @@ SVG::SVG(const char* filename, const BoundingBox &bbox)
         h, w);
 }
 
+SVG::~SVG()
+{
+    this->Close();
+}
+
 void
 SVG::draw(const Line &line, std::string stroke, coord_t stroke_width)
 {
@@ -68,10 +73,10 @@ void SVG::draw(const ThickLine &line, const std::string &fill, const std::string
 }
 
 void
-SVG::draw(const Lines &lines, std::string stroke)
+SVG::draw(const Lines &lines, std::string stroke, coord_t stroke_width)
 {
     for (Lines::const_iterator it = lines.begin(); it != lines.end(); ++it)
-        this->draw(*it, stroke);
+        this->draw(*it, stroke, stroke_width);
 }
 
 void
@@ -94,6 +99,12 @@ SVG::draw(const ExPolygon &expolygon, std::string fill)
     this->path(d, true);
 }
 
+void
+SVG::draw(const ExPolygonCollection &coll, std::string fill)
+{
+    this->draw(coll.expolygons, fill);
+}
+
 void
 SVG::draw(const ExPolygons &expolygons, std::string fill)
 {
@@ -126,7 +137,7 @@ void
 SVG::draw(const Polylines &polylines, std::string stroke, coord_t stroke_width)
 {
     for (Polylines::const_iterator it = polylines.begin(); it != polylines.end(); ++it)
-        this->draw(*it, fill, stroke_width);
+        this->draw(*it, stroke, stroke_width);
 }
 
 void SVG::draw(const ThickLines &thicklines, const std::string &fill, const std::string &stroke, coord_t stroke_width)
@@ -202,9 +213,12 @@ SVG::get_path_d(const MultiPoint &mp, bool closed) const
 void
 SVG::Close()
 {
-    fprintf(this->f, "</svg>\n");
+    if (this->f != NULL) {
-    fclose(this->f);
+        fprintf(this->f, "</svg>\n");
-    printf("SVG written to %s\n", this->filename.c_str());
+        fclose(this->f);
+        this->f = NULL;
+        printf("SVG written to %s\n", this->filename.c_str());
+    }
 }
 
 }

xs/src/libslic3r/SVG.hpp

@@ -3,6 +3,7 @@
 
 #include "libslic3r.h"
 #include "ExPolygon.hpp"
+#include "ExPolygonCollection.hpp"
 #include "Line.hpp"
 #include "TriangleMesh.hpp"
 
@@ -17,12 +18,14 @@ class SVG
 
     SVG(const char* filename);
     SVG(const char* filename, const BoundingBox &bbox);
+    ~SVG();
     void draw(const Line &line, std::string stroke = "black", coord_t stroke_width = 0);
     void draw(const ThickLine &line, const std::string &fill, const std::string &stroke, coord_t stroke_width = 0);
-    void draw(const Lines &lines, std::string stroke = "black");
+    void draw(const Lines &lines, std::string stroke = "black", coord_t stroke_width = 0);
     void draw(const IntersectionLines &lines, std::string stroke = "black");
     void draw(const ExPolygon &expolygon, std::string fill = "grey");
     void draw(const ExPolygons &expolygons, std::string fill = "grey");
+    void draw(const ExPolygonCollection &coll, std::string fill = "grey");
     void draw(const Polygon &polygon, std::string fill = "grey");
     void draw(const Polygons &polygons, std::string fill = "grey");
     void draw(const Polyline &polyline, std::string stroke = "black", coord_t stroke_width = 0);

xs/src/libslic3r/libslic3r.h

@@ -64,6 +64,7 @@ constexpr auto LOOP_CLIPPING_LENGTH_OVER_NOZZLE_DIAMETER = 0.15;
 constexpr coord_t SMALL_PERIMETER_LENGTH = scale_(6.5) * 2 * PI;
 constexpr coordf_t INSET_OVERLAP_TOLERANCE = 0.4;
 constexpr coordf_t EXTERNAL_INFILL_MARGIN = 3;
+constexpr coord_t SCALED_EXTERNAL_INFILL_MARGIN = scale_(EXTERNAL_INFILL_MARGIN);
 
 enum Axis { X=0, Y, Z };