From 3c31856b8557defc15c6d08c09dbe139f3dd6303 Mon Sep 17 00:00:00 2001 From: Clifford Garwood Date: Fri, 18 Sep 2026 00:54:54 -0400 Subject: [PATCH] Bound the second-layer temperature lookup in filament-slot space nozzle_temperature is variant-expanded, so its length is columns rather than filament slots. On the dynamic-nozzle path that makes it longer than the slot count, and an out-of-slot index reached get_filament_config_index() and came back as filament 0 - the clamp the bounds-checking was meant to remove. Bound by the slot count instead; on the ordinary path the two are equal and nothing changes. The is_extruder_used write gains the matching lower-bound guard. IMEXHelpers.hpp now states both halves of the rule its call sites follow. Bound anything derived from the extruder map against the filament slot count before using it as a filament id, not against the option about to be read. And a miss is -1, which is a correct tool qualifier but matches no physical head, so it cannot serve as a skip-the-primary sentinel: which head prints a filament is answered by the filament and the map, never by a mode role, since a primary-mode print may use any or all tools, one at a time. The consequence is recorded there rather than left implicit. The two skip sites skip nothing for a slot past the end of the map, so a plate with more slots than nozzles double-writes the primary's pressure advance. It is narrow and unreported, and a guard there would be a smaller change than naming a head. The header also records that RepRapFirmware sends an unqualified pressure advance as M572 D0, naming drive 0 absolutely rather than the active tool. Co-Authored-By: Claude Opus 5 (1M context) --- src/libslic3r/GCode.cpp | 23 +++++++++++++++++------ src/libslic3r/IMEXHelpers.hpp | 17 +++++++++++++++-- tests/libslic3r/test_imex_helpers.cpp | 18 ++++++++++++++++++ 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index 6abaae9f5b..4a2b65cf5e 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -2883,8 +2883,8 @@ static std::vector get_imex_active_tools(const Print& print) if (active_mode == kImexPrimaryMode) return active_tools; - // An unresolved mode, and a mode the tools array is too short to cover, both hand back - // an empty tools string, which parses to no tools. + // An unresolved mode, and a mode the tools array is too short to cover, both hand back an + // empty tools string, which parses to no tools. for (const auto& [phys, role] : parse_imex_active_tools(find_imex_mode(print.config(), active_mode).active_tools)) active_tools.push_back(phys); return active_tools; @@ -3418,16 +3418,23 @@ void GCode::_do_export(Print& print, GCodeOutputStream &file, ThumbnailsGenerato const auto plate_head_map = parse_imex_head_filament_map( print.objects().front()->config().imex_head_filament_map.value); const ConfigOptionInts& pem = print.config().physical_extruder_map; - // Bounds-checked, not get_at() -- see IMEXHelpers.hpp. The value is a skip-primary - // sentinel below, so a clamp would suppress whichever head sits at pem[0]. + // Bounds-checked, not get_at(): a clamp would suppress whichever head sits at pem[0]. + // A miss stays -1 and matches no head, so nothing is skipped -- see IMEXHelpers.hpp. const int primary_physical = ((int) initial_extruder_id >= 0 && (int) initial_extruder_id < (int) pem.values.size()) ? pem.values[(int) initial_extruder_id] : -1; + // Bound by the array, not by the filament count. The array is padded to + // MAXIMUM_EXTRUDER_NUMBER on purpose: start G-code addresses HEADS through it + // (fdm_toolchanger_common.json gates M104 T0..T5 on it), and a parallel copy print has + // more heads than filaments by definition. PlaceholderParser clamps an out-of-range + // first_layer_temperature read to filament 0, which is the right temperature when every + // head is printing the same filament. Narrowing this to the filament count leaves the + // secondary carriages unheated. for (int logical : imex_secondary_logical_slots( get_imex_active_tools(print), primary_physical, plate_head_map, pem)) - if (logical < (int)is_extruder_used.size()) + if (logical >= 0 && logical < (int) is_extruder_used.size()) is_extruder_used[logical] = true; } @@ -5955,7 +5962,11 @@ LayerResult GCode::process_layer( // Mutually exclusive with the `else` below, so a head skipped here gets no // transition at all. `tool_idx` is physical; the printing head uses this layer's // own filament, the parallel carriages resolve through the head map. - const int num_filament_columns = (int)print.config().nozzle_temperature.values.size(); + // nozzle_temperature is variant-expanded, so its length is columns, not slots: + // bound in slot space, or an out-of-slot logical reaches get_filament_config_index + // and comes back as filament 0. + const int num_filament_columns = std::min((int) print.config().nozzle_temperature.values.size(), + (int) print.config().filament_diameter.values.size()); // Bounds-checked, not get_at() -- see IMEXHelpers.hpp. A clamp would hand the // "initial" branch below to whichever secondary sits on pem[0], giving it the wrong // filament's transition temperature and never its own. diff --git a/src/libslic3r/IMEXHelpers.hpp b/src/libslic3r/IMEXHelpers.hpp index 9c58ca7029..a60c006a72 100644 --- a/src/libslic3r/IMEXHelpers.hpp +++ b/src/libslic3r/IMEXHelpers.hpp @@ -122,8 +122,21 @@ std::vector imex_mode_table(const ConfigBase& cfg); // // The same nozzle-vs-slot divergence bites in the other direction: anything derived from pem // -- `resolve_filament_for_head`, `first_filament_for_physical_head` -- answers in NOZZLE index -// space, so bound it against the filament array you are about to index before using it as a -// filament id. +// space, so bound it against the FILAMENT SLOT COUNT before using it as a filament id. Not +// against the option you are about to read: a variant-expanded option such as +// nozzle_temperature is as long as its columns, and is_extruder_used is padded to +// MAXIMUM_EXTRUDER_NUMBER, so both admit slots the project does not have. +// +// A miss is -1, and -1 is a correct tool QUALIFIER but not a skip-the-primary sentinel: no +// physical head equals it, so a test written as `head == primary` stops skipping anything. Do +// not substitute the mode's declared primary there. Which head prints a filament is answered by +// the filament and pem, never by a mode role: a primary-mode print may use any or all tools, one +// at a time, and pem is what says which. When pem cannot answer, the head is genuinely unknown - +// an omitted qualifier then applies to the active tool, which is the head printing that filament, +// so the honest options are to leave it unqualified or to emit nothing, not to name a carriage. +// Known gap, unreported: the two skip sites in GCode.cpp therefore skip nothing for a filament +// slot past the end of pem, so a plate with more slots than nozzles double-writes the primary's +// pressure advance and hands it another filament's transition temperature. // // -1 as a tool qualifier reaches GCodeWriter::set_pressure_advance, which omits the qualifier // on Klipper, Marlin and BBL but substitutes the historical `D0` on RepRapFirmware diff --git a/tests/libslic3r/test_imex_helpers.cpp b/tests/libslic3r/test_imex_helpers.cpp index 8c78e6a88c..710de4956a 100644 --- a/tests/libslic3r/test_imex_helpers.cpp +++ b/tests/libslic3r/test_imex_helpers.cpp @@ -400,6 +400,24 @@ TEST_CASE("imex_secondary_logical_slots - drops unrouted physicals, deduplicates REQUIRE(out2 == std::vector{0}); // both secondaries point at slot 0; only emitted once } +TEST_CASE("imex_secondary_logical_slots - a -1 primary skips nothing", "[IMEX]") { + // Why callers must not pass the pem-miss sentinel as the primary: no physical head equals + // -1, so the primary is enumerated like a secondary and its first-routed slot is returned. + // Downstream that marks the primary's slot as loaded, emits a second pressure advance over + // the one set_extruder() already wrote, and hands it another filament's transition + // temperature. Take the primary from the mode instead (imex_primary_tool_for_mode). + ConfigOptionInts pem; pem.values = {0, 1}; + CHECK(imex_secondary_logical_slots({0, 1}, /*primary*/0, {}, pem) == std::vector{1}); + CHECK(imex_secondary_logical_slots({0, 1}, /*primary*/-1, {}, pem) == std::vector{0, 1}); +} + +TEST_CASE("imex_primary_tool_for_mode - names the declared primary, -1 when there is none", "[IMEX]") { + CHECK(imex_primary_tool_for_mode("0:P,1:C") == 0); + CHECK(imex_primary_tool_for_mode("2:P,0:C,1:C") == 2); + CHECK(imex_primary_tool_for_mode("0:C,1:C") == -1); + CHECK(imex_primary_tool_for_mode("") == -1); +} + TEST_CASE("imex_secondary_logical_slots - only-primary-active returns empty", "[IMEX]") { // Primary mode (just T0 active) → no secondaries. auto pem = make_pem({0, 0, 0, 0, 1, 2, 3});