Keep User Preset Values on Extruder Variants They Don't List (#16046)

* Keep User Preset Values on Extruder Variants They Don't List

A user preset stores the variant list its parent had when it was saved.
When the parent later gains variants, update_diff_values_to_child_config
matched variants by name only and left the new ones at the parent's
value, so the user's settings were silently replaced there, and a
re-save wrote the system values into the user's file.

An unmatched parent variant now takes the child's first variant of the
same extruder, the rule slicing already uses in get_config_index_base.
A child without a variant list covers the parent's first extruder. The
name match also no longer indexes the child's extruder ids when it has
none.

* Share One Variant Column Rule Between Slicing, User Presets and Projects

Three places chose which variant column a value comes from, each with
its own copy of "the same variant and owner, else the owner's first
column": get_config_index_base when slicing, the user preset merge in
update_diff_values_to_child_config, and normalize_filament_values_to_variants
for projects and the CLI.

find_variant_column now holds that rule and map_variant_columns applies
it to a variant list, so a change to how missing variants are filled
reaches all three. Each caller keeps its own copy step. There is no
behaviour change: G-code is identical before and after. The one
relaxation is that get_config_index_base no longer reads past a short
id list when its two lists differ in length, which its assert already
rules out.

* Rename variant column helpers to variant index

---------

Co-authored-by: SoftFever <softfeverever@gmail.com>
This commit is contained in:
HanifKoh
2026-10-02 20:13:06 +08:00
committed by GitHub
co-authored by SoftFever
parent 1255af1e9c
commit 205de9ce63
3 changed files with 175 additions and 29 deletions
+59 -27
View File
@@ -8,6 +8,7 @@
#include "format.hpp"
#include "GCode/Thumbnails.hpp"
#include <numeric>
#include <set>
#include <boost/algorithm/string/case_conv.hpp>
#include <boost/algorithm/string/replace.hpp>
@@ -671,19 +672,46 @@ std::string get_extruder_variant_string(ExtruderType extruder_type, NozzleVolume
return variant_string;
}
int find_variant_index(const std::string& variant, int variant_id_1based, const std::vector<std::string>& variant_list, const std::vector<int>& variant_ids_1based)
{
const int count = int(variant_list.empty() ? variant_ids_1based.size() : variant_list.size());
if (count == 0)
return 0;
auto same_id = [&](int index) {
return variant_id_1based < 0 || variant_ids_1based.empty() || (index < int(variant_ids_1based.size()) && variant_ids_1based[index] == variant_id_1based);
};
for (int index = 0; index < int(variant_list.size()); ++index)
if (variant_list[index] == variant && same_id(index))
return index;
// Without this variant, use the id's own first variant (usually Standard), not variant index 0,
// which belongs to the first filament or extruder.
for (int index = 0; index < count; ++index)
if (same_id(index))
return index;
return -1;
}
std::vector<int> map_variant_indices(const std::vector<std::string>& variants, const std::vector<int>& ids,
const std::vector<std::string>& from_variants, const std::vector<int>& from_ids)
{
const size_t count = variants.empty() ? ids.size() : variants.size();
std::vector<int> variant_index(count);
for (size_t index = 0; index < count; ++index) {
if (!ids.empty() && index >= ids.size()) {
variant_index[index] = -1;
continue;
}
variant_index[index] = find_variant_index(index < variants.size() ? variants[index] : std::string(),
ids.empty() ? -1 : ids[index], from_variants, from_ids);
}
return variant_index;
}
int get_config_index_base(NozzleVolumeType volume_type, ExtruderType extruder_type, int variant_id_1based, const std::vector<std::string>& variant_list, const std::vector<int>& variant_ids_1based)
{
assert(variant_list.size() == variant_ids_1based.size());
std::string extruder_variant = get_extruder_variant_string(extruder_type, volume_type);
for (int index = 0; index < int(variant_list.size()); ++index) {
if (extruder_variant == variant_list[index] && variant_ids_1based[index] == variant_id_1based) { return index; }
}
// Without this variant, use the id's own first variant (usually Standard), not variant index 0,
// which belongs to the first filament or extruder.
for (int index = 0; index < int(variant_list.size()); ++index) {
if (variant_ids_1based[index] == variant_id_1based) { return index; }
}
return 0;
const int index = find_variant_index(get_extruder_variant_string(extruder_type, volume_type), variant_id_1based, variant_list, variant_ids_1based);
return std::max(index, 0);
}
std::set<NozzleVolumeType> get_extruder_supported_nozzle_volume_types(const DynamicPrintConfig &printer_config, int extruder_id)
@@ -10741,14 +10769,23 @@ void normalize_filament_values_to_variants(DynamicPrintConfig &config)
const int filament_count = *std::max_element(self_index->values.begin(), self_index->values.end());
if (filament_count <= 0 || size_t(filament_count) >= self_index->size())
return;
// The values are one per filament, without variant strings, or a single value for all of them. The
// variant strings do not change today's mapping; they are passed so a rule that reads them applies here too.
const auto *variants = config.option<ConfigOptionStrings>("filament_extruder_variant");
const std::vector<std::string> variant_list = variants && variants->size() == self_index->size() ? variants->values : std::vector<std::string>();
std::vector<int> filament_ids(filament_count);
std::iota(filament_ids.begin(), filament_ids.end(), 1);
const std::vector<int> from_filaments = map_variant_indices(variant_list, self_index->values, {}, filament_ids);
const std::vector<int> from_single = map_variant_indices(variant_list, self_index->values, {}, {});
for (const std::string &key : filament_options_with_variant) {
auto *opt = dynamic_cast<ConfigOptionVectorBase *>(config.option(key));
if (opt == nullptr || (opt->size() != size_t(filament_count) && opt->size() != 1))
continue;
std::unique_ptr<ConfigOption> per_filament(opt->clone());
// set_at() takes the first value for a filament past the end of a single-value vector
const std::vector<int> &variant_index = opt->size() == size_t(filament_count) ? from_filaments : from_single;
std::unique_ptr<ConfigOption> source(opt->clone());
// -1 and a single-value source both resolve to the first value through get_at()
for (size_t variant = 0; variant < self_index->size(); ++variant)
opt->set_at(per_filament.get(), variant, self_index->values[variant] - 1);
opt->set_at(source.get(), variant, variant_index[variant]);
}
}
@@ -11582,8 +11619,14 @@ void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig&
else
variant_index.resize(1, 0);
// A parent variant the child does not list (the parent gained it after the child was saved, or the
// child lists none) takes the child's first variant of the same extruder, as slicing does.
// Left unmatched, the parent's value would silently replace the user's.
if (target_variant_count == 0) {
variant_index[0] = 0;
// The child's one value belongs to the extruder of the parent's first variant.
if (cur_variant_count > 0)
variant_index = map_variant_indices(cur_extruder_variants, cur_extruder_ids, {},
cur_extruder_ids.empty() ? std::vector<int>() : std::vector<int>{cur_extruder_ids[0]});
}
else if ((cur_extruder_ids.size() > 0) && cur_variant_count != cur_extruder_ids.size()){
//should not happen
@@ -11595,19 +11638,8 @@ void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig&
BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << boost::format(" size of %1% = %2%, not equal to size of %3% = %4%")
%extruder_variant_name %target_variant_count %extruder_id_name %target_extruder_ids.size();
}
else {
for (int i = 0; i < cur_variant_count; i++)
{
for (int j = 0; j < target_variant_count; j++)
{
if ((cur_extruder_variants[i] == target_extruder_variants[j])
&&(cur_extruder_ids.empty() || (cur_extruder_ids[i] == target_extruder_ids[j])))
{
variant_index[i] = j;
break;
}
}
}
else if (cur_variant_count > 0) {
variant_index = map_variant_indices(cur_extruder_variants, cur_extruder_ids, target_extruder_variants, target_extruder_ids);
}
const t_config_option_keys &keys = new_config.keys();
+13 -2
View File
@@ -556,8 +556,19 @@ enum PrimeVolumeMode {
extern std::string get_extruder_variant_string(ExtruderType extruder_type, NozzleVolumeType nozzle_volume_type);
// Base slot lookup: scans a variant list (paired with its 1-based extruder/filament ids) for the
// entry matching the given extruder/volume type and id. Returns 0 when no entry matches.
// The variant index a value is taken from: in a variant list paired with its 1-based extruder or
// filament ids, the variant with the same variant string and id, else that id's first variant, else -1.
// variant_id_1based < 0 or empty variant_ids_1based match any id. A list without variant strings has
// one variant per id, and one with neither variant strings nor ids has a single variant.
extern int find_variant_index(const std::string& variant, int variant_id_1based, const std::vector<std::string>& variant_list, const std::vector<int>& variant_ids_1based);
// find_variant_index for every variant of a list paired with its ids, into from_variants/from_ids.
// A variant past the end of a shorter id list has no id and gets -1.
extern std::vector<int> map_variant_indices(const std::vector<std::string>& variants, const std::vector<int>& ids,
const std::vector<std::string>& from_variants, const std::vector<int>& from_ids);
// Variant index lookup: scans a variant list (paired with its 1-based extruder/filament ids) for the
// entry matching the given extruder/volume type and id, as find_variant_index. Returns 0 when the id
// has no variant.
extern int get_config_index_base(NozzleVolumeType volume_type, ExtruderType extruder_type, int variant_id_1based, const std::vector<std::string>& variant_list, const std::vector<int>& variant_ids_1based);
static std::set<NozzleVolumeType> get_valid_nozzle_volume_type() {
+103
View File
@@ -421,6 +421,109 @@ SCENARIO("update_diff_values_to_child_config tolerates legacy machine-limit vect
}
}
TEST_CASE("A variant index comes from the same variant and id, else the id's first variant", "[Config][Variant]") {
const std::vector<std::string> variant_list{"Direct Drive Standard", "Direct Drive High Flow", "Direct Drive Standard"};
const std::vector<int> variant_ids{1, 1, 2};
SECTION("same variant and id") {
CHECK(Slic3r::find_variant_index("Direct Drive High Flow", 1, variant_list, variant_ids) == 1);
CHECK(Slic3r::find_variant_index("Direct Drive Standard", 2, variant_list, variant_ids) == 2);
}
SECTION("a variant the id lacks falls back to the id's first variant") {
CHECK(Slic3r::find_variant_index("Bowden Standard", 1, variant_list, variant_ids) == 0);
CHECK(Slic3r::find_variant_index("Direct Drive High Flow", 2, variant_list, variant_ids) == 2);
}
SECTION("an id with no variants matches none") {
CHECK(Slic3r::find_variant_index("Direct Drive Standard", 3, variant_list, variant_ids) == -1);
}
SECTION("a negative id or a list without ids matches any id") {
CHECK(Slic3r::find_variant_index("Direct Drive High Flow", -1, variant_list, variant_ids) == 1);
CHECK(Slic3r::find_variant_index("Direct Drive High Flow", 2, variant_list, {}) == 1);
}
SECTION("a variant past a shorter id list gets no variant index") {
CHECK(Slic3r::map_variant_indices(variant_list, {1}, variant_list, variant_ids) == std::vector<int>{0, -1, -1});
}
SECTION("a list without variant strings has one variant per id, and an empty one a single variant") {
CHECK(Slic3r::map_variant_indices(variant_list, variant_ids, {}, {1, 2}) == std::vector<int>{0, 0, 1});
CHECK(Slic3r::map_variant_indices(variant_list, variant_ids, {}, {}) == std::vector<int>{0, 0, 0});
}
}
SCENARIO("update_diff_values_to_child_config keeps a child's values on variants it does not list",
"[Config][Variant]") {
std::set<std::string> no_keys;
auto variants = [](std::initializer_list<std::string> names) { return new Slic3r::ConfigOptionStrings(names); };
GIVEN("A filament parent with three variants") {
Slic3r::DynamicPrintConfig parent;
parent.set_key_value("filament_extruder_variant",
variants({"Direct Drive Standard", "Bowden Standard", "Direct Drive High Flow"}));
parent.set_deserialize_strict("nozzle_temperature", "220,220,220");
WHEN("the child was saved when the parent had only its first variant") {
Slic3r::DynamicPrintConfig child;
child.set_key_value("filament_extruder_variant", variants({"Direct Drive Standard"}));
child.set_deserialize_strict("nozzle_temperature", "199");
parent.update_diff_values_to_child_config(child, "", "filament_extruder_variant",
Slic3r::filament_options_with_variant, no_keys);
THEN("the child's value applies to every variant") {
REQUIRE(parent.opt_serialize("nozzle_temperature") == "199,199,199");
}
}
WHEN("the child lists every variant, in another order") {
Slic3r::DynamicPrintConfig child;
child.set_key_value("filament_extruder_variant",
variants({"Bowden Standard", "Direct Drive High Flow", "Direct Drive Standard"}));
child.set_deserialize_strict("nozzle_temperature", "190,205,199");
parent.update_diff_values_to_child_config(child, "", "filament_extruder_variant",
Slic3r::filament_options_with_variant, no_keys);
THEN("each variant keeps its own value") {
REQUIRE(parent.opt_serialize("nozzle_temperature") == "199,190,205");
}
}
WHEN("the child lists no variants") {
Slic3r::DynamicPrintConfig child;
child.set_deserialize_strict("nozzle_temperature", "199");
parent.update_diff_values_to_child_config(child, "", "filament_extruder_variant",
Slic3r::filament_options_with_variant, no_keys);
THEN("the child's value applies to every variant") {
REQUIRE(parent.opt_serialize("nozzle_temperature") == "199,199,199");
}
}
}
GIVEN("A two-extruder printer parent with two variants per extruder") {
Slic3r::DynamicPrintConfig parent;
parent.set_key_value("printer_extruder_variant",
variants({"Direct Drive Standard", "Direct Drive High Flow", "Direct Drive Standard", "Direct Drive High Flow"}));
parent.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1, 1, 2, 2}));
parent.set_deserialize_strict("retraction_length", "0.8,0.8,0.8,0.8");
WHEN("the child lists only the Standard variant of each extruder") {
Slic3r::DynamicPrintConfig child;
child.set_key_value("printer_extruder_variant", variants({"Direct Drive Standard", "Direct Drive Standard"}));
child.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1, 2}));
child.set_deserialize_strict("retraction_length", "1.1,2.2");
parent.update_diff_values_to_child_config(child, "printer_extruder_id", "printer_extruder_variant",
Slic3r::printer_options_with_variant_1,
Slic3r::printer_options_with_variant_2);
THEN("each extruder's High Flow variant takes that extruder's value") {
REQUIRE(parent.opt_serialize("retraction_length") == "1.1,1.1,2.2,2.2");
}
}
WHEN("the child lists no variants") {
Slic3r::DynamicPrintConfig child;
child.set_deserialize_strict("retraction_length", "1.1");
parent.update_diff_values_to_child_config(child, "printer_extruder_id", "printer_extruder_variant",
Slic3r::printer_options_with_variant_1,
Slic3r::printer_options_with_variant_2);
THEN("only the first extruder's variants take the child's value") {
REQUIRE(parent.opt_serialize("retraction_length") == "1.1,1.1,0.8,0.8");
}
}
}
}
// SCENARIO("DynamicPrintConfig JSON serialization", "[Config]") {
// WHEN("DynamicPrintConfig is serialized and deserialized") {
// auto now = std::chrono::high_resolution_clock::now();