Improve bridge detection algorithm. #2477

t/bridges.t

@@ -1,4 +1,4 @@
-use Test::More tests => 16;
+use Test::More tests => 18;
 use strict;
 use warnings;
 
@@ -82,13 +82,11 @@ 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:
+    # GH #2477: This test case failed when we computed coverage by summing length of centerlines
-    # This rectangle-shaped bridge is actually unsupported (i.e. the potential anchors are
+    # instead of summing their covered area.
-    # 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]),
+        Slic3r::Polygon->new([30299990,14299990],[1500010,14299990],[1500010,1500010],[30299990,1500010]),
     );
     my $lower = [
         Slic3r::ExPolygon->new(

xs/src/libslic3r/BridgeDetector.cpp

@@ -3,6 +3,8 @@
 #include "Geometry.hpp"
 #include <algorithm>
 
+#include "SVG.hpp"
+
 namespace Slic3r {
 
 BridgeDetector::BridgeDetector(const ExPolygon &_expolygon, const ExPolygonCollection &_lower_slices,
@@ -32,7 +34,7 @@ BridgeDetector::BridgeDetector(const ExPolygon &_expolygon, const ExPolygonColle
         svg.draw(this->expolygon);
         svg.draw(this->lower_slices, "red");
         svg.draw(this->_anchors, "yellow");
-        //svg.draw(this->_edges, "black", scale_(0.2));
+        svg.draw(this->_edges, "black", scale_(0.2));
         svg.Close();
     }
     #endif
@@ -63,14 +65,9 @@ BridgeDetector::detect_angle()
             angles.push_back(i * this->resolution);
     
         // we also test angles of each bridge contour
-        {
+        for (const Polygon &p : (Polygons)this->expolygon)
-            Polygons pp = this->expolygon;
+            for (const Line &line : p.lines())
-            for (Polygons::const_iterator p = pp.begin(); p != pp.end(); ++p) {
+                angles.push_back(line.direction());
-                Lines lines = p->lines();
-                for (Lines::const_iterator line = lines.begin(); line != lines.end(); ++line)
-                    angles.push_back(line->direction());
-            }
-        }
     
         /*  we also test angles of each open supporting edge
             (this finds the optimal angle for C-shaped supports) */
@@ -97,7 +94,7 @@ BridgeDetector::detect_angle()
             candidates.push_back(BridgeDirection(angle));
     }
     
-    const double line_increment = this->extrusion_width;
+    const coord_t line_increment = this->extrusion_width;
     bool have_coverage = false;
     for (BridgeDirection &candidate : candidates) {
         Polygons my_clip_area = clip_area;
@@ -120,30 +117,24 @@ BridgeDetector::detect_angle()
         
         const Lines clipped_lines = intersection_ln(lines, my_clip_area);
         
-        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();
+            candidate.max_length = std::max(candidate.max_length, line.length());
-            lengths.push_back(len);
+            // Calculate coverage as actual covered area, because length of centerlines
-            total_length += len;
+            // is not accurate enough when such lines are slightly skewed and not parallel
+            // to the sides; calculating area will compute them as triangles.
+            // TODO: use a faster algorithm for computing covered area by using a sweep line
+            // instead of intersecting many lines.
+            candidate.coverage += Slic3r::Geometry::area(intersection(
+                my_clip_area,
+                offset((Polyline)line, +this->extrusion_width/2)
+            ));
         }
-        if (total_length) have_coverage = true;
+        if (candidate.coverage > 0) have_coverage = true;
-        
-        // sum length of bridged lines
-        candidate.coverage = total_length;
-        
-        /*  The following produces more correct results in some cases and more broken in others.
-            TODO: investigate, as it looks more reliable than line clipping. */
-        // $directions_coverage{$angle} = sum(map $_->area, @{$self->coverage($angle)}) // 0;
-        
-        // max length of bridged lines
-        if (!lengths.empty())
-            candidate.max_length = *std::max_element(lengths.begin(), lengths.end());
     }
     
     // if no direction produced coverage, then there's no bridge direction
@@ -187,21 +178,21 @@ BridgeDetector::coverage(double angle) const
     
     // Compute trapezoids according to a vertical orientation
     Polygons trapezoids;
-    for (ExPolygons::const_iterator it = grown.begin(); it != grown.end(); ++it)
+    for (const ExPolygon &e : grown)
-        it->get_trapezoids2(&trapezoids, PI/2.0);
+        e.get_trapezoids2(&trapezoids, PI/2.0);
     
     // get anchors, convert them to Polygons and rotate them too
     Polygons anchors;
-    for (ExPolygons::const_iterator anchor = this->_anchors.begin(); anchor != this->_anchors.end(); ++anchor) {
+    for (const ExPolygon &anchor : this->_anchors) {
-        Polygons pp = *anchor;
+        Polygons pp = anchor;
-        for (Polygons::iterator p = pp.begin(); p != pp.end(); ++p)
+        for (Polygon &p : pp)
-            p->rotate(PI/2.0 - angle, Point(0,0));
+            p.rotate(PI/2.0 - angle, Point(0,0));
-        anchors.insert(anchors.end(), pp.begin(), pp.end());
+        append_to(anchors, pp);
     }
     
     Polygons covered;
-    for (Polygons::const_iterator trapezoid = trapezoids.begin(); trapezoid != trapezoids.end(); ++trapezoid) {
+    for (const Polygon &trapezoid : trapezoids) {
-        Lines supported = intersection_ln(trapezoid->lines(), anchors);
+        Lines supported = intersection_ln(trapezoid.lines(), anchors);
         
         // not nice, we need a more robust non-numeric check
         for (size_t i = 0; i < supported.size(); ++i) {
@@ -211,7 +202,7 @@ BridgeDetector::coverage(double angle) const
             }
         }
 
-        if (supported.size() >= 2) covered.push_back(*trapezoid);        
+        if (supported.size() >= 2) covered.push_back(trapezoid);        
     }
     
     // merge trapezoids and rotate them back
@@ -251,10 +242,8 @@ BridgeDetector::unsupported_edges(double angle) const
     
     // get bridge edges (both contour and holes)
     Polylines bridge_edges;
-    {
+    for (const Polygon &p : (Polygons)this->expolygon)
-        Polygons pp = this->expolygon;
+        bridge_edges.push_back(p.split_at_first_point());
-        bridge_edges.insert(bridge_edges.end(), pp.begin(), pp.end());  // this uses split_at_first_point()
-    }
     
     // get unsupported edges
     Polylines _unsupported = diff_pl(
@@ -269,13 +258,10 @@ BridgeDetector::unsupported_edges(double angle) const
         direction might still benefit from anchors if long enough)
         double angle_tolerance = PI / 180.0 * 5.0; */
     Polylines unsupported;
-    for (Polylines::const_iterator polyline = _unsupported.begin(); polyline != _unsupported.end(); ++polyline) {
+    for (const Polyline &polyline : _unsupported)
-        Lines lines = polyline->lines();
+        for (const Line &line : polyline.lines())
-        for (Lines::const_iterator line = lines.begin(); line != lines.end(); ++line) {
+            if (!Slic3r::Geometry::directions_parallel(line.direction(), angle))
-            if (!Slic3r::Geometry::directions_parallel(line->direction(), angle))
+                unsupported.push_back(line);
-                unsupported.push_back(*line);
-        }
-    }
     return unsupported;
     
     /*

xs/src/libslic3r/BridgeDetector.hpp

@@ -15,7 +15,7 @@ public:
     // Lower slices, all regions.
     ExPolygonCollection lower_slices;
     // Scaled extrusion width of the infill.
-    double extrusion_width;
+    coord_t extrusion_width;
     // Angle resolution for the brute force search of the best bridging angle.
     double resolution;
     // The final optimal angle.

xs/src/libslic3r/Geometry.cpp

@@ -296,13 +296,27 @@ template<class T>
 bool
 contains(const std::vector<T> &vector, const Point &point)
 {
-    for (typename std::vector<T>::const_iterator it = vector.begin(); it != vector.end(); ++it) {
+    for (const T &it : vector)
-        if (it->contains(point)) return true;
+        if (it.contains(point))
-    }
+            return true;
+    
     return false;
 }
+template bool contains(const Polygons &vector, const Point &point);
 template bool contains(const ExPolygons &vector, const Point &point);
 
+template<class T>
+double
+area(const std::vector<T> &vector)
+{
+    double area = 0;
+    for (const T &it : vector)
+        area += it.area();
+    
+    return area;
+}
+template double area(const Polygons &vector);
+
 double
 rad2deg(double angle)
 {

xs/src/libslic3r/Geometry.hpp

@@ -20,6 +20,7 @@ void chained_path(const Points &points, std::vector<Points::size_type> &retval);
 template<class T> void chained_path_items(Points &points, T &items, T &retval);
 bool directions_parallel(double angle1, double angle2, double max_diff = 0);
 template<class T> bool contains(const std::vector<T> &vector, const Point &point);
+template<class T> double area(const std::vector<T> &vector);
 double rad2deg(double angle);
 double rad2deg_dir(double angle);
 double deg2rad(double angle);

xs/src/libslic3r/LayerRegion.cpp

@@ -1,10 +1,16 @@
 #include "Layer.hpp"
 #include "BridgeDetector.hpp"
 #include "ClipperUtils.hpp"
+#include "Geometry.hpp"
 #include "PerimeterGenerator.hpp"
 #include "Print.hpp"
 #include "Surface.hpp"
 
+#include "SVG.hpp"
+
+
+
+
 namespace Slic3r {
 
 Flow
@@ -65,19 +71,50 @@ LayerRegion::make_perimeters(const SurfaceCollection &slices, SurfaceCollection*
     g.process();
 }
 
-// This function reads lower_layer->slices and writes this->bridged and this->fill_surfaces,
+// This function reads layer->slices andlower_layer->slices
-// so it's thread-safe.
+// and writes this->bridged and this->fill_surfaces, so it's thread-safe.
 void
 LayerRegion::process_external_surfaces()
 {
-    const Surfaces &surfaces = this->fill_surfaces.surfaces;
+    Surfaces &surfaces = this->fill_surfaces.surfaces;
+    
+    for (size_t j = 0; j < surfaces.size(); ++j) {
+        Surface &surface = surfaces[j];
+        
+        if (this->layer()->lower_layer != NULL && surface.is_bridge()) {
+            // If this bridge has one or more holes that are internal surfaces
+            // (thus not visible from the outside), like a slab sustained by 
+            // pillars, include them in the bridge in order to have better and
+            // more continuous bridging.
+            Polygons &holes = surface.expolygon.holes;
+            for (int i = 0; i < holes.size(); ++i) {
+                // reverse the hole and consider it a polygon
+                Polygon h = holes[i];
+                h.reverse();
+            
+                // Is this hole fully contained in the layer slices?
+                if (diff(h, this->layer()->slices).empty()) {
+                    // remove any other surface contained in this hole
+                    for (int k = 0; k < surfaces.size(); ++k) {
+                        if (k == j) continue;
+                        if (h.contains(surfaces[k].expolygon.contour.first_point())) {
+                            surfaces.erase(surfaces.begin() + k);
+                            --k;
+                        }
+                    }
+                    
+                    holes.erase(holes.begin() + i);
+                    --i;
+                }
+            }
+        }
+    }
     
     SurfaceCollection bottom;
+    Polygons removed_holes;
     for (const Surface &surface : surfaces) {
         if (!surface.is_bottom()) continue;
         
-        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
             also, supply the original expolygon instead of the grown one, because in case
@@ -104,6 +141,7 @@ LayerRegion::process_external_surfaces()
             }
         }
         
+        const ExPolygons grown = offset_ex(surface.expolygon, +SCALED_EXTERNAL_INFILL_MARGIN);
         Surface templ = surface;
         templ.bridge_angle = angle;
         bottom.append(grown, templ);

xs/src/libslic3r/SVG.cpp

@@ -92,10 +92,9 @@ SVG::draw(const ExPolygon &expolygon, std::string fill)
     this->fill = fill;
     
     std::string d;
-    Polygons pp = expolygon;
+    for (const Polygon &p : (Polygons)expolygon)
-    for (Polygons::const_iterator p = pp.begin(); p != pp.end(); ++p) {
+        d += this->get_path_d(p, true) + " ";
-        d += this->get_path_d(*p, true) + " ";
+    
-    }
     this->path(d, true);
 }
 
@@ -105,11 +104,18 @@ SVG::draw(const ExPolygonCollection &coll, std::string fill)
     this->draw(coll.expolygons, fill);
 }
 
+void
+SVG::draw(const SurfaceCollection &coll, std::string fill)
+{
+    for (const Surface &s : coll.surfaces)
+        this->draw(s.expolygon, fill);
+}
+
 void
 SVG::draw(const ExPolygons &expolygons, std::string fill)
 {
-    for (ExPolygons::const_iterator it = expolygons.begin(); it != expolygons.end(); ++it)
+    for (const ExPolygon &e : expolygons)
-        this->draw(*it, fill);
+        this->draw(e, fill);
 }
 
 void

xs/src/libslic3r/SVG.hpp

@@ -5,6 +5,7 @@
 #include "ExPolygon.hpp"
 #include "ExPolygonCollection.hpp"
 #include "Line.hpp"
+#include "SurfaceCollection.hpp"
 #include "TriangleMesh.hpp"
 
 namespace Slic3r {
@@ -26,6 +27,7 @@ class SVG
     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 SurfaceCollection &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/SurfaceCollection.hpp

@@ -41,6 +41,7 @@ class SurfaceCollection
     bool empty() const { return this->surfaces.empty(); };
     size_t size() const { return this->surfaces.size(); };
     void clear() { this->surfaces.clear(); };
+    void erase(size_t i) { this->surfaces.erase(this->surfaces.begin() + i); };
 };
 
 }