Compare commits

...
Author SHA1 Message Date
Hanif Koh 904b837799 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.

Treat the keys in is_filament_extruder_override_key, now including the
ironing overrides, as "nil means off": 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. 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.
2026-10-19 20:04:22 +08:00
5 changed files with 150 additions and 12 deletions

No files matched your search

+4 -3
View File
@@ -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<int>& restore_index, int stride) = 0;
virtual void set_with_restore_2(const ConfigOptionVectorBase* rhs, std::vector<int>& restore_index, int start, int len, bool skip_error = false) = 0;
virtual void set_only_diff(const ConfigOptionVectorBase* rhs, std::vector<int>& 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<int>& diff_index, int stride, bool keep_nil = false) = 0;
virtual void set_to_index(const ConfigOptionVectorBase* rhs, std::vector<int>& 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<int>& diff_index, int stride) override
virtual void set_only_diff(const ConfigOptionVectorBase* rhs, std::vector<int>& 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];
}
}
+8 -5
View File
@@ -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<ConfigOptionVectorBase*>(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<ConfigOptionVectorBase*>(opt_src);
@@ -2198,9 +2198,12 @@ int PresetCollection::get_differed_values_to_update(Preset& preset, std::map<std
// option's non-nil default (e.g. filament_retract_before_wipe -> 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<const ConfigOptionVectorBase*>(preset.config.option(opt_key));
if (opt_vec == nullptr)
continue;
+11 -4
View File
@@ -119,12 +119,18 @@ const std::vector<std::string> 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));
}
}
}
+72
View File
@@ -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<std::string> 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<std::string> 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<std::string> 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);
}
}
@@ -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<ConfigOptionStrings>("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<ConfigOptionStrings>("filament_extruder_variant", true)->values = { "Direct Drive Standard", "Direct Drive High Flow" };
parent.config.option<ConfigOptionPercentsNullable>("filament_retract_before_wipe", true)->values = { 100., 100. };
parent.config.option<ConfigOptionFloatsNullable>("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<ConfigOptionString>(BBL_JSON_KEY_INHERITS, true)->value = parent.name;
child.config.option<ConfigOptionPercentsNullable>("filament_retract_before_wipe")->set_at_to_nil(0);
child.config.option<ConfigOptionFloatsNullable>("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<std::string, std::string> 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<std::string, std::string> 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);
}