juli/OrcaSlicer
1
1
Fork 0
mirror of https://github.com/OrcaSlicer/OrcaSlicer.git synced 2026-10-10 17:21:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(imex): size physical_extruder_map from the nozzle count

Browse Source 
physical_extruder_map has one entry per logical extruder -- the index space of
nozzle_diameter -- and its consumers size their own arrays from that count. It was
being derived from printer_extruder_id, which is indexed by variant slot: one entry
per extruder+variant pair. An X1 Carbon has one nozzle and printer_extruder_id
{1,1}; an H2D 0.4 has two nozzles and {1,1,2,2,2}. The two spaces coincide only
when every extruder declares a single variant.

The visible effect was on the standby cool-down. set_extruder skips it when the
outgoing and incoming filaments share a physical extruder, and that check is not
gated on IMEX. With the map built from the wrong array, two filaments on a
dual-nozzle machine read as sharing one hotend and the cool-down was dropped --
caught by "Toolchange temperature commands are unchanged when the wipe tower wait
is off", which failed on all five CI platforms with the ;cooldown line missing.

Derive the identity over the nozzle count instead, the same fallback Plater.cpp
already applies where a profile authors no map. A profile counts as authoring one
only when its length matches the nozzle count, so the single-element PrintConfig
default is replaced rather than read as a one-extruder machine. Authored maps pass
through untouched, including the {1,0} numbering permutation the BBL dual-nozzle
profiles ship.

Tests pin the four branches and the length invariant the consumers depend on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clifford Garwood
2026-08-07 12:20:52 -04:00
co-authored by Claude Opus 5
parent 1a30c49993
commit df4009e734
4 changed files with 62 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libslic3r/IMEXHelpers.cpp
+11 -10
View File
@@ -1,6 +1,7 @@
#include "libslic3r/IMEXHelpers.hpp"
#include <algorithm>
#include <numeric>
#include <cassert>
#include <cctype>
#include <set>
@@ -12,17 +13,17 @@
namespace Slic3r {
ConfigOptionInts effective_physical_extruder_map(const ConfigOptionInts* explicit_pem,
const ConfigOptionInts* printer_extruder_id)
ConfigOptionInts effective_physical_extruder_map(const ConfigOptionInts* explicit_pem, int nozzle_count)
{
if (explicit_pem && explicit_pem->values.size() >= 2)
// One entry per logical extruder, so a profile has authored a map only when its length
// matches. The single-element PrintConfig default does not, and is replaced rather than
// treated as a one-extruder machine.
if (explicit_pem && nozzle_count > 0 && (int) explicit_pem->values.size() == nozzle_count)
return *explicit_pem;
ConfigOptionInts derived;
if (printer_extruder_id) {
derived.values.reserve(printer_extruder_id->values.size());
for (int v : printer_extruder_id->values)
derived.values.push_back(v - 1);
}
derived.values.resize(std::max(nozzle_count, 1));
std::iota(derived.values.begin(), derived.values.end(), 0);
return derived;
}
@@ -31,8 +32,8 @@ ConfigOptionInts effective_physical_extruder_map(const PresetBundle& pb)
const ConfigOptionInts* explicit_pem = pb.project_config.option<ConfigOptionInts>("physical_extruder_map");
if (!explicit_pem || explicit_pem->values.size() < 2)
explicit_pem = pb.printers.get_edited_preset().config.option<ConfigOptionInts>("physical_extruder_map");
const ConfigOptionInts* pei = pb.printers.get_edited_preset().config.option<ConfigOptionInts>("printer_extruder_id");
return effective_physical_extruder_map(explicit_pem, pei);
const auto* nozzles = pb.printers.get_edited_preset().config.option<ConfigOptionFloats>("nozzle_diameter");
return effective_physical_extruder_map(explicit_pem, nozzles ? (int) nozzles->values.size() : 0);
}
int imex_pem_tool_for(int filament_id, const std::string& parallel_mode, const ConfigOptionInts& pem)
src/libslic3r/IMEXHelpers.hpp
+13 -16
View File
@@ -49,8 +49,8 @@ inline constexpr const char* kImexPrimaryMode = "primary";
// resolve_filament_for_head(plate_head_filament_map, pem, physical_idx)
// (or the simpler `first_filament_for_physical_head` if no per-plate override).
//
// The reverse translation (logical → physical) is just `pem.get_at(filament_id)`,
// already encapsulated in `imex_pem_tool_for` for the per-tool-qualifier case.
// The reverse translation (logical → physical) is `pem.get_at(logical_idx)`, already
// encapsulated in `imex_pem_tool_for` for the per-tool-qualifier case.
//
// Past bugs in this class:
// - GCode PA emission used the inline `pem.get_at(filament_id)` form at two
@@ -65,21 +65,18 @@ inline constexpr const char* kImexPrimaryMode = "primary";
// route through one of the helpers below.
// =============================================================================
// Returns the effective physical_extruder_map given an optionally-explicit map and
// the printer's `printer_extruder_id`. If `explicit_pem` has size >= 2 the caller
// authored one, and it is returned verbatim. Otherwise the map is auto-derived
// from `printer_extruder_id` by converting each 1-indexed value to 0-indexed.
// Returns an empty ConfigOptionInts if neither source yields any values.
// Slice-time (PrintApply) and GUI ghost-color paths both call this so a printer
// profile without an explicit pem still gets a consistent mapping.
ConfigOptionInts effective_physical_extruder_map(const ConfigOptionInts* explicit_pem,
const ConfigOptionInts* printer_extruder_id);
// physical_extruder_map has one entry per LOGICAL extruder -- the index space of
// nozzle_diameter -- with each value a physical extruder index. It is NOT derivable from
// printer_extruder_id, which is indexed by variant slot (one entry per extruder+variant pair):
// an X1 Carbon has one nozzle and printer_extruder_id {1,1}, an H2D two nozzles and {1,1,2,2,2}.
//
// A profile has authored a map only when its length matches nozzle_count; otherwise the identity
// is returned, which is the same default upstream applies (see Plater.cpp's extruder_map).
ConfigOptionInts effective_physical_extruder_map(const ConfigOptionInts* explicit_pem, int nozzle_count);
// GUI overload: resolves the effective pem from a live PresetBundle using the
// project_config → printer preset fallback, then derives from printer_extruder_id
// if neither holds a user-authored map (size >= 2). Use this instead of open-coding
// the lookup at ghost-color, tooltip, click-gate, and cache-key call sites so they
// all agree on what the slicer will see.
// GUI overload: resolves the effective pem from a live PresetBundle using the project_config
// → printer preset fallback. Use this instead of open-coding the lookup at ghost-color,
// tooltip, click-gate and cache-key call sites so they all agree with what the slicer sees.
ConfigOptionInts effective_physical_extruder_map(const PresetBundle& pb);
// Returns the physical extruder index to emit as a per-tool qualifier (PA / temperature)
src/libslic3r/PrintApply.cpp
+4 -6
View File
@@ -1230,15 +1230,13 @@ Print::ApplyStatus Print::apply(const Model &model, DynamicPrintConfig new_full_
}
}
// Derive physical_extruder_map (0-indexed) from printer_extruder_id (1-indexed) when the
// map hasn't been explicitly configured in the printer profile (size <= 1 = default).
// This gives all firmware code a consistent slot → physical-extruder translation,
// including AFC/MMU setups where multiple tool slots share one physical extruder.
// Fill in physical_extruder_map when the printer profile has not authored one. It has one
// entry per logical extruder, so it is sized from the nozzle count -- not from
// printer_extruder_id, which is indexed by variant slot.
{
auto* pem = new_full_config.option<ConfigOptionInts>("physical_extruder_map", true);
const auto* pei = new_full_config.option<ConfigOptionInts>("printer_extruder_id");
if (pem) {
pem->values = effective_physical_extruder_map(pem, pei).values;
pem->values = effective_physical_extruder_map(pem, extruder_count).values;
// m_ori_full_print_config was snapshotted above, before this derivation, and the
// selector write-back path rebuilds m_full_print_config from that snapshot. Without
// mirroring the derived map into it, m_full_print_config keeps the unexpanded default
tests/libslic3r/test_imex_helpers.cpp
+34 -20
View File
@@ -13,38 +13,52 @@ static ConfigOptionInts make_pem(std::vector<int> v) {
return o;
}
TEST_CASE("effective_physical_extruder_map - explicit wins", "[IMEX]") {
TEST_CASE("effective_physical_extruder_map - authored map wins", "[IMEX]") {
// An AFC manifold on a 7-extruder machine: logical 0-3 all feed physical 0. Not derivable,
// so it must be honoured verbatim.
auto explicit_pem = make_pem({0, 0, 0, 0, 1, 2, 3});
auto pei = make_pem({1, 2, 3, 4}); // would derive to {0,1,2,3}
auto out = effective_physical_extruder_map(&explicit_pem, &pei);
auto out = effective_physical_extruder_map(&explicit_pem, 7);
REQUIRE(out.values == std::vector<int>{0, 0, 0, 0, 1, 2, 3});
}
TEST_CASE("effective_physical_extruder_map - default pem falls back to pei derive", "[IMEX]") {
auto default_pem = make_pem({0}); // size 1, the PrintConfig default
auto pei = make_pem({1, 2}); // 1-indexed IDEX
auto out = effective_physical_extruder_map(&default_pem, &pei);
REQUIRE(out.values == std::vector<int>{0, 1}); // 1-indexed → 0-indexed
TEST_CASE("effective_physical_extruder_map - a permutation is honoured", "[IMEX]") {
// Shipping BBL dual-nozzle profiles author {1,0}: a slicer/firmware numbering swap, not a
// sharing map. Deriving over it would silently renumber both extruders.
auto explicit_pem = make_pem({1, 0});
auto out = effective_physical_extruder_map(&explicit_pem, 2);
REQUIRE(out.values == std::vector<int>{1, 0});
}
TEST_CASE("effective_physical_extruder_map - null explicit, pei present", "[IMEX]") {
auto pei = make_pem({1, 2, 3});
auto out = effective_physical_extruder_map(nullptr, &pei);
REQUIRE(out.values == std::vector<int>{0, 1, 2});
TEST_CASE("effective_physical_extruder_map - unauthored derives the identity", "[IMEX]") {
// The PrintConfig default is a single element, which is not an authored map on a 2-extruder
// machine. Identity is what upstream itself falls back to.
auto default_pem = make_pem({0});
auto out = effective_physical_extruder_map(&default_pem, 2);
REQUIRE(out.values == std::vector<int>{0, 1});
}
TEST_CASE("effective_physical_extruder_map - both absent yields empty", "[IMEX]") {
auto out = effective_physical_extruder_map(nullptr, nullptr);
REQUIRE(out.values.empty());
TEST_CASE("effective_physical_extruder_map - result is always one entry per extruder", "[IMEX]") {
// The defining property: consumers index this map by logical extruder and size their own
// arrays from nozzle_diameter, so a shorter map is an out-of-bounds read in several of them.
for (int n : { 1, 2, 4, 7 }) {
DYNAMIC_SECTION("nozzle count " << n) {
REQUIRE((int) effective_physical_extruder_map(nullptr, n).values.size() == n);
auto stale = make_pem({0}); // wrong length: must not be mistaken for authored
REQUIRE((int) effective_physical_extruder_map(&stale, n).values.size() == n);
}
}
}
TEST_CASE("effective_physical_extruder_map - degenerate nozzle count still yields a usable map", "[IMEX]") {
// Never hand back an empty map: several consumers index it as a raw vector.
REQUIRE(effective_physical_extruder_map(nullptr, 0).values == std::vector<int>{0});
}
TEST_CASE("effective_physical_extruder_map - IDEX ghost-color regression guard", "[IMEX]") {
// Printer with printer_extruder_id = [1, 2] and no explicit pem (default {0}).
// Before the GUI fix, this scenario produced a black ghost on T1 because
// first_filament_for_physical_head({0}, 1) == -1.
// Dual extruder, no authored map. Before the GUI fix this produced a black ghost on T1
// because first_filament_for_physical_head({0}, 1) == -1.
auto default_pem = make_pem({0});
auto pei = make_pem({1, 2});
auto pem = effective_physical_extruder_map(&default_pem, &pei);
auto pem = effective_physical_extruder_map(&default_pem, 2);
REQUIRE(first_filament_for_physical_head(pem, 0) == 0);
REQUIRE(first_filament_for_physical_head(pem, 1) == 1); // no longer -1
}