diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index 4ed9855401..73b4d90946 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -7644,13 +7644,7 @@ std::string GCode::set_extruder(unsigned int new_filament_id, double print_z, bo // In IMEX parallel modes each carriage needs an explicit tool address. // In primary mode (single active tool) regular tool changes handle PA // so no qualifier is needed — same as a non-IMEX printer. - // Guard the pem lookup: PrintApply populates pem when printer_extruder_id - // is set, but defense-in-depth prevents a throw from get_at on any - // empty-pem path that might slip through in exotic profiles. - const bool imex_parallel = !m_imex_parallel_mode.empty() && m_imex_parallel_mode != kImexPrimaryMode; - const int pa_tool = (imex_parallel && !m_config.physical_extruder_map.values.empty()) - ? m_config.physical_extruder_map.get_at((int)new_filament_id) - : -1; + const int pa_tool = imex_pem_tool_for((int)new_filament_id, m_imex_parallel_mode, m_config.physical_extruder_map); gcode += m_writer.set_pressure_advance(m_config.pressure_advance.get_at(new_filament_id), pa_tool); // Orca: Adaptive PA // Reset Adaptive PA processor last PA value @@ -7950,11 +7944,7 @@ std::string GCode::set_extruder(unsigned int new_filament_id, double print_z, bo gcode += m_ooze_prevention.post_toolchange(*this); if (m_config.enable_pressure_advance.get_at(new_filament_id)) { - // Empty-pem guard mirrors the earlier PA site; get_at throws on empty values. - const bool imex_parallel = !m_imex_parallel_mode.empty() && m_imex_parallel_mode != kImexPrimaryMode; - const int pa_tool = (imex_parallel && !m_config.physical_extruder_map.values.empty()) - ? m_config.physical_extruder_map.get_at((int)new_filament_id) - : -1; + const int pa_tool = imex_pem_tool_for((int)new_filament_id, m_imex_parallel_mode, m_config.physical_extruder_map); gcode += m_writer.set_pressure_advance(m_config.pressure_advance.get_at(new_filament_id), pa_tool); // Orca: Adaptive PA // Reset Adaptive PA processor last PA value diff --git a/src/libslic3r/IMEXHelpers.cpp b/src/libslic3r/IMEXHelpers.cpp index cc29ed77c4..592a85c539 100644 --- a/src/libslic3r/IMEXHelpers.cpp +++ b/src/libslic3r/IMEXHelpers.cpp @@ -33,6 +33,14 @@ ConfigOptionInts effective_physical_extruder_map(const PresetBundle& pb) return effective_physical_extruder_map(explicit_pem, pei); } +int imex_pem_tool_for(int filament_id, const std::string& parallel_mode, const ConfigOptionInts& pem) +{ + const bool imex_parallel = !parallel_mode.empty() && parallel_mode != kImexPrimaryMode; + if (!imex_parallel || pem.values.empty()) + return -1; + return pem.get_at(filament_id); +} + int first_filament_for_physical_head(const ConfigOptionInts& pem, int physical) { const auto& v = pem.values; diff --git a/src/libslic3r/IMEXHelpers.hpp b/src/libslic3r/IMEXHelpers.hpp index a276e6b011..6ae53c4534 100644 --- a/src/libslic3r/IMEXHelpers.hpp +++ b/src/libslic3r/IMEXHelpers.hpp @@ -37,6 +37,14 @@ ConfigOptionInts effective_physical_extruder_map(const ConfigOptionInts* explici // all agree on what the slicer will see. ConfigOptionInts effective_physical_extruder_map(const PresetBundle& pb); +// Returns the physical extruder index to emit as a per-tool qualifier (PA / temperature) +// for the given filament slot, or -1 when the current IMEX state does not warrant a +// per-tool qualification: non-IMEX / primary mode, or the pem is empty / unpopulated. +// Used at tool-change and second-layer transition call sites where IMEX parallel modes +// route emission through the physical extruder, while ordinary tool changes emit bare +// firmware commands like any non-IMEX printer. +int imex_pem_tool_for(int filament_id, const std::string& parallel_mode, const ConfigOptionInts& pem); + // Returns the lowest 0-based logical filament index L such that pem[L] == physical. // Returns -1 if no filament routes to `physical`. // Degenerate case: an empty pem returns 0 when `physical == 0` (identity-on-head-0 diff --git a/tests/fff_print/test_gcodewriter.cpp b/tests/fff_print/test_gcodewriter.cpp index ef8fb58b41..321799c935 100644 --- a/tests/fff_print/test_gcodewriter.cpp +++ b/tests/fff_print/test_gcodewriter.cpp @@ -68,6 +68,270 @@ SCENARIO("lift() is not ignored after unlift() at normal values of Z", "[GCodeWr } } +SCENARIO("set_pressure_advance emits nothing for negative PA", "[GCodeWriter][PressureAdvance]") { + GIVEN("A default GCodeWriter") { + GCodeWriter writer; + THEN("Negative PA returns empty regardless of firmware flavor") { + writer.config.gcode_flavor.value = gcfKlipper; + REQUIRE(writer.set_pressure_advance(-1.0).empty()); + writer.config.gcode_flavor.value = gcfRepRapFirmware; + REQUIRE(writer.set_pressure_advance(-0.001).empty()); + writer.config.gcode_flavor.value = gcfMarlinFirmware; + REQUIRE(writer.set_pressure_advance(-100.0).empty()); + } + } +} + +SCENARIO("set_pressure_advance emits Klipper form with optional EXTRUDER=extruder", "[GCodeWriter][PressureAdvance]") { + GIVEN("A Klipper-flavored GCodeWriter") { + GCodeWriter writer; + writer.config.gcode_flavor.value = gcfKlipper; + + WHEN("set_pressure_advance is called without a tool index") { + std::string out = writer.set_pressure_advance(0.05); + THEN("Output contains SET_PRESSURE_ADVANCE ADVANCE=0.05 with no EXTRUDER qualifier") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("SET_PRESSURE_ADVANCE")); + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("ADVANCE=0.05")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("EXTRUDER=")); + } + } + WHEN("set_pressure_advance is called with tool=0") { + std::string out = writer.set_pressure_advance(0.04, 0); + THEN("Output targets EXTRUDER=extruder (no trailing index)") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("EXTRUDER=extruder ")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("EXTRUDER=extruder0")); + } + } + WHEN("set_pressure_advance is called with tool=3") { + std::string out = writer.set_pressure_advance(0.06, 3); + THEN("Output targets EXTRUDER=extruder3") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("EXTRUDER=extruder3")); + } + } + } +} + +SCENARIO("set_pressure_advance emits RepRapFirmware form with optional D", "[GCodeWriter][PressureAdvance]") { + GIVEN("An RRF-flavored GCodeWriter") { + GCodeWriter writer; + writer.config.gcode_flavor.value = gcfRepRapFirmware; + + WHEN("set_pressure_advance is called without a tool index") { + std::string out = writer.set_pressure_advance(0.07); + THEN("Output is bare M572 S... with no D qualifier") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M572 S0.07")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" D")); + } + } + WHEN("set_pressure_advance is called with tool=0") { + std::string out = writer.set_pressure_advance(0.08, 0); + THEN("Output contains D0 (explicit tool 0, not the current-tool fallback)") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M572 D0 S0.08")); + } + } + WHEN("set_pressure_advance is called with tool=2") { + std::string out = writer.set_pressure_advance(0.09, 2); + THEN("Output contains D2") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M572 D2 S0.09")); + } + } + } +} + +SCENARIO("set_pressure_advance emits Marlin 2.x form with optional T", "[GCodeWriter][PressureAdvance]") { + GIVEN("A Marlin 2-flavored GCodeWriter") { + GCodeWriter writer; + writer.config.gcode_flavor.value = gcfMarlinFirmware; + + WHEN("set_pressure_advance is called without a tool index") { + std::string out = writer.set_pressure_advance(0.10); + THEN("Output is bare M900 K... with no T qualifier") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.1 ")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T")); + } + } + WHEN("set_pressure_advance is called with tool=0") { + std::string out = writer.set_pressure_advance(0.11, 0); + THEN("Output contains T0") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.11 T0")); + } + } + WHEN("set_pressure_advance is called with tool=1") { + std::string out = writer.set_pressure_advance(0.12, 1); + THEN("Output contains T1") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.12 T1")); + } + } + } +} + +SCENARIO("set_pressure_advance emits Marlin Legacy form without tool qualifier even when tool index is supplied", + "[GCodeWriter][PressureAdvance]") { + GIVEN("A Marlin Legacy-flavored GCodeWriter") { + GCodeWriter writer; + writer.config.gcode_flavor.value = gcfMarlinLegacy; + + WHEN("set_pressure_advance is called without a tool index") { + std::string out = writer.set_pressure_advance(0.05); + THEN("Output is bare M900 K...") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.05")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T")); + } + } + WHEN("set_pressure_advance is called with tool=2 (a hypothetical IMEX secondary)") { + std::string out = writer.set_pressure_advance(0.06, 2); + THEN("Output is still bare M900 — Marlin Legacy has no per-tool LA") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.06")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T2")); + } + } + } +} + +SCENARIO("set_temperature per-flavor command routing", "[GCodeWriter][Temperature]") { + GIVEN("temperature=210, no tool index, no wait") { + WHEN("flavor is Marlin 2") { + std::string out = GCodeWriter::set_temperature(210, gcfMarlinFirmware, false, -1, std::string()); + THEN("output is M104 S210 with no tool qualifier") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M104 S210")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("M109")); + } + } + WHEN("flavor is RepRapFirmware") { + std::string out = GCodeWriter::set_temperature(210, gcfRepRapFirmware, false, -1, std::string()); + THEN("output is G10 S210 (M104 is deprecated on RRF)") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("G10 S210")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("M104")); + } + } + WHEN("flavor is Mach3 or Machinekit") { + std::string mach3 = GCodeWriter::set_temperature(210, gcfMach3, false, -1, std::string()); + std::string machinekit = GCodeWriter::set_temperature(210, gcfMachinekit, false, -1, std::string()); + THEN("output uses P-prefix for the value instead of S") { + REQUIRE_THAT(mach3, Catch::Matchers::ContainsSubstring("M104 P210")); + REQUIRE_THAT(machinekit, Catch::Matchers::ContainsSubstring("M104 P210")); + REQUIRE_THAT(mach3, !Catch::Matchers::ContainsSubstring("S210")); + } + } + } +} + +SCENARIO("set_temperature wait=true handling per firmware", "[GCodeWriter][Temperature]") { + WHEN("flavor is Marlin 2 with wait") { + std::string out = GCodeWriter::set_temperature(210, gcfMarlinFirmware, true, -1, std::string()); + THEN("output is M109 S210 (blocking wait)") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M109 S210")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("M104")); + } + } + WHEN("flavor is MakerWare or Sailfish with wait") { + std::string mw = GCodeWriter::set_temperature(210, gcfMakerWare, true, -1, std::string()); + std::string sf = GCodeWriter::set_temperature(210, gcfSailfish, true, -1, std::string()); + THEN("output is empty — these flavors don't support blocking waits") { + REQUIRE(mw.empty()); + REQUIRE(sf.empty()); + } + } + WHEN("flavor is Teacup with wait") { + std::string out = GCodeWriter::set_temperature(210, gcfTeacup, true, -1, std::string()); + THEN("output emits M104 + a separate M116 poll (Teacup doesn't support M109)") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M104 S210")); + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M116")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("M109")); + } + } + WHEN("flavor is RepRapFirmware with wait") { + std::string out = GCodeWriter::set_temperature(210, gcfRepRapFirmware, true, -1, std::string()); + THEN("output emits G10 + M116 (same poll pattern as Teacup)") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("G10 S210")); + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M116")); + } + } +} + +SCENARIO("set_temperature per-tool qualifier routing for IMEX secondary carriages", + "[GCodeWriter][Temperature]") { + // IMEX secondary tools never go through a tool-change, so layer-change temperature + // for them is emitted via the tool-qualified static set_temperature overload. + GIVEN("temperature=220, tool=2, no wait") { + WHEN("flavor is Marlin 2") { + std::string out = GCodeWriter::set_temperature(220, gcfMarlinFirmware, false, 2, std::string()); + THEN("output contains T2 qualifier") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M104 S220 T2")); + } + } + WHEN("flavor is RepRapFirmware") { + std::string out = GCodeWriter::set_temperature(220, gcfRepRapFirmware, false, 2, std::string()); + THEN("output uses P-prefix for tool (RRF convention), not T") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("G10 S220 P2")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T2")); + } + } + WHEN("flavor is Klipper") { + std::string out = GCodeWriter::set_temperature(220, gcfKlipper, false, 1, std::string()); + THEN("output contains T1 qualifier (Klipper layer-change temperature uses T)") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M104 S220 T1")); + } + } + } +} + +SCENARIO("set_temperature instance overload forces tool=-1 on single-extruder writers", + "[GCodeWriter][Temperature]") { + // Guards against spuriously emitting `T0` on printers that only have one extruder. + // The instance overload discards the tool argument when !multiple_extruders. + GIVEN("A default GCodeWriter (multiple_extruders=false)") { + GCodeWriter writer; + writer.config.gcode_flavor.value = gcfMarlinFirmware; + + WHEN("set_temperature is called with tool=2") { + std::string out = writer.set_temperature(210, false, 2); + THEN("output has no T qualifier despite the caller passing tool=2") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M104 S210")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T")); + } + } + } + GIVEN("A GCodeWriter with multiple_extruders=true (not SEMM)") { + GCodeWriter writer; + writer.config.gcode_flavor.value = gcfMarlinFirmware; + writer.multiple_extruders = true; + + WHEN("set_temperature is called with tool=2") { + std::string out = writer.set_temperature(210, false, 2); + THEN("tool argument passes through — output contains T2") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M104 S210 T2")); + } + } + } +} + +SCENARIO("set_pressure_advance emits BBL M900 L1000 M10 regardless of tool index", + "[GCodeWriter][PressureAdvance]") { + GIVEN("A BBL-flagged GCodeWriter (the flag overrides firmware flavor routing)") { + GCodeWriter writer; + writer.set_is_bbl_machine(true); + // Flavor intentionally set to something other than the BBL branch to prove the flag wins. + writer.config.gcode_flavor.value = gcfMarlinFirmware; + + WHEN("set_pressure_advance is called without a tool index") { + std::string out = writer.set_pressure_advance(0.05); + THEN("Output is the BBL-specific M900 Kx L1000 M10 form") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.05 L1000 M10")); + } + } + WHEN("set_pressure_advance is called with a tool index") { + std::string out = writer.set_pressure_advance(0.05, 2); + THEN("BBL output is unchanged — no per-tool qualifier is emitted on BBL printers") { + REQUIRE_THAT(out, Catch::Matchers::ContainsSubstring("M900 K0.05 L1000 M10")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring(" T2")); + REQUIRE_THAT(out, !Catch::Matchers::ContainsSubstring("EXTRUDER=")); + } + } + } +} + SCENARIO("set_speed emits values with fixed-point output.", "[GCodeWriter]") { GIVEN("GCodeWriter instance") { diff --git a/tests/libslic3r/test_3mf.cpp b/tests/libslic3r/test_3mf.cpp index 7d7593948e..4cb0848869 100644 --- a/tests/libslic3r/test_3mf.cpp +++ b/tests/libslic3r/test_3mf.cpp @@ -1,7 +1,9 @@ #include "libslic3r/Model.hpp" #include "libslic3r/Format/3mf.hpp" +#include "libslic3r/Format/bbs_3mf.hpp" #include "libslic3r/Format/STL.hpp" +#include "libslic3r/Utils.hpp" #include @@ -133,6 +135,220 @@ SCENARIO("Export+Import geometry to/from 3mf file cycle", "[3mf]") { } } +SCENARIO("BBS 3MF round-trips per-plate IMEX state (parallel mode + head filament map)", "[3mf][IMEX]") { + // Regression guard for the class of bug where per-plate state silently drops through + // save/load (the variant-truncation bug was the precipitating example; IMEX plate state + // rides the same XML metadata path and is equally vulnerable). + // BBS exporter scaffolds a backup dir under temporary_dir() for the project config file; + // point it at a writable location for the test process. + set_temporary_dir(boost::filesystem::temp_directory_path().string()); + + GIVEN("A Model with a single object on plate 0 and IMEX plate state set") { + Model src_model; + std::string src_file = std::string(TEST_DATA_DIR) + "/test_3mf/Prusa.stl"; + REQUIRE(load_stl(src_file.c_str(), &src_model)); + src_model.add_default_instances(); + + DynamicPrintConfig src_config; + + PlateDataPtrs src_plates; + auto *plate0 = new PlateData(); + plate0->plate_index = 0; + plate0->config.set_key_value("imex_parallel_mode", new ConfigOptionString("copy_mode")); + plate0->config.set_key_value("imex_head_filament_map", new ConfigOptionString("1:2,2:3")); + src_plates.push_back(plate0); + + WHEN("the model is saved to BBS 3MF and loaded back") { + std::string test_file = std::string(TEST_DATA_DIR) + "/test_3mf/imex_roundtrip.3mf"; + + StoreParams store_params; + store_params.path = test_file.c_str(); + store_params.model = &src_model; + store_params.plate_data_list = src_plates; + store_params.config = &src_config; + REQUIRE(store_bbs_3mf(store_params)); + + Model dst_model; + DynamicPrintConfig dst_config; + PlateDataPtrs dst_plates; + std::vector dst_presets; + bool is_bbl = false; + bool is_orca = false; + Semver file_version; + ConfigSubstitutionContext ctxt{ ForwardCompatibilitySubstitutionRule::Disable }; + bool loaded = load_bbs_3mf(test_file.c_str(), &dst_config, &ctxt, &dst_model, + &dst_plates, &dst_presets, &is_bbl, &is_orca, &file_version); + boost::filesystem::remove(test_file); + + THEN("load succeeds") { + REQUIRE(loaded); + } + THEN("the loaded plate list has the same number of plates") { + REQUIRE(dst_plates.size() == src_plates.size()); + } + THEN("imex_parallel_mode round-trips with its original value") { + REQUIRE(dst_plates.size() >= 1); + auto *mode_opt = dst_plates[0]->config.option("imex_parallel_mode"); + REQUIRE(mode_opt != nullptr); + REQUIRE(mode_opt->value == "copy_mode"); + } + THEN("imex_head_filament_map round-trips with its original value") { + REQUIRE(dst_plates.size() >= 1); + auto *hfm_opt = dst_plates[0]->config.option("imex_head_filament_map"); + REQUIRE(hfm_opt != nullptr); + REQUIRE(hfm_opt->value == "1:2,2:3"); + } + + release_PlateData_list(dst_plates); + } + + release_PlateData_list(src_plates); + } +} + +SCENARIO("BBS 3MF round-trips distinct IMEX state across multiple plates", "[3mf][IMEX]") { + // Guards against a plate-indexing regression where IMEX metadata lands on the wrong + // plate or bleeds across plates on reload. Each plate carries distinct mode + head- + // filament-map values; the reload must reproduce them in the same order. + set_temporary_dir(boost::filesystem::temp_directory_path().string()); + + GIVEN("A Model with two plates each carrying different IMEX state") { + Model src_model; + std::string src_file = std::string(TEST_DATA_DIR) + "/test_3mf/Prusa.stl"; + REQUIRE(load_stl(src_file.c_str(), &src_model)); + src_model.add_default_instances(); + + DynamicPrintConfig src_config; + + PlateDataPtrs src_plates; + auto *plate0 = new PlateData(); + plate0->plate_index = 0; + plate0->config.set_key_value("imex_parallel_mode", new ConfigOptionString("copy_mode")); + plate0->config.set_key_value("imex_head_filament_map", new ConfigOptionString("1:2")); + src_plates.push_back(plate0); + + auto *plate1 = new PlateData(); + plate1->plate_index = 1; + plate1->config.set_key_value("imex_parallel_mode", new ConfigOptionString("mirror_mode")); + plate1->config.set_key_value("imex_head_filament_map", new ConfigOptionString("2:4,3:5")); + src_plates.push_back(plate1); + + WHEN("the model is saved to BBS 3MF and loaded back") { + std::string test_file = std::string(TEST_DATA_DIR) + "/test_3mf/imex_multiplate_roundtrip.3mf"; + + StoreParams store_params; + store_params.path = test_file.c_str(); + store_params.model = &src_model; + store_params.plate_data_list = src_plates; + store_params.config = &src_config; + REQUIRE(store_bbs_3mf(store_params)); + + Model dst_model; + DynamicPrintConfig dst_config; + PlateDataPtrs dst_plates; + std::vector dst_presets; + bool is_bbl = false; + bool is_orca = false; + Semver file_version; + ConfigSubstitutionContext ctxt{ ForwardCompatibilitySubstitutionRule::Disable }; + bool loaded = load_bbs_3mf(test_file.c_str(), &dst_config, &ctxt, &dst_model, + &dst_plates, &dst_presets, &is_bbl, &is_orca, &file_version); + boost::filesystem::remove(test_file); + + THEN("load succeeds and both plates are returned") { + REQUIRE(loaded); + REQUIRE(dst_plates.size() == 2); + } + THEN("plate 0 retains its own IMEX state (copy_mode, 1:2) and does not inherit plate 1's") { + REQUIRE(dst_plates.size() == 2); + auto *mode = dst_plates[0]->config.option("imex_parallel_mode"); + auto *hfm = dst_plates[0]->config.option("imex_head_filament_map"); + REQUIRE(mode != nullptr); + REQUIRE(hfm != nullptr); + REQUIRE(mode->value == "copy_mode"); + REQUIRE(hfm->value == "1:2"); + } + THEN("plate 1 retains its own IMEX state (mirror_mode, 2:4,3:5) and does not inherit plate 0's") { + REQUIRE(dst_plates.size() == 2); + auto *mode = dst_plates[1]->config.option("imex_parallel_mode"); + auto *hfm = dst_plates[1]->config.option("imex_head_filament_map"); + REQUIRE(mode != nullptr); + REQUIRE(hfm != nullptr); + REQUIRE(mode->value == "mirror_mode"); + REQUIRE(hfm->value == "2:4,3:5"); + } + + release_PlateData_list(dst_plates); + } + + release_PlateData_list(src_plates); + } +} + +SCENARIO("BBS 3MF does not emit IMEX metadata when plate is in primary mode", "[3mf][IMEX]") { + // The serialization guard short-circuits when the mode is empty or "primary", so loading + // a plate that was saved in primary mode must not leave a stale imex_parallel_mode option + // on the plate's config. If this regressed, we'd see ghost "primary" strings appearing on + // plates that had no IMEX state at all. + set_temporary_dir(boost::filesystem::temp_directory_path().string()); + + GIVEN("A Model with plate 0 in primary mode and an empty head-filament map") { + Model src_model; + std::string src_file = std::string(TEST_DATA_DIR) + "/test_3mf/Prusa.stl"; + REQUIRE(load_stl(src_file.c_str(), &src_model)); + src_model.add_default_instances(); + + DynamicPrintConfig src_config; + PlateDataPtrs src_plates; + auto *plate0 = new PlateData(); + plate0->plate_index = 0; + plate0->config.set_key_value("imex_parallel_mode", new ConfigOptionString("primary")); + plate0->config.set_key_value("imex_head_filament_map", new ConfigOptionString("")); + src_plates.push_back(plate0); + + WHEN("the model is saved to BBS 3MF and loaded back") { + std::string test_file = std::string(TEST_DATA_DIR) + "/test_3mf/imex_primary_roundtrip.3mf"; + + StoreParams store_params; + store_params.path = test_file.c_str(); + store_params.model = &src_model; + store_params.plate_data_list = src_plates; + store_params.config = &src_config; + REQUIRE(store_bbs_3mf(store_params)); + + Model dst_model; + DynamicPrintConfig dst_config; + PlateDataPtrs dst_plates; + std::vector dst_presets; + bool is_bbl = false; + bool is_orca = false; + Semver file_version; + ConfigSubstitutionContext ctxt{ ForwardCompatibilitySubstitutionRule::Disable }; + bool loaded = load_bbs_3mf(test_file.c_str(), &dst_config, &ctxt, &dst_model, + &dst_plates, &dst_presets, &is_bbl, &is_orca, &file_version); + boost::filesystem::remove(test_file); + + THEN("load succeeds") { + REQUIRE(loaded); + } + THEN("the loaded plate has no imex_parallel_mode option set (primary is not serialized)") { + REQUIRE(dst_plates.size() >= 1); + auto *mode_opt = dst_plates[0]->config.option("imex_parallel_mode"); + REQUIRE(mode_opt == nullptr); + } + THEN("the loaded plate has no imex_head_filament_map option set (empty is not serialized)") { + REQUIRE(dst_plates.size() >= 1); + auto *hfm_opt = dst_plates[0]->config.option("imex_head_filament_map"); + REQUIRE(hfm_opt == nullptr); + } + + release_PlateData_list(dst_plates); + } + + release_PlateData_list(src_plates); + } +} + SCENARIO("2D convex hull of sinking object", "[3mf][.]") { GIVEN("model") { // load a model diff --git a/tests/libslic3r/test_config.cpp b/tests/libslic3r/test_config.cpp index 2bee0d7d11..00cb68d58b 100644 --- a/tests/libslic3r/test_config.cpp +++ b/tests/libslic3r/test_config.cpp @@ -265,6 +265,143 @@ SCENARIO("DynamicPrintConfig serialization", "[Config]") { } } +SCENARIO("update_non_diff_values_to_base_config does not truncate stride=2 child vectors when child has more extruders than parent", + "[Config][Variant]") { + GIVEN("A 2-extruder child with stride=2 machine limits inheriting from a 1-extruder parent") { + // Stride=2 keys store (normal, silent) pairs per variant: a 2-extruder child has size 4, + // a 1-extruder parent has size 2. The truncation guard must fire here too. + Slic3r::DynamicPrintConfig child; + Slic3r::DynamicPrintConfig parent; + + child.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1, 2})); + child.set_key_value("printer_extruder_variant", new Slic3r::ConfigOptionStrings({"Direct Drive Standard", "Direct Drive Standard"})); + child.set_key_value("machine_max_acceleration_x", new Slic3r::ConfigOptionFloats({500.0, 200.0, 600.0, 300.0})); + + parent.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1})); + parent.set_key_value("printer_extruder_variant", new Slic3r::ConfigOptionStrings({"Direct Drive Standard"})); + parent.set_key_value("machine_max_acceleration_x", new Slic3r::ConfigOptionFloats({1000.0, 400.0})); + + const Slic3r::t_config_option_keys keys = { + "machine_max_acceleration_x", "printer_extruder_id", "printer_extruder_variant" + }; + const std::set different_keys = { + "machine_max_acceleration_x", "printer_extruder_id", "printer_extruder_variant" + }; + + WHEN("update_non_diff_values_to_base_config is called") { + std::string id_name = "printer_extruder_id"; + std::string var_name = "printer_extruder_variant"; + child.update_non_diff_values_to_base_config( + parent, keys, different_keys, id_name, var_name, + Slic3r::printer_options_with_variant_1, + Slic3r::printer_options_with_variant_2); + + THEN("machine_max_acceleration_x retains size 4 (2 variants × 2 silent modes)") { + REQUIRE(child.option("machine_max_acceleration_x")->values.size() == 4); + } + THEN("machine_max_acceleration_x preserves both extruders' normal and silent values") { + auto* v = child.option("machine_max_acceleration_x"); + REQUIRE_THAT(v->values[0], Catch::Matchers::WithinAbs(500.0, 1e-9)); + REQUIRE_THAT(v->values[1], Catch::Matchers::WithinAbs(200.0, 1e-9)); + REQUIRE_THAT(v->values[2], Catch::Matchers::WithinAbs(600.0, 1e-9)); + REQUIRE_THAT(v->values[3], Catch::Matchers::WithinAbs(300.0, 1e-9)); + } + } + } +} + +SCENARIO("update_non_diff_values_to_base_config runs the merge path in the equal-size case", + "[Config][Variant]") { + // Distinguishes the fix's `cur > target` guard from a stricter `cur >= target`. + // With `cur > target` (correct): equal-size does NOT fire the guard; merge runs via + // set_with_restore, which builds variant_index by matching (extruder_variant, extruder_id) + // pairs between child and parent. When the variants don't match, variant_index positions + // stay at -1, and set_with_restore overwrites those child positions with parent values. + // With `cur >= target` (regressed): guard fires; merge is skipped; child values stay intact. + // Using mismatched variants makes the two outcomes observably different. + GIVEN("A 2-extruder child and parent with matching extruder counts but mismatched variant names") { + Slic3r::DynamicPrintConfig child; + Slic3r::DynamicPrintConfig parent; + + child.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1, 2})); + child.set_key_value("printer_extruder_variant", new Slic3r::ConfigOptionStrings({"Bowden Standard", "Bowden Standard"})); + child.set_key_value("retraction_length", new Slic3r::ConfigOptionFloats({1.5, 2.5})); + + parent.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1, 2})); + parent.set_key_value("printer_extruder_variant", new Slic3r::ConfigOptionStrings({"Direct Drive Standard", "Direct Drive Standard"})); + parent.set_key_value("retraction_length", new Slic3r::ConfigOptionFloats({0.8, 0.8})); + + const Slic3r::t_config_option_keys keys = { + "retraction_length", "printer_extruder_id", "printer_extruder_variant" + }; + const std::set different_keys = { + "retraction_length", "printer_extruder_id", "printer_extruder_variant" + }; + + WHEN("update_non_diff_values_to_base_config is called") { + std::string id_name = "printer_extruder_id"; + std::string var_name = "printer_extruder_variant"; + child.update_non_diff_values_to_base_config( + parent, keys, different_keys, id_name, var_name, + Slic3r::printer_options_with_variant_1, + Slic3r::printer_options_with_variant_2); + + THEN("retraction_length retains size 2") { + REQUIRE(child.option("retraction_length")->values.size() == 2); + } + THEN("retraction_length gets parent values — proves the merge ran (guard did not fire)") { + // If the guard regressed to `cur >= target`, this path would be skipped and + // retraction_length would remain {1.5, 2.5}. The correct `cur > target` guard + // does not fire for equal-size, the merge proceeds, and with mismatched + // variants the child positions receive parent values. + auto* rl = child.option("retraction_length"); + REQUIRE_THAT(rl->values[0], Catch::Matchers::WithinAbs(0.8, 1e-9)); + REQUIRE_THAT(rl->values[1], Catch::Matchers::WithinAbs(0.8, 1e-9)); + } + } + } +} + +SCENARIO("update_non_diff_values_to_base_config truncation guard does not affect non-variant scalar keys", + "[Config][Variant]") { + // The fix is scoped to options in printer_options_with_variant_1 / _2. A non-variant scalar + // listed in `keys` and `different_keys` should hit the "nothing to do" branch and remain + // untouched regardless of child/parent extruder count mismatch. + GIVEN("A 2-extruder child inheriting from a 1-extruder parent, with a non-variant scalar key in `keys`") { + Slic3r::DynamicPrintConfig child; + Slic3r::DynamicPrintConfig parent; + + child.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1, 2})); + child.set_key_value("printer_extruder_variant", new Slic3r::ConfigOptionStrings({"Direct Drive Standard", "Direct Drive Standard"})); + child.set_key_value("layer_height", new Slic3r::ConfigOptionFloat(0.20)); + + parent.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1})); + parent.set_key_value("printer_extruder_variant", new Slic3r::ConfigOptionStrings({"Direct Drive Standard"})); + parent.set_key_value("layer_height", new Slic3r::ConfigOptionFloat(0.28)); + + const Slic3r::t_config_option_keys keys = { + "layer_height", "printer_extruder_id", "printer_extruder_variant" + }; + const std::set different_keys = { + "layer_height", "printer_extruder_id", "printer_extruder_variant" + }; + + WHEN("update_non_diff_values_to_base_config is called") { + std::string id_name = "printer_extruder_id"; + std::string var_name = "printer_extruder_variant"; + child.update_non_diff_values_to_base_config( + parent, keys, different_keys, id_name, var_name, + Slic3r::printer_options_with_variant_1, + Slic3r::printer_options_with_variant_2); + + THEN("the non-variant scalar layer_height is left unchanged on the child") { + REQUIRE_THAT(child.option("layer_height")->value, + Catch::Matchers::WithinAbs(0.20, 1e-9)); + } + } + } +} + SCENARIO("update_non_diff_values_to_base_config preserves child vectors when child has more extruders than parent", "[Config][Variant]") { GIVEN("A 2-extruder child printer config inheriting from a 1-extruder parent") { diff --git a/tests/libslic3r/test_imex_helpers.cpp b/tests/libslic3r/test_imex_helpers.cpp index 3f95a3e9d8..289e9baeab 100644 --- a/tests/libslic3r/test_imex_helpers.cpp +++ b/tests/libslic3r/test_imex_helpers.cpp @@ -49,6 +49,40 @@ TEST_CASE("effective_physical_extruder_map — IDEX ghost-color regression guard REQUIRE(first_filament_for_physical_head(pem, 1) == 1); // no longer -1 } +TEST_CASE("imex_pem_tool_for — non-parallel mode returns -1", "[IMEX]") { + // Empty mode string (not in IMEX) and "primary" (IMEX present but not parallel) both + // short-circuit to bare-firmware PA/temp emission. + auto pem = make_pem({0, 1, 2}); + REQUIRE(imex_pem_tool_for(0, "", pem) == -1); + REQUIRE(imex_pem_tool_for(1, "primary", pem) == -1); + REQUIRE(imex_pem_tool_for(2, "primary", pem) == -1); +} + +TEST_CASE("imex_pem_tool_for — parallel mode with empty pem returns -1", "[IMEX]") { + // Defense-in-depth for exotic profiles where pem never gets populated. get_at would + // throw on an empty pem; the helper must return -1 instead so the writer emits bare. + ConfigOptionInts empty_pem; + REQUIRE(imex_pem_tool_for(0, "copy_mode", empty_pem) == -1); + REQUIRE(imex_pem_tool_for(3, "mirror_mode", empty_pem) == -1); +} + +TEST_CASE("imex_pem_tool_for — identity pem routes filament to itself", "[IMEX]") { + // IDEX with printer_extruder_id = [1, 2] auto-derives pem = [0, 1]; non-MMU is identity. + auto pem = make_pem({0, 1}); + REQUIRE(imex_pem_tool_for(0, "copy_mode", pem) == 0); + REQUIRE(imex_pem_tool_for(1, "copy_mode", pem) == 1); +} + +TEST_CASE("imex_pem_tool_for — MMU collapse routes multiple logical slots to one physical", "[IMEX]") { + // 7-slot printer: first four slots share physical extruder 0 (a 4-lane MMU), + // slots 4/5/6 are independent on 1/2/3. Filament index is the logical slot; + // the helper returns the physical extruder carrying it. + auto pem = make_pem({0, 0, 0, 0, 1, 2, 3}); + REQUIRE(imex_pem_tool_for(0, "copy_mode", pem) == 0); + REQUIRE(imex_pem_tool_for(3, "copy_mode", pem) == 0); // MMU collapse: T3 logical → T0 physical + REQUIRE(imex_pem_tool_for(6, "copy_mode", pem) == 3); +} + TEST_CASE("first_filament_for_physical_head — identity pem", "[IMEX]") { auto pem = make_pem({0, 1, 2, 3}); REQUIRE(first_filament_for_physical_head(pem, 0) == 0);