diff --git a/src/libslic3r/Config.hpp b/src/libslic3r/Config.hpp index 9d76262ec6..ae234f8b35 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]; } } 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..10eeadbaa2 100644 --- a/src/libslic3r/PrintConfig.cpp +++ b/src/libslic3r/PrintConfig.cpp @@ -119,12 +119,18 @@ const std::vector filament_extruder_override_keys = { }; // 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. +// while filament_retract_length_nc and the ironing overrides are defined separately. +// Keep the generator list unchanged and use this helper for behavior checks that need +// the full override set. A nil in one of these keys means the override is disabled, so +// child presets store and load it verbatim instead of treating nil as "inherit". 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"; + opt_key == "filament_retract_length_nc" || + opt_key == "filament_ironing_flow" || + opt_key == "filament_ironing_spacing" || + opt_key == "filament_ironing_inset" || + opt_key == "filament_ironing_speed"; } size_t get_extruder_index(const GCodeConfig& config, unsigned int filament_id) @@ -12255,7 +12261,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..2e346eb46a 100644 --- a/tests/libslic3r/test_config.cpp +++ b/tests/libslic3r/test_config.cpp @@ -1665,3 +1665,75 @@ TEST_CASE("Default options of enum lists get their definition's keys map", "[Con } CHECK(enum_lists > 0); } + +TEST_CASE("A disabled filament override survives the merge with a parent that sets it", "[Config][Variant]") +{ + std::set no_keys; + Slic3r::DynamicPrintConfig parent; + parent.set_deserialize_strict("filament_retract_before_wipe", "100%"); + parent.set_deserialize_strict("filament_ironing_flow", "15%"); + 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); + CHECK(parent.opt_serialize("filament_retract_before_wipe") == "nil"); + CHECK(parent.opt_serialize("filament_ironing_flow") == "nil"); +} + +SCENARIO("A two-variant child's disabled override column is matched to the parent's variant by name", "[Config][Variant]") +{ + std::set no_keys; + 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"); + } + } + WHEN("the child was saved with a single variant") { + Slic3r::DynamicPrintConfig child; + child.set_deserialize_strict("filament_extruder_variant", "Direct Drive Standard"); + child.set_deserialize_strict("filament_retract_before_wipe", "nil"); + parent.update_diff_values_to_child_config(child, "", "filament_extruder_variant", + Slic3r::filament_options_with_variant, no_keys); + THEN("its one value applies to every variant of that extruder, as slicing does") { + CHECK(parent.opt_serialize("filament_retract_before_wipe") == "nil,nil"); + } + } + } +} + +TEST_CASE("Every listed filament override is nullable", "[Config]") +{ + 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(Slic3r::is_filament_extruder_override_key(key) == (overrides.count(key) == 1)); + if (overrides.count(key) == 1) + 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); +}