mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-10-11 01:41:03 +00:00
Stop three IMEX changes from altering non-IMEX behaviour
None of these came from the review; they were found auditing the branch for the
same class of leak review comment 6 identified.
physical_extruder_map was normalised through effective_physical_extruder_map on
every Print::apply(), so any printer with more than one nozzle and no authored map
got the identity [0,1,...,n-1] where the single-element {0} default belongs. The
key carries two readings: the IMEX paths index it by logical extruder and need one
entry per extruder, while the inherited BBL paths read it through the clamping
get_at(), for which {0} means "everything is physical 0". Deriving unconditionally
imposed the IMEX reading on profiles that mean the other one, changing the config
block line and the {first_tools} / {first_filaments} / {curr_physical_extruder_id}
placeholders for multi-nozzle non-IMEX printers. Gated on is_imex; every consumer
needing the per-extruder form is already IMEX-gated, and profiles with an authored
map of the right length are unaffected either way.
That gating unmasked a latent out-of-bounds read: WipeTower's M104/M109 emitters
index m_physical_extruder_map by tool with no bounds check, which reads past the
end of the single-element default on a multi-nozzle machine. Upstream's bug, from
the BambuStudio wipe tower sync, previously hidden because the map was being
widened for everyone. Now bounds-checked, falling back to the tool's own index --
the form GCodeProcessor already uses for the same map.
Preset::save() and get_preset_differed_for_save() carried a branch storing the
full vector whenever a child and its parent had different lengths. It was written
against a set_with_nil that threw on mismatched sizes; upstream #13035 replaced
that with a tolerant version that keeps the child vector verbatim and nil-marks
only the overlapping range, and that fix was already in the tree when this branch
was rebased. Left in, it defeated the delta encoding for every user printer preset
whose extruder count differs from its parent's: a 7-extruder profile inheriting a
single-extruder base wrote out all of its per-variant retraction keys as literals,
including ones identical to the parent, pinning them against future vendor updates
while the UI still reported the preset as inheriting. Removed; the two save loops
are now identical to upstream. Note this only affects new saves -- presets already
written keep their frozen values until re-saved.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
a5ba39393e
commit
5ac1d05fcb
@@ -1350,7 +1350,7 @@ public:
|
||||
buffer += "M400\n";
|
||||
buffer += "M104";
|
||||
if (target_extruder != -1)
|
||||
buffer += (" T" + std::to_string(m_physical_extruder_map[target_extruder]));
|
||||
buffer += (" T" + std::to_string(physical_extruder_for(target_extruder)));
|
||||
buffer += " S" + std::to_string(target_temp) + " N0"; // N0 means the gcode is generated by slicer
|
||||
if (!comment.empty()) buffer += " ;" + comment;
|
||||
buffer += '\n';
|
||||
@@ -1362,7 +1362,7 @@ public:
|
||||
{
|
||||
std::string buffer = "M109";
|
||||
if (target_extruder != -1)
|
||||
buffer += (" T" + std::to_string(m_physical_extruder_map[target_extruder]));
|
||||
buffer += (" T" + std::to_string(physical_extruder_for(target_extruder)));
|
||||
buffer += " S" + std::to_string(target_temp) + " N0"; // N0 means the gcode is generated by slicer
|
||||
if (!comment.empty()) buffer += " ;" + comment;
|
||||
buffer += '\n';
|
||||
@@ -1382,6 +1382,15 @@ public:
|
||||
void set_multi_nozzle_group_result(const MultiNozzleUtils::LayeredNozzleGroupResult *multi_nozzle_group_result) { m_multi_nozzle_group_result = multi_nozzle_group_result; }
|
||||
void set_physical_extruder_map(const std::vector<int> &physical_extruder_map) { m_physical_extruder_map = physical_extruder_map; }
|
||||
|
||||
// physical_extruder_map defaults to the single element {0} and is only widened to one entry
|
||||
// per extruder on IMEX printers, so indexing it by tool is out of range on any other
|
||||
// multi-extruder machine. Fall back to the tool's own index, matching the bounds-checked
|
||||
// form GCodeProcessor uses for the same map.
|
||||
int physical_extruder_for(int tool) const
|
||||
{
|
||||
return tool >= 0 && tool < (int) m_physical_extruder_map.size() ? m_physical_extruder_map[tool] : tool;
|
||||
}
|
||||
|
||||
private:
|
||||
std::string set_normal_acceleration() {
|
||||
std::vector<unsigned int> accelerations = m_is_first_layer ? m_first_layer_normal_accelerations : m_normal_accelerations;
|
||||
|
||||
@@ -733,12 +733,6 @@ void Preset::save(DynamicPrintConfig* parent_config)
|
||||
ConfigOptionVectorBase* opt_vec_inherit = static_cast<ConfigOptionVectorBase*>(parent_config->option(option));
|
||||
if (opt_vec_src->size() == 1)
|
||||
opt_dst->set(opt_src);
|
||||
else if (opt_vec_src->size() != opt_vec_inherit->size()) {
|
||||
// Size mismatch (e.g. new multi-extruder profile inheriting from a
|
||||
// single-extruder base): nil-delta encoding requires matching sizes,
|
||||
// so fall back to storing the full vector.
|
||||
opt_dst->set(opt_src);
|
||||
}
|
||||
else if (key_set1->find(option) != key_set1->end()) {
|
||||
opt_vec_dst->set_with_nil(opt_vec_src, opt_vec_inherit, 1);
|
||||
}
|
||||
@@ -1905,12 +1899,6 @@ Preset* PresetCollection::get_preset_differed_for_save(Preset& preset)
|
||||
ConfigOptionVectorBase* opt_vec_inherit = static_cast<ConfigOptionVectorBase*>(parent_preset->config.option(option));
|
||||
if (opt_vec_src->size() == 1)
|
||||
opt_dst->set(opt_src);
|
||||
else if (opt_vec_src->size() != opt_vec_inherit->size()) {
|
||||
// Size mismatch (e.g. new multi-extruder profile inheriting from a
|
||||
// single-extruder base): nil-delta encoding requires matching sizes,
|
||||
// so fall back to storing the full vector.
|
||||
opt_dst->set(opt_src);
|
||||
}
|
||||
else if (key_set1->find(option) != key_set1->end()) {
|
||||
opt_vec_dst->set_with_nil(opt_vec_src, opt_vec_inherit, 1);
|
||||
}
|
||||
|
||||
@@ -1313,7 +1313,19 @@ Print::ApplyStatus Print::apply(const Model &model, DynamicPrintConfig new_full_
|
||||
// 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.
|
||||
{
|
||||
//
|
||||
// IMEX printers only. physical_extruder_map carries two readings in this tree (see
|
||||
// IMEXHelpers.hpp): the IMEX paths need one entry per logical extruder, the inherited BBL
|
||||
// paths read it through the clamping get_at(), which collapses every logical extruder to
|
||||
// physical 0 while the profile default is the single-element {0}. Deriving the identity
|
||||
// unconditionally would impose the IMEX reading on printers that mean the other one and
|
||||
// change what they emit -- {first_tools}/{first_filaments}/{first_non_support_*} and
|
||||
// {most_used_physical_extruder_id}/{curr_physical_extruder_id} in custom start G-code, plus
|
||||
// the `; physical_extruder_map =` line in the G-code config block -- for every multi-nozzle
|
||||
// profile that never authored a map. Non-IMEX printers therefore keep the config value
|
||||
// exactly as it arrives, which is what upstream slices with.
|
||||
const auto* is_imex_opt = new_full_config.option<ConfigOptionBool>("is_imex");
|
||||
if (is_imex_opt && is_imex_opt->value) {
|
||||
auto* pem = new_full_config.option<ConfigOptionInts>("physical_extruder_map", true);
|
||||
if (pem) {
|
||||
pem->values = effective_physical_extruder_map(pem, extruder_count).values;
|
||||
|
||||
Reference in New Issue
Block a user