Bugfix: spAligned didn't always work in multi-island layers. #3458

xs/src/libslic3r/GCode.cpp

@@ -334,25 +334,19 @@ GCode::extrude(ExtrusionLoop loop, std::string description, double speed)
         
         // restore original winding order so that concave and convex detection always happens
         // on the right/outer side of the polygon
-        if (was_clockwise) {
+        if (was_clockwise)
-            for (Polygons::iterator p = simplified.begin(); p != simplified.end(); ++p)
+            for (Polygon &p : simplified)
-                p->reverse();
+                p.reverse();
-        }
         
         // concave vertices have priority
         Points candidates;
-        for (Polygons::const_iterator p = simplified.begin(); p != simplified.end(); ++p) {
+        for (const Polygon &p : simplified)
-            Points concave = p->concave_points(PI*4/3);
+            append_to(candidates, p.concave_points(PI*4/3));
-            candidates.insert(candidates.end(), concave.begin(), concave.end());
-        }
         
         // if no concave points were found, look for convex vertices
-        if (candidates.empty()) {
+        if (candidates.empty())
-            for (Polygons::const_iterator p = simplified.begin(); p != simplified.end(); ++p) {
+            for (const Polygon &p : simplified)
-                Points convex = p->convex_points(PI*2/3);
+                append_to(candidates, p.convex_points(PI*2/3));
-                candidates.insert(candidates.end(), convex.begin(), convex.end());
-            }
-        }
         
         // retrieve the last start position for this object
         if (this->layer != NULL) {
@@ -375,10 +369,9 @@ GCode::extrude(ExtrusionLoop loop, std::string description, double speed)
             if (!loop.split_at_vertex(point)) loop.split_at(point);
         } else if (!candidates.empty()) {
             Points non_overhang;
-            for (Points::const_iterator p = candidates.begin(); p != candidates.end(); ++p) {
+            for (const Point &p : candidates)
-                if (!loop.has_overhang_point(*p))
+                if (!loop.has_overhang_point(p))
-                    non_overhang.push_back(*p);
+                    non_overhang.push_back(p);
-            }
             
             if (!non_overhang.empty())
                 candidates = non_overhang;
@@ -389,8 +382,15 @@ GCode::extrude(ExtrusionLoop loop, std::string description, double speed)
             point = last_pos.projection_onto(polygon);
             loop.split_at(point);
         }
-        if (this->layer != NULL)
+        if (this->layer != NULL) {
+            /* Keeping a single starting point for each object works best with objects
+               having a single island. In the other cases, this works when layers are 
+               similar, so the algorithm picks the same chain of points. But when an 
+               island disappears, the others might suffer from a starting point change
+               even if their shape didn't change. We should probably keep multiple
+               starting points for each layer and test all of them. */
             this->_seam_position[this->layer->object()] = point;
+        }
     } else if (seam_position == spRandom) {
         if (loop.role == elrContourInternalPerimeter) {
             Polygon polygon = loop.polygon();

xs/src/libslic3r/Layer.cpp

@@ -98,19 +98,21 @@ Layer::make_slices()
     this->slices.expolygons.reserve(slices.size());
     
     // prepare ordering points
+    // While it's more computationally expensive, we use centroid()
+    // instead of first_point() because it's [much more] deterministic
+    // and preserves ordering across similar layers.
     Points ordering_points;
     ordering_points.reserve(slices.size());
-    for (ExPolygons::const_iterator ex = slices.begin(); ex != slices.end(); ++ex)
+    for (const ExPolygon &ex : slices)
-        ordering_points.push_back(ex->contour.first_point());
+        ordering_points.push_back(ex.contour.centroid());
     
     // sort slices
     std::vector<Points::size_type> order;
     Slic3r::Geometry::chained_path(ordering_points, order);
     
     // populate slices vector
-    for (std::vector<Points::size_type>::const_iterator it = order.begin(); it != order.end(); ++it) {
+    for (const Points::size_type &o : order)
-        this->slices.expolygons.push_back(slices[*it]);
+        this->slices.expolygons.push_back(slices[o]);
-    }
 }
 
 void