Improve cancel object label placement.

* Make LabelObjects hold more state
* Place cancel object labels at proper positions in the gcode

src/libslic3r/GCode.cpp

@@ -2507,7 +2507,7 @@ void GCodeGenerator::process_layer_single_object(
 {
     bool     first     = true;
     // Delay layer initialization as many layers may not print with all extruders.
-    auto init_layer_delayed = [this, &print_instance, &layer_to_print, &first, &gcode]() {
+    auto init_layer_delayed = [this, &print_instance, &layer_to_print, &first]() {
         if (first) {
             first = false;
             const PrintObject &print_object = print_instance.print_object;
@@ -3123,16 +3123,18 @@ std::string GCodeGenerator::_extrude(
     std::string gcode;
     const std::string_view description_bridge = path_attr.role.is_bridge() ? " (bridge)"sv : ""sv;
 
+    const std::string instance_change_gcode{this->m_label_objects.maybe_change_instance()};
+    std::string travel_instance_change_gcode = m_writer.multiple_extruders ? "" : instance_change_gcode;
     if (!m_current_layer_first_position) {
         const Vec3crd point = to_3d(path.front().point, scaled(this->m_last_layer_z));
-        gcode += this->travel_to_first_position(point, unscaled(point.z()), this->m_label_objects.maybe_change_instance());
+        gcode += this->travel_to_first_position(point, unscaled(point.z()), travel_instance_change_gcode);
     } else {
         // go to first point of extrusion path
         if (!this->last_position) {
             const double z = this->m_last_layer_z;
             const std::string comment{"move to print after unknown position"};
             gcode += this->retract_and_wipe();
-            gcode += m_label_objects.maybe_change_instance();
+            gcode += travel_instance_change_gcode;
             gcode += this->m_writer.travel_to_xy(this->point_to_gcode(path.front().point), comment);
             gcode += this->m_writer.get_travel_to_z_gcode(z, comment);
         } else if ( this->last_position != path.front().point) {
@@ -3140,8 +3142,10 @@ std::string GCodeGenerator::_extrude(
             comment += description;
             comment += description_bridge;
             comment += " point";
-            const std::string travel_gcode{this->travel_to(*this->last_position, path.front().point, path_attr.role, comment, this->m_label_objects.maybe_change_instance())};
+            const std::string travel_gcode{this->travel_to(*this->last_position, path.front().point, path_attr.role, comment, travel_instance_change_gcode)};
             gcode += travel_gcode;
+        } else {
+            travel_instance_change_gcode = "";
         }
     }
 
@@ -3152,6 +3156,9 @@ std::string GCodeGenerator::_extrude(
         this->m_already_unretracted = true;
         gcode += "FIRST_UNRETRACT" + this->unretract();
     }
+    if (travel_instance_change_gcode.empty()) {
+        gcode += instance_change_gcode;
+    }
 
     if (!m_pending_pre_extrusion_gcode.empty()) {
         // There is G-Code that is due to be inserted before an extrusion starts. Insert it.
@@ -3601,10 +3608,10 @@ std::string GCodeGenerator::set_extruder(unsigned int extruder_id, double print_
         return gcode;
     }
 
-    // prepend retraction on the current extruder
+    std::string gcode{this->m_label_objects.maybe_stop_instance()};
-    std::string gcode = this->retract_and_wipe(true);
 
-    gcode += this->m_label_objects.maybe_stop_instance();
+    // prepend retraction on the current extruder
+    gcode += this->retract_and_wipe(true);
 
     // Always reset the extrusion path, even if the tool change retract is set to zero.
     m_wipe.reset_path();

src/libslic3r/GCode/LabelObjects.cpp

@@ -56,7 +56,7 @@ void LabelObjects::init(const SpanOfConstPtrs<PrintObject>& objects, LabelObject
     for (const PrintObject* po : objects)
         for (const PrintInstance& pi : po->instances())
             model_object_to_print_instances[pi.model_instance->get_object()].emplace_back(&pi);
-    
+
     // Now go through the map, assign a unique_id to each of the PrintInstances and get the indices of the
     // respective ModelObject and ModelInstance so we can use them in the tags. This will maintain
     // indices even in case that some instances are rotated (those end up in different PrintObjects)

tests/fff_print/CMakeLists.txt

@@ -16,6 +16,7 @@ add_executable(${_TEST_NAME}_tests
 	test_gcode_travels.cpp
 	test_gcodefindreplace.cpp
 	test_gcodewriter.cpp
+	test_cancel_object.cpp
     test_layers.cpp
 	test_model.cpp
 	test_multi.cpp

tests/fff_print/test_cancel_object.cpp

@@ -0,0 +1,249 @@
+#include <catch2/catch.hpp>
+#include <sstream>
+#include <fstream>
+
+#include "libslic3r/GCode.hpp"
+#include "test_data.hpp"
+
+using namespace Slic3r;
+using namespace Test;
+
+constexpr bool debug_files{false};
+
+std::string remove_object(const std::string &gcode, const int id) {
+    std::string result{gcode};
+    std::string start_token{"M486 S" + std::to_string(id) + "\n"};
+    std::string end_token{"M486 S-1\n"};
+
+    std::size_t start{result.find(start_token)};
+
+    while (start != std::string::npos) {
+        std::size_t end_token_start{result.find(end_token, start)};
+        std::size_t end{end_token_start + end_token.size()};
+        result.replace(start, end - start, "");
+        start = result.find(start_token);
+    }
+    return result;
+}
+
+TEST_CASE("Remove object sanity check", "[CancelObject]") {
+    // clang-format off
+    const std::string gcode{
+        "the\n"
+        "M486 S2\n"
+        "to delete\n"
+        "M486 S-1\n"
+        "kept\n"
+        "M486 S2\n"
+        "to also delete\n"
+        "M486 S-1\n"
+        "lines\n"
+    };
+    // clang-format on
+
+    const std::string result{remove_object(gcode, 2)};
+
+    // clang-format off
+    CHECK(result == std::string{
+        "the\n"
+        "kept\n"
+        "lines\n"
+    });
+    // clang-format on
+}
+
+void check_retraction(const std::string &gcode, double offset = 0.0) {
+    GCodeReader parser;
+    std::map<int, double> retracted;
+    unsigned count{0};
+    std::set<int> there_is_unretract;
+    int extruder_id{0};
+
+    parser.parse_buffer(
+        gcode,
+        [&](Slic3r::GCodeReader &self, const Slic3r::GCodeReader::GCodeLine &line) {
+            INFO("Line number: " + std::to_string(++count));
+            INFO("Extruder id: " + std::to_string(extruder_id));
+            if (!line.raw().empty() && line.raw().front() == 'T') {
+                extruder_id = std::stoi(std::string{line.raw().back()});
+            }
+            if (line.dist_XY(self) < std::numeric_limits<double>::epsilon()) {
+                if (line.has_e() && line.e() < 0) {
+                    retracted[extruder_id] += line.e();
+                }
+                if (line.has_e() && line.e() > 0) {
+                    INFO("Line: " + line.raw());
+                    if (there_is_unretract.count(extruder_id) == 0) {
+                        there_is_unretract.insert(extruder_id);
+                        REQUIRE(retracted[extruder_id] + offset + line.e() == Approx(0.0));
+                    } else {
+                        REQUIRE(retracted[extruder_id] + line.e() == Approx(0.0));
+                    }
+                    retracted[extruder_id] = 0.0;
+                }
+            }
+        }
+    );
+}
+
+void add_object(
+    Model &model, const std::string &name, const int extruder, const Vec3d &offset = Vec3d::Zero()
+) {
+    std::string extruder_id{std::to_string(extruder)};
+    ModelObject *object = model.add_object();
+    object->name = name;
+    ModelVolume *volume = object->add_volume(Test::mesh(Test::TestMesh::cube_20x20x20));
+    volume->set_material_id("material" + extruder_id);
+    volume->translate(offset);
+    DynamicPrintConfig config;
+    config.set_deserialize_strict({
+        {"extruder", extruder_id},
+    });
+    volume->config.assign_config(config);
+    object->add_instance();
+    object->ensure_on_bed();
+}
+
+class CancelObjectFixture
+{
+public:
+    CancelObjectFixture() {
+        config.set_deserialize_strict({
+            {"gcode_flavor", "marlin2"},
+            {"gcode_label_objects", "firmware"},
+            {"gcode_comments", "1"},
+            {"use_relative_e_distances", "1"},
+            {"wipe", "0"},
+            {"skirts", "0"},
+        });
+
+        add_object(two_cubes, "no_offset_cube", 0);
+        add_object(two_cubes, "offset_cube", 0, {30.0, 0.0, 0.0});
+
+        add_object(multimaterial_cubes, "no_offset_cube", 1);
+        add_object(multimaterial_cubes, "offset_cube", 2, {30.0, 0.0, 0.0});
+
+        retract_length = config.option<ConfigOptionFloats>("retract_length")->get_at(0);
+        retract_length_toolchange = config.option<ConfigOptionFloats>("retract_length_toolchange")
+                                        ->get_at(0);
+    }
+
+    DynamicPrintConfig config{Slic3r::DynamicPrintConfig::full_print_config()};
+
+    Model two_cubes;
+    Model multimaterial_cubes;
+
+    double retract_length{};
+    double retract_length_toolchange{};
+};
+
+TEST_CASE_METHOD(CancelObjectFixture, "Single extruder", "[CancelObject]") {
+    Print print;
+    print.apply(two_cubes, config);
+    print.validate();
+    const std::string gcode{Test::gcode(print)};
+
+    if constexpr (debug_files) {
+        std::ofstream output{"single_extruder_two.gcode"};
+        output << gcode;
+    }
+
+    SECTION("One remaining") {
+        const std::string removed_object_gcode{remove_object(gcode, 0)};
+        REQUIRE(removed_object_gcode.find("M486 S1\n") != std::string::npos);
+        if constexpr (debug_files) {
+            std::ofstream output{"single_extruder_one.gcode"};
+            output << removed_object_gcode;
+        }
+
+        check_retraction(removed_object_gcode);
+    }
+
+    SECTION("All cancelled") {
+        const std::string removed_all_gcode{remove_object(remove_object(gcode, 0), 1)};
+
+        // First retraction is not compensated - set offset.
+        check_retraction(removed_all_gcode, retract_length);
+    }
+}
+
+TEST_CASE_METHOD(CancelObjectFixture, "Multiple extruders", "[CancelObject]") {
+    auto wipe_tower = GENERATE(0, 1);
+    INFO("With wipe tower: " + std::string{wipe_tower == 0 ? "false" : "true"});
+    config.set_deserialize_strict(
+        {{"nozzle_diameter", "0.4,0.4"},
+         {"toolchange_gcode", "T[next_extruder]"},
+         {"wipe_tower", wipe_tower},
+         {"wipe_tower_x", "50.0"},
+         {"wipe_tower_y", "50.0"}}
+    );
+
+    Print print;
+    print.apply(multimaterial_cubes, config);
+    print.validate();
+
+    const std::string gcode{Test::gcode(print)};
+
+    if constexpr (debug_files) {
+        const std::string prefix = wipe_tower == 1 ? "wipe_tower_" : "";
+        std::ofstream output{prefix + "multi_extruder_two.gcode"};
+        output << gcode;
+    }
+
+    REQUIRE(gcode.find("T1\n") != std::string::npos);
+
+    SECTION("One remaining") {
+        const std::string removed_object_gcode{remove_object(gcode, 0)};
+        REQUIRE(removed_object_gcode.find("M486 S1\n") != std::string::npos);
+        if constexpr (debug_files) {
+            const std::string prefix = wipe_tower == 1 ? "wipe_tower_" : "";
+            std::ofstream output{prefix + "multi_extruder_one.gcode"};
+            output << removed_object_gcode;
+        }
+
+        check_retraction(removed_object_gcode);
+    }
+
+    SECTION("All cancelled") {
+        const std::string removed_all_gcode{remove_object(remove_object(gcode, 0), 1)};
+        if constexpr (debug_files) {
+            const std::string prefix = wipe_tower == 1 ? "wipe_tower_" : "";
+            std::ofstream output{prefix + "multi_extruder_none.gcode"};
+            output << removed_all_gcode;
+        }
+
+        check_retraction(removed_all_gcode);
+    }
+}
+
+TEST_CASE_METHOD(CancelObjectFixture, "Sequential print", "[CancelObject]") {
+    config.set_deserialize_strict({{"complete_objects", 1}});
+
+    Print print;
+    print.apply(two_cubes, config);
+    print.validate();
+    const std::string gcode{Test::gcode(print)};
+
+    if constexpr (debug_files) {
+        std::ofstream output{"sequential_print_two.gcode"};
+        output << gcode;
+    }
+
+    SECTION("One remaining") {
+        const std::string removed_object_gcode{remove_object(gcode, 0)};
+        REQUIRE(removed_object_gcode.find("M486 S1\n") != std::string::npos);
+        if constexpr (debug_files) {
+            std::ofstream output{"sequential_print_one.gcode"};
+            output << removed_object_gcode;
+        }
+
+        check_retraction(removed_object_gcode);
+    }
+
+    SECTION("All cancelled") {
+        const std::string removed_all_gcode{remove_object(remove_object(gcode, 0), 1)};
+
+        // First retraction is not compensated - set offset.
+        check_retraction(removed_all_gcode, retract_length);
+    }
+}