From 7de85397a603ef488155096df8ce4c977f6520b7 Mon Sep 17 00:00:00 2001 From: Hanif Koh Date: Mon, 19 Oct 2026 18:49:44 +0800 Subject: [PATCH] Keep a Disabled Filament Override Off After a Restart A child preset stores "nil" both for an override the user switched off and for a per-variant column it inherits from its parent, and the loader filled every nil from the parent. Whenever the parent preset sets Retract Before Wipe or another override, switching it off lasted only until the next restart, project open or newer cloud copy. Mark the filament overrides on their option definition with nil_means_disabled and make is_filament_extruder_override_key answer from it. Preset::save and the project-embedded preset diff write such an option with all its columns, and the loader keeps its nil columns while still mapping them onto the parent's variants by name, so nil in one of these keys can only mean "off". Every other nullable option keeps nil as "inherit this column". The cloud diff only force-emits a disabled override whose default is a real value. --- src/libslic3r/Config.hpp | 11 +++- src/libslic3r/Preset.cpp | 13 ++-- src/libslic3r/PrintConfig.cpp | 20 ++++-- tests/libslic3r/test_config.cpp | 65 +++++++++++++++++++ .../libslic3r/test_preset_bundle_loading.cpp | 55 ++++++++++++++++ 5 files changed, 150 insertions(+), 14 deletions(-) diff --git a/src/libslic3r/Config.hpp b/src/libslic3r/Config.hpp index 9d76262ec6..427818a6aa 100644 --- a/src/libslic3r/Config.hpp +++ b/src/libslic3r/Config.hpp @@ -377,7 +377,8 @@ public: virtual void set(const ConfigOption* rhs, size_t start, size_t len) = 0; virtual void set_with_restore(const ConfigOptionVectorBase* rhs, std::vector& restore_index, int stride) = 0; virtual void set_with_restore_2(const ConfigOptionVectorBase* rhs, std::vector& restore_index, int start, int len, bool skip_error = false) = 0; - virtual void set_only_diff(const ConfigOptionVectorBase* rhs, std::vector& diff_index, int stride) = 0; + // keep_nil: copy a nil column from rhs as well, for options whose nil means "disabled" rather than "inherit". + virtual void set_only_diff(const ConfigOptionVectorBase* rhs, std::vector& diff_index, int stride, bool keep_nil = false) = 0; virtual void set_to_index(const ConfigOptionVectorBase* rhs, std::vector& dest_index, int stride) = 0; virtual void set_with_nil(const ConfigOptionVectorBase* rhs, const ConfigOptionVectorBase* inherits, int stride) = 0; // Resize the vector of values, copy the newly added values from opt_default if provided. @@ -580,7 +581,7 @@ public: //set a item related with extruder variants when loading user config, only set the different value of some extruder //rhs: item from user config //diff_index: which index in this vector need to be set - virtual void set_only_diff(const ConfigOptionVectorBase* rhs, std::vector& diff_index, int stride) override + virtual void set_only_diff(const ConfigOptionVectorBase* rhs, std::vector& diff_index, int stride, bool keep_nil) override { if (rhs->type() == this->type()) { // Assign the first value of the rhs vector. @@ -592,7 +593,7 @@ public: for (size_t i = 0; i < diff_index.size(); i++) { if (diff_index[i] != -1) { for (size_t j = 0; j < stride; j++) { - if (!other->is_nil(diff_index[i] * stride)) + if (keep_nil || !other->is_nil(diff_index[i] * stride)) this->values[i * stride + j] = other->values[diff_index[i] * stride + j]; } } @@ -2311,6 +2312,10 @@ public: ConfigOptionType type = coNone; // If a type is nullable, then it accepts a "nil" value (scalar) or "nil" values (vector). bool nullable = false; + // A nullable option whose "nil" means "switched off" (a filament override), not "no value here, + // inherit the parent's". Child presets store and load such a nil verbatim instead of folding it + // into the parent's value. + bool nil_means_disabled = false; // Default value of this option. The default value object is owned by ConfigDef, it is released in its destructor. Slic3r::clonable_ptr default_value; void set_default_value(const ConfigOption* ptr) { this->default_value = Slic3r::clonable_ptr(ptr); } diff --git a/src/libslic3r/Preset.cpp b/src/libslic3r/Preset.cpp index 4ae928cc95..247031cf00 100644 --- a/src/libslic3r/Preset.cpp +++ b/src/libslic3r/Preset.cpp @@ -797,7 +797,7 @@ bool Preset::save(DynamicPrintConfig* parent_config) { ConfigOption *opt_src = config.option(option); ConfigOption *opt_dst = temp_config.option(option, true); - if (opt_dst->is_scalar() || !(opt_dst->nullable())) + if (opt_dst->is_scalar() || !(opt_dst->nullable()) || is_filament_extruder_override_key(option)) opt_dst->set(opt_src); else { ConfigOptionVectorBase* opt_vec_src = static_cast(opt_src); @@ -2131,7 +2131,7 @@ Preset* PresetCollection::get_preset_differed_for_save(Preset& preset) { ConfigOption *opt_src = preset.config.option(option); ConfigOption *opt_dst = temp_config.option(option, true); - if (opt_dst->is_scalar() || !(opt_dst->nullable())) + if (opt_dst->is_scalar() || !(opt_dst->nullable()) || is_filament_extruder_override_key(option)) opt_dst->set(opt_src); else { ConfigOptionVectorBase* opt_vec_src = static_cast(opt_src); @@ -2198,9 +2198,12 @@ int PresetCollection::get_differed_values_to_update(Preset& preset, std::map 100%), silently // resurrecting an override the user turned off. See GitHub issue on Retract Before Wipe. if (m_type == Preset::TYPE_FILAMENT) { - for (const std::string& opt_key : filament_extruder_override_keys) { - if (key_values.count(opt_key)) - continue; // already carried by the diff + for (const std::string& opt_key : preset.config.keys()) { + if (!is_filament_extruder_override_key(opt_key) || key_values.count(opt_key)) + continue; // not an override, or already carried by the diff + const ConfigOptionDef *def = print_config_def.get(opt_key); + if (def == nullptr || !def->default_value || def->default_value->is_nil()) + continue; // the option's default is nil too, so an absent key cannot come back as a value const auto* opt_vec = dynamic_cast(preset.config.option(opt_key)); if (opt_vec == nullptr) continue; diff --git a/src/libslic3r/PrintConfig.cpp b/src/libslic3r/PrintConfig.cpp index bab1d1b586..def3ff8049 100644 --- a/src/libslic3r/PrintConfig.cpp +++ b/src/libslic3r/PrintConfig.cpp @@ -118,13 +118,14 @@ const std::vector filament_extruder_override_keys = { "filament_retraction_distances_when_cut" }; -// Some filament override parameters are generated from filament_extruder_override_keys, -// while filament_retract_length_nc is defined separately. Keep the generator list -// unchanged and use this helper for behavior checks that need the full override set. +// A filament override is any option whose definition says its nil means "disabled" rather +// than "inherit": the keys generated from filament_extruder_override_keys, plus the ones +// defined separately (filament_retract_length_nc, the ironing overrides). Child presets +// store and load such a nil verbatim instead of folding it into the parent's value. bool is_filament_extruder_override_key(const std::string &opt_key) { - return std::find(filament_extruder_override_keys.begin(), filament_extruder_override_keys.end(), opt_key) != filament_extruder_override_keys.end() || - opt_key == "filament_retract_length_nc"; + const ConfigOptionDef *def = print_config_def.get(opt_key); + return def != nullptr && def->nil_means_disabled; } size_t get_extruder_index(const GCodeConfig& config, unsigned int filament_id) @@ -4133,6 +4134,7 @@ void PrintConfigDef::init_fff_params() def->max = 100; def->mode = comAdvanced; def->nullable = true; + def->nil_means_disabled = true; def->set_default_value(new ConfigOptionPercentsNullable{ ConfigOptionPercentsNullable::nil_value() }); def = this->add("filament_ironing_spacing", coFloats); @@ -4145,6 +4147,7 @@ void PrintConfigDef::init_fff_params() def->max = 1; def->mode = comAdvanced; def->nullable = true; + def->nil_means_disabled = true; def->set_default_value(new ConfigOptionFloatsNullable{ ConfigOptionFloatsNullable::nil_value() }); def = this->add("filament_ironing_inset", coFloats); @@ -4157,6 +4160,7 @@ void PrintConfigDef::init_fff_params() def->max = 100; def->mode = comAdvanced; def->nullable = true; + def->nil_means_disabled = true; def->set_default_value(new ConfigOptionFloatsNullable{ ConfigOptionFloatsNullable::nil_value() }); def = this->add("filament_ironing_speed", coFloats); @@ -4168,6 +4172,7 @@ void PrintConfigDef::init_fff_params() def->min = 1; def->mode = comAdvanced; def->nullable = true; + def->nil_means_disabled = true; def->set_default_value(new ConfigOptionFloatsNullable{ ConfigOptionFloatsNullable::nil_value() }); def = this->add("fuzzy_skin", coEnum); @@ -8678,6 +8683,7 @@ void PrintConfigDef::init_fff_params() auto it_opt = options.find(extruder_raw_key); assert(it_opt != options.end()); def = this->add_nullable(opt_key, it_opt->second.type); + def->nil_means_disabled = true; def->label = it_opt->second.label; def->full_label = it_opt->second.full_label; def->tooltip = it_opt->second.tooltip; @@ -8827,6 +8833,7 @@ void PrintConfigDef::init_fff_params() def->sidetext = L("mm"); def->mode = comDevelop; def->nullable = true; + def->nil_means_disabled = true; def->min = 0; def->max = 18; def->set_default_value(new ConfigOptionFloatsNullable { 10. }); @@ -12255,7 +12262,8 @@ void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig& opt_src->set(opt_target); } else - opt_vec_src->set_only_diff(opt_vec_dest, variant_index, stride); + // An override's nil means "disabled" and is the child's value; every other nil column inherits. + opt_vec_src->set_only_diff(opt_vec_dest, variant_index, stride, is_filament_extruder_override_key(opt)); } } } diff --git a/tests/libslic3r/test_config.cpp b/tests/libslic3r/test_config.cpp index 20177bcc07..5b25d1699f 100644 --- a/tests/libslic3r/test_config.cpp +++ b/tests/libslic3r/test_config.cpp @@ -1665,3 +1665,68 @@ TEST_CASE("Default options of enum lists get their definition's keys map", "[Con } CHECK(enum_lists > 0); } + +SCENARIO("update_diff_values_to_child_config keeps a child's disabled filament override", "[Config][Variant]") +{ + std::set no_keys; + GIVEN("A filament parent that overrides retract before wipe and ironing flow") { + Slic3r::DynamicPrintConfig parent; + parent.set_deserialize_strict("filament_retract_before_wipe", "100%"); + parent.set_deserialize_strict("filament_ironing_flow", "15%"); + WHEN("the child disables both overrides") { + Slic3r::DynamicPrintConfig child; + child.set_deserialize_strict("filament_retract_before_wipe", "nil"); + child.set_deserialize_strict("filament_ironing_flow", "nil"); + parent.update_diff_values_to_child_config(child, "", "filament_extruder_variant", + Slic3r::filament_options_with_variant, no_keys); + THEN("both overrides stay disabled") { + CHECK(parent.opt_serialize("filament_retract_before_wipe") == "nil"); + CHECK(parent.opt_serialize("filament_ironing_flow") == "nil"); + } + } + } + GIVEN("A two-variant filament parent") { + Slic3r::DynamicPrintConfig parent; + parent.set_deserialize_strict("filament_extruder_variant", "Direct Drive Standard;Direct Drive High Flow"); + parent.set_deserialize_strict("filament_retract_before_wipe", "100%,100%"); + parent.set_deserialize_strict("filament_flow_ratio", "1,1"); + WHEN("the child disables the override on one variant and leaves a plain option nil on it") { + Slic3r::DynamicPrintConfig child; + child.set_deserialize_strict("filament_extruder_variant", "Direct Drive Standard;Direct Drive High Flow"); + child.set_deserialize_strict("filament_retract_before_wipe", "nil,100%"); + child.set_deserialize_strict("filament_flow_ratio", "nil,1.1"); + parent.update_diff_values_to_child_config(child, "", "filament_extruder_variant", + Slic3r::filament_options_with_variant, no_keys); + THEN("the override column is disabled while the plain option's nil column inherits") { + CHECK(parent.opt_serialize("filament_retract_before_wipe") == "nil,100%"); + CHECK(parent.opt_serialize("filament_flow_ratio") == "1,1.1"); + } + } + WHEN("the child lists the same variants in the other order") { + Slic3r::DynamicPrintConfig child; + child.set_deserialize_strict("filament_extruder_variant", "Direct Drive High Flow;Direct Drive Standard"); + child.set_deserialize_strict("filament_retract_before_wipe", "70%,nil"); + child.set_deserialize_strict("filament_flow_ratio", "1.1,nil"); + parent.update_diff_values_to_child_config(child, "", "filament_extruder_variant", + Slic3r::filament_options_with_variant, no_keys); + THEN("columns are matched by variant name, the disabled one included") { + CHECK(parent.opt_serialize("filament_retract_before_wipe") == "nil,70%"); + CHECK(parent.opt_serialize("filament_flow_ratio") == "1,1.1"); + } + } + } +} + +TEST_CASE("Every filament override is the only kind of option whose nil means disabled", "[Config]") +{ + // Listed by hand on purpose: the helper answers from the flag, so it cannot be the oracle here. + std::set overrides(Slic3r::filament_extruder_override_keys.begin(), Slic3r::filament_extruder_override_keys.end()); + overrides.insert({ "filament_retract_length_nc", "filament_ironing_flow", "filament_ironing_spacing", "filament_ironing_inset", "filament_ironing_speed" }); + for (const auto &[key, def] : Slic3r::print_config_def.options) { + INFO(key); + CHECK(def.nil_means_disabled == (overrides.count(key) == 1)); + CHECK(Slic3r::is_filament_extruder_override_key(key) == def.nil_means_disabled); + if (def.nil_means_disabled) + CHECK(def.nullable); + } +} diff --git a/tests/libslic3r/test_preset_bundle_loading.cpp b/tests/libslic3r/test_preset_bundle_loading.cpp index 1b4927f08c..9053fc5496 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -8417,3 +8417,58 @@ TEST_CASE("An empty choice list in a G-code's settings keeps the value it had be CHECK(config.opt_serialize("nozzle_volume_type") == "High Flow"); CHECK(config.option("filament_multi_colour")->values.empty()); } + +TEST_CASE("A disabled filament override is saved and reloaded column by column", "[Preset][Bundle]") +{ + ScopedPresetDataDir data_dir; + PresetBundle bundle; + + Preset &parent = add_inmemory_preset(bundle.filaments, "Parent PLA @System"); + parent.config.option("filament_extruder_variant", true)->values = { "Direct Drive Standard", "Direct Drive High Flow" }; + parent.config.option("filament_retract_before_wipe", true)->values = { 100., 100. }; + parent.config.option("filament_flow_ratio", true)->values = { 1., 1. }; + + // The child turns the override off on the first variant and changes a plain option on the second. + const fs::path file = data_dir.tmp.path() / PRESET_FILAMENT_NAME / "Child PLA.json"; + Preset child(Preset::TYPE_FILAMENT, "Child PLA"); + child.file = file.string(); + child.version = *Semver::parse("1.0.0"); + child.config = parent.config; + child.config.option(BBL_JSON_KEY_INHERITS, true)->value = parent.name; + child.config.option("filament_retract_before_wipe")->set_at_to_nil(0); + child.config.option("filament_flow_ratio")->values = { 1., 1.1 }; + REQUIRE(child.save(&parent.config)); + + // On disk the override keeps every column, so its nil can only mean "off"; a plain + // option still stores nil for the columns that match the parent. + DynamicPrintConfig saved; + std::map key_values; + std::string reason; + saved.load_from_json(file.string(), ForwardCompatibilitySubstitutionRule::EnableSilent, key_values, reason); + REQUIRE(reason.empty()); + CHECK(saved.opt_serialize("filament_retract_before_wipe") == "nil,100%"); + CHECK(saved.opt_serialize("filament_flow_ratio") == "nil,1.1"); + + PresetsConfigSubstitutions substitutions; + bundle.filaments.load_presets(data_dir.tmp.path().string(), PRESET_FILAMENT_NAME, substitutions, + ForwardCompatibilitySubstitutionRule::EnableSilent); + const Preset *loaded = bundle.filaments.find_preset("Child PLA"); + REQUIRE(loaded != nullptr); + CHECK(loaded->config.opt_serialize("filament_retract_before_wipe") == "nil,100%"); + CHECK(loaded->config.opt_serialize("filament_flow_ratio") == "1,1.1"); +} + +TEST_CASE("The cloud diff force-emits a disabled override only where the cloud would fill a value in", "[Preset][Bundle]") +{ + PresetBundle bundle; + add_inmemory_preset(bundle.filaments, "Parent PLA @System"); + Preset &child = add_inmemory_preset(bundle.filaments, "Child PLA", "Parent PLA @System"); + // Every override is nil in both parent and child, so none is in the plain diff. + std::map key_values; + REQUIRE(bundle.filaments.get_differed_values_to_update(child, key_values) >= 0); + // Retract Before Wipe defaults to 100%: an absent key would come back as 100%, so nil is sent. + CHECK(key_values.count("filament_retract_before_wipe") == 1); + CHECK(key_values["filament_retract_before_wipe"] == "nil"); + // An ironing override defaults to nil: an absent key stays nil, so it is not sent. + CHECK(key_values.count("filament_ironing_flow") == 0); +}