Compare commits

...
Author SHA1 Message Date
Hanif Koh 7de85397a6 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.
2026-10-19 18:49:44 +08:00
5 changed files with 150 additions and 14 deletions

No files matched your search

+8 -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];
}
}
@@ -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<const ConfigOption> default_value;
void set_default_value(const ConfigOption* ptr) { this->default_value = Slic3r::clonable_ptr<const ConfigOption>(ptr); }
+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;
+14 -6
View File
@@ -118,13 +118,14 @@ const std::vector<std::string> 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));
}
}
}
+65
View File
@@ -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<std::string> 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<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(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);
}
}
@@ -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);
}