Fixed extrusion of gap fill of classic perimeter generator

after recent refactoring / sorting of extrusions into LayerIslands.

src/libslic3r/Fill/Fill.cpp

@@ -539,20 +539,36 @@ void Layer::make_fills(FillAdaptive::Octree* adaptive_fill_octree, FillAdaptive:
 		}
     }
 
-    //FIXME Don't copy thin fill extrusions into fills, just use these thin fill extrusions
+	for (LayerSlice &lslice : this->lslices_ex)
-    // from the G-code export directly.
+		for (LayerIsland &island : lslice.islands) {
-#if 0
+			if (! island.thin_fills.empty()) {
-    // add thin fill regions
+				// Copy thin fills into fills packed as a collection.
-    // Unpacks the collection, creates multiple collections per path.
+				// Fills are always stored as collections, the rest of the pipeline (wipe into infill, G-code generator) relies on it.
-    // The path type could be ExtrusionPath, ExtrusionLoop or ExtrusionEntityCollection.
+				LayerRegion				  &layerm	  = *this->get_region(island.perimeters.region());
-    // Why the paths are unpacked?
+				ExtrusionEntityCollection &collection = *(new ExtrusionEntityCollection());
-	for (LayerRegion *layerm : m_regions)
+				layerm.m_fills.entities.push_back(&collection);
-	    for (const ExtrusionEntity *thin_fill : layerm->thin_fills().entities) {
+				collection.entities.reserve(island.thin_fills.size());
-	        ExtrusionEntityCollection &collection = *(new ExtrusionEntityCollection());
+				for (uint32_t fill_id : island.thin_fills)
-	        layerm->m_fills.entities.push_back(&collection);
+					collection.entities.push_back(layerm.thin_fills().entities[fill_id]->clone());
-	        collection.entities.push_back(thin_fill->clone());
+				island.fills.push_back({ island.perimeters.region(), { uint32_t(layerm.m_fills.entities.size() - 1), uint32_t(layerm.m_fills.entities.size()) } });
-	    }
+			}
-#endif
+			// Sort the fills by region ID.
+			std::sort(island.fills.begin(), island.fills.end(), [](auto &l, auto &r){ return l.region() < r.region() || (l.region() == r.region() && *l.begin() < *r.begin()); });
+			// Compress continuous fill ranges of the same region.
+			{
+				size_t k = 0;
+				for (size_t i = 0; i < island.fills.size(); ++ i) {
+					uint32_t region_id = island.fills[i].region();
+					uint32_t begin     = *island.fills[i].begin();
+					uint32_t end       = *island.fills[i].end();
+					size_t   j         = i + 1;
+					for (; j < island.fills.size() && island.fills[j].region() == region_id && *island.fills[j].begin() == end; ++ j)
+						end = *island.fills[j].end();
+					island.fills[k ++] = { region_id, { begin, end } };
+				}
+				island.fills.erase(island.fills.begin() + k, island.fills.end());
+			}
+		}
 
 #ifndef NDEBUG
 	for (LayerRegion *layerm : m_regions)

src/libslic3r/GCode.cpp

@@ -2355,21 +2355,28 @@ void GCode::process_layer_single_object(
     ExtrusionEntitiesPtr temp_fill_extrusions;
     if (const Layer *layer = layer_to_print.object_layer; layer)
         for (const LayerSlice &lslice : layer->lslices_ex) {
-            auto extrude_infill_range = [&](const LayerRegion &layerm, const ExtrusionEntityCollection &fills, const ExtrusionRange fill_range, bool ironing) {
+            auto extrude_infill_range = [&](
+                const LayerRegion &layerm, const ExtrusionEntityCollection &fills,
+                LayerExtrusionRanges::const_iterator it_fill_ranges_begin, LayerExtrusionRanges::const_iterator it_fill_ranges_end, bool ironing) {
                 // PrintObjects own the PrintRegions, thus the pointer to PrintRegion would be unique to a PrintObject, they would not
                 // identify the content of PrintRegion accross the whole print uniquely. Translate to a Print specific PrintRegion.
                 const PrintRegion &region = print.get_print_region(layerm.region().print_region_id());
                 temp_fill_extrusions.clear();
-                for (uint32_t fill_id : fill_range)
+                for (auto it_fill_range = it_fill_ranges_begin; it_fill_range != it_fill_ranges_end; ++ it_fill_range) {
-                    if (auto *eec = static_cast<ExtrusionEntityCollection*>(layerm.fills().entities[fill_id]);
+                    assert(it_fill_range->region() == it_fill_ranges_begin->region());
-                        (eec->role() == erIroning) == ironing && shall_print_this_extrusion_collection(eec, region)) {
+                    for (uint32_t fill_id : *it_fill_range) {
-                        if (eec->can_reverse())
+                        assert(dynamic_cast<ExtrusionEntityCollection*>(fills.entities[fill_id]));
-                            // Flatten the infill collection for better path planning.
+                        if (auto *eec = static_cast<ExtrusionEntityCollection*>(fills.entities[fill_id]);
-                            for (auto *ee : eec->entities)
+                            (eec->role() == erIroning) == ironing && shall_print_this_extrusion_collection(eec, region)) {
-                                temp_fill_extrusions.emplace_back(ee);
+                            if (eec->can_reverse())
-                        else
+                                // Flatten the infill collection for better path planning.
-                            temp_fill_extrusions.emplace_back(eec);
+                                for (auto *ee : eec->entities)
+                                    temp_fill_extrusions.emplace_back(ee);
+                            else
+                                temp_fill_extrusions.emplace_back(eec);
+                        }
                     }
+                }
                 if (! temp_fill_extrusions.empty()) {
                     init_layer_delayed();
                     m_config.apply(region.config());
@@ -2395,7 +2402,8 @@ void GCode::process_layer_single_object(
                     // identify the content of PrintRegion accross the whole print uniquely. Translate to a Print specific PrintRegion.
                     const PrintRegion &region = print.get_print_region(layerm.region().print_region_id());
                     bool first = true;
-                    for (uint32_t perimeter_id : island.perimeters)
+                    for (uint32_t perimeter_id : island.perimeters) {
+                        assert(dynamic_cast<const ExtrusionEntityCollection*>(layerm.perimeters().entities[perimeter_id]));
                         if (const auto *eec = static_cast<const ExtrusionEntityCollection*>(layerm.perimeters().entities[perimeter_id]);
                             shall_print_this_extrusion_collection(eec, region)) {
                             // This may not apply to Arachne, but maybe the Arachne gap fill should disable reverse as well?
@@ -2408,15 +2416,15 @@ void GCode::process_layer_single_object(
                             for (const ExtrusionEntity *ee : *eec)
                                 gcode += this->extrude_entity(*ee, comment_perimeter, -1.);
                         }
+                    }
                 };
                 auto process_infill = [&]() {
-                    for (LayerExtrusionRange fill_range : island.fills) {
+                    for (auto it = island.fills.begin(); it != island.fills.end(); ++ it) {
-                        const LayerRegion &layerm = *layer->get_region(fill_range.region());
+                        // Gather range of fill ranges with the same region.
-                        extrude_infill_range(layerm, layerm.fills(), fill_range, false /* normal extrusions, not ironing */);
+                        auto it_end = it;
-                    }
+                        for (++ it_end; it_end != island.fills.end() && it->region() == it_end->region(); ++ it_end) ;
-                    {
+                        const LayerRegion &layerm = *layer->get_region(it->region());
-                        const LayerRegion &layerm = *layer->get_region(island.perimeters.region());
+                        extrude_infill_range(layerm, layerm.fills(), it, it_end, false /* normal extrusions, not ironing */);
-                        extrude_infill_range(layerm, layerm.thin_fills(), island.thin_fills, false);
                     }
                 };
                 if (print.config().infill_first) {
@@ -2432,9 +2440,12 @@ void GCode::process_layer_single_object(
             // First Ironing changes extrusion rate quickly, second single ironing may be done over multiple perimeter regions.
             // Ironing in a second phase is safer, but it may be less efficient.
             for (const LayerIsland &island : lslice.islands) {
-                for (LayerExtrusionRange fill_range : island.fills) {
+                for (auto it = island.fills.begin(); it != island.fills.end(); ++ it) {
-                    const LayerRegion &layerm = *layer->get_region(fill_range.region());
+                    // Gather range of fill ranges with the same region.
-                    extrude_infill_range(layerm, layerm.fills(), fill_range, true /* ironing, not normal extrusions */);
+                    auto it_end = it;
+                    for (++ it_end; it_end != island.fills.end() && it->region() == it_end->region(); ++ it_end) ;
+                    const LayerRegion &layerm = *layer->get_region(it->region());
+                    extrude_infill_range(layerm, layerm.fills(), it, it_end, true /* ironing, not normal extrusions */);
                 }
             }
         }

src/libslic3r/GCode/ToolOrdering.cpp

@@ -772,6 +772,7 @@ void WipingExtrusions::ensure_perimeters_infills_order(const Print& print, const
 
                 for (const ExtrusionEntity* ee : layerm->fills()) { // iterate through all infill Collections
                     auto* fill = dynamic_cast<const ExtrusionEntityCollection*>(ee);
+                    assert(fill);
 
                     if (!is_overriddable(*fill, lt, print.config(), *object, region)
                      || is_entity_overridden(fill, copy) )
@@ -795,6 +796,7 @@ void WipingExtrusions::ensure_perimeters_infills_order(const Print& print, const
                 // Now the same for perimeters - see comments above for explanation:
                 for (const ExtrusionEntity* ee : layerm->perimeters()) { // iterate through all perimeter Collections
                     auto* fill = dynamic_cast<const ExtrusionEntityCollection*>(ee);
+                    assert(fill);
                     if (is_overriddable(*fill, lt, print.config(), *object, region) && ! is_entity_overridden(fill, copy))
                         set_extruder_override(fill, copy, (print.config().infill_first ? last_nonsoluble_extruder : first_nonsoluble_extruder), num_of_copies);
                 }

src/libslic3r/PerimeterGenerator.cpp

@@ -114,7 +114,7 @@ ExtrusionMultiPath PerimeterGenerator::thick_polyline_to_multi_path(const ThickP
     return multi_path;
 }
 
-static void variable_width(const ThickPolylines &polylines, ExtrusionRole role, const Flow &flow, std::vector<ExtrusionEntity *> &out)
+static void variable_width_classic(const ThickPolylines &polylines, ExtrusionRole role, const Flow &flow, std::vector<ExtrusionEntity *> &out)
 {
     // This value determines granularity of adaptive width, as G-code does not allow
     // variable extrusion within a single move; this value shall only affect the amount
@@ -251,7 +251,7 @@ static void fuzzy_extrusion_line(Arachne::ExtrusionLine &ext_lines, double fuzzy
 
 using PerimeterGeneratorLoops = std::vector<PerimeterGeneratorLoop>;
 
-static ExtrusionEntityCollection traverse_loops(const PerimeterGenerator::Parameters &params, const Polygons &lower_slices_polygons_cache, const PerimeterGeneratorLoops &loops, ThickPolylines &thin_walls)
+static ExtrusionEntityCollection traverse_loops_classic(const PerimeterGenerator::Parameters &params, const Polygons &lower_slices_polygons_cache, const PerimeterGeneratorLoops &loops, ThickPolylines &thin_walls)
 {
     // loops is an arrayref of ::Loop objects
     // turn each one into an ExtrusionLoop object
@@ -322,7 +322,7 @@ static ExtrusionEntityCollection traverse_loops(const PerimeterGenerator::Parame
     
     // Append thin walls to the nearest-neighbor search (only for first iteration)
     if (! thin_walls.empty()) {
-        variable_width(thin_walls, erExternalPerimeter, params.ext_perimeter_flow, coll.entities);
+        variable_width_classic(thin_walls, erExternalPerimeter, params.ext_perimeter_flow, coll.entities);
         thin_walls.clear();
     }
     
@@ -342,7 +342,7 @@ static ExtrusionEntityCollection traverse_loops(const PerimeterGenerator::Parame
         } else {
             const PerimeterGeneratorLoop &loop = loops[idx.first];
             assert(thin_walls.empty());
-            ExtrusionEntityCollection children = traverse_loops(params, lower_slices_polygons_cache, loop.children, thin_walls);
+            ExtrusionEntityCollection children = traverse_loops_classic(params, lower_slices_polygons_cache, loop.children, thin_walls);
             out.entities.reserve(out.entities.size() + children.entities.size() + 1);
             ExtrusionLoop *eloop = static_cast<ExtrusionLoop*>(coll.entities[idx.first]);
             coll.entities[idx.first] = nullptr;
@@ -623,7 +623,7 @@ void PerimeterGenerator::process_arachne(
     // Loops with the external thin walls
     ExtrusionEntityCollection  &out_loops,
     // Gaps without the thin walls
-    ExtrusionEntityCollection  &out_gap_fill,
+    ExtrusionEntityCollection  & /* out_gap_fill */,
     // Infills without the gap fills
     ExPolygons                 &out_fill_expolygons)
 {
@@ -1043,7 +1043,7 @@ void PerimeterGenerator::process_classic(
             }
         }
         // at this point, all loops should be in contours[0]
-        ExtrusionEntityCollection entities = traverse_loops(params, lower_slices_polygons_cache, contours.front(), thin_walls);
+        ExtrusionEntityCollection entities = traverse_loops_classic(params, lower_slices_polygons_cache, contours.front(), thin_walls);
         // if brim will be printed, reverse the order of perimeters so that
         // we continue inwards after having finished the brim
         // TODO: add test for perimeter order
@@ -1069,7 +1069,7 @@ void PerimeterGenerator::process_classic(
             ex.medial_axis(min, max, &polylines);
         if (! polylines.empty()) {
 			ExtrusionEntityCollection gap_fill;
-			variable_width(polylines, erGapFill, params.solid_infill_flow, gap_fill.entities);
+			variable_width_classic(polylines, erGapFill, params.solid_infill_flow, gap_fill.entities);
             /*  Make sure we don't infill narrow parts that are already gap-filled
                 (we only consider this surface's gaps to reduce the diff() complexity).
                 Growing actual extrusions ensures that gaps not filled by medial axis