diff --git a/src/libslic3r/PrintConfig.cpp b/src/libslic3r/PrintConfig.cpp index 76073b4981..f1390f5fcd 100644 --- a/src/libslic3r/PrintConfig.cpp +++ b/src/libslic3r/PrintConfig.cpp @@ -11513,12 +11513,18 @@ void DynamicPrintConfig::update_non_diff_values_to_base_config(DynamicPrintConfi int cur_variant_count = cur_extruder_variants.size(); int target_variant_count = target_extruder_variants.size(); + // A base variant this config does not list (the base gained it after the config was saved, or the + // config lists none) takes this config's first variant of the same extruder, as a user preset's + // values do in update_diff_values_to_child_config. Left unmatched, the base's value would silently + // replace the user's. variant_index.resize(target_variant_count, -1); if (cur_variant_count == 0) { // Defensive: target_variant_count may be 0 if the preset doesn't carry extruder_variant_name. // In that case keep variant_index empty and let the downstream size checks produce a useful error. if (!variant_index.empty()) - variant_index[0] = 0; + // This config's one value belongs to the extruder of the base's first variant. + variant_index = map_variant_indices(target_extruder_variants, target_extruder_ids, {}, + target_extruder_ids.empty() ? std::vector() : std::vector{target_extruder_ids[0]}); } else if ((cur_extruder_ids.size() > 0) && cur_variant_count != cur_extruder_ids.size()){ //should not happen @@ -11531,18 +11537,7 @@ void DynamicPrintConfig::update_non_diff_values_to_base_config(DynamicPrintConfi %extruder_variant_name %target_variant_count %extruder_id_name %target_extruder_ids.size(); } else { - for (int i = 0; i < target_variant_count; i++) - { - for (int j = 0; j < cur_variant_count; j++) - { - if ((target_extruder_variants[i] == cur_extruder_variants[j]) - &&(target_extruder_ids.empty() || (target_extruder_ids[i] == cur_extruder_ids[j]))) - { - variant_index[i] = j; - break; - } - } - } + variant_index = map_variant_indices(target_extruder_variants, target_extruder_ids, cur_extruder_variants, cur_extruder_ids); } for (auto& opt : keys) { @@ -11567,6 +11562,13 @@ void DynamicPrintConfig::update_non_diff_values_to_base_config(DynamicPrintConfi if (cur_variant_count > target_variant_count) continue; + // The variant lists are the base's layout itself, which every other value is + // carried onto: a variant this config lacks keeps its own name and id. + if (opt == extruder_id_name || opt == extruder_variant_name) { + opt_src->set(opt_target); + continue; + } + int stride = 1; if (key_set2.find(opt) != key_set2.end()) stride = 2; diff --git a/tests/libslic3r/test_config.cpp b/tests/libslic3r/test_config.cpp index bb51735bfe..b0e296d2d4 100644 --- a/tests/libslic3r/test_config.cpp +++ b/tests/libslic3r/test_config.cpp @@ -536,6 +536,110 @@ SCENARIO("update_diff_values_to_child_config keeps a child's values on variants } } +SCENARIO("update_non_diff_values_to_base_config keeps a project's changed values on variants it does not list", + "[Config][Variant]") { + std::set no_keys; + auto variants = [](std::initializer_list names) { return new Slic3r::ConfigOptionStrings(names); }; + + GIVEN("A filament base with three variants") { + Slic3r::DynamicPrintConfig base; + base.set_key_value("filament_extruder_variant", + variants({"Direct Drive Standard", "Bowden Standard", "Direct Drive High Flow"})); + base.set_deserialize_strict("nozzle_temperature", "220,220,220"); + + WHEN("the project was saved when the base had only its first variant") { + Slic3r::DynamicPrintConfig project; + project.set_key_value("filament_extruder_variant", variants({"Direct Drive Standard"})); + project.set_deserialize_strict("nozzle_temperature", "199"); + + AND_WHEN("the project lists the value as changed") { + project.update_non_diff_values_to_base_config(base, project.keys(), {"nozzle_temperature"}, "", "filament_extruder_variant", + Slic3r::filament_options_with_variant, no_keys); + THEN("the project's value applies to every variant") { + REQUIRE(project.opt_serialize("nozzle_temperature") == "199,199,199"); + } + } + AND_WHEN("the project does not list the value as changed") { + project.update_non_diff_values_to_base_config(base, project.keys(), {}, "", "filament_extruder_variant", + Slic3r::filament_options_with_variant, no_keys); + THEN("the base's values replace it") { + REQUIRE(project.opt_serialize("nozzle_temperature") == "220,220,220"); + } + } + } + WHEN("the project lists every variant, in another order") { + Slic3r::DynamicPrintConfig project; + project.set_key_value("filament_extruder_variant", + variants({"Bowden Standard", "Direct Drive High Flow", "Direct Drive Standard"})); + project.set_deserialize_strict("nozzle_temperature", "190,205,199"); + project.update_non_diff_values_to_base_config(base, project.keys(), {"nozzle_temperature"}, "", "filament_extruder_variant", + Slic3r::filament_options_with_variant, no_keys); + THEN("each variant keeps its own value") { + REQUIRE(project.opt_serialize("nozzle_temperature") == "199,190,205"); + } + } + WHEN("the project lists no variants") { + Slic3r::DynamicPrintConfig project; + project.set_deserialize_strict("nozzle_temperature", "199"); + project.update_non_diff_values_to_base_config(base, project.keys(), {"nozzle_temperature"}, "", "filament_extruder_variant", + Slic3r::filament_options_with_variant, no_keys); + THEN("the project's value applies to every variant") { + REQUIRE(project.opt_serialize("nozzle_temperature") == "199,199,199"); + } + } + } + + GIVEN("A two-extruder printer base with two variants per extruder") { + Slic3r::DynamicPrintConfig base; + base.set_key_value("printer_extruder_variant", + variants({"Direct Drive Standard", "Direct Drive High Flow", "Direct Drive Standard", "Direct Drive High Flow"})); + base.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1, 1, 2, 2})); + base.set_deserialize_strict("retraction_length", "0.8,0.8,0.8,0.8"); + + WHEN("the project lists only the Standard variant of each extruder") { + Slic3r::DynamicPrintConfig project; + project.set_key_value("printer_extruder_variant", variants({"Direct Drive Standard", "Direct Drive Standard"})); + project.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1, 2})); + project.set_deserialize_strict("retraction_length", "1.1,2.2"); + project.update_non_diff_values_to_base_config(base, project.keys(), {"retraction_length"}, "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(project.opt_serialize("retraction_length") == "1.1,1.1,2.2,2.2"); + } + } + WHEN("the project lists only the Standard variant of each extruder, and the variant lists as changed") { + Slic3r::DynamicPrintConfig project; + project.set_key_value("printer_extruder_variant", variants({"Direct Drive Standard", "Direct Drive Standard"})); + project.set_key_value("printer_extruder_id", new Slic3r::ConfigOptionInts({1, 2})); + project.set_deserialize_strict("machine_max_speed_x", "300,100,400,150"); + base.set_deserialize_strict("machine_max_speed_x", "500,200,500,200,500,200,500,200"); + project.update_non_diff_values_to_base_config(base, project.keys(), + {"machine_max_speed_x", "printer_extruder_id", "printer_extruder_variant"}, + "printer_extruder_id", "printer_extruder_variant", + Slic3r::printer_options_with_variant_1, + Slic3r::printer_options_with_variant_2); + THEN("the variant lists are the base's") { + REQUIRE(project.opt_serialize("printer_extruder_variant") == base.opt_serialize("printer_extruder_variant")); + REQUIRE(project.opt_serialize("printer_extruder_id") == "1,1,2,2"); + } + THEN("each extruder's High Flow variant takes that extruder's pair of limits") { + REQUIRE(project.opt_serialize("machine_max_speed_x") == "300,100,300,100,400,150,400,150"); + } + } + WHEN("the project lists no variants") { + Slic3r::DynamicPrintConfig project; + project.set_deserialize_strict("retraction_length", "1.1"); + project.update_non_diff_values_to_base_config(base, project.keys(), {"retraction_length"}, "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 project's value") { + REQUIRE(project.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(); diff --git a/tests/libslic3r/test_preset_bundle_loading.cpp b/tests/libslic3r/test_preset_bundle_loading.cpp index 11e66a90b1..02761ea5c6 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -6081,8 +6081,9 @@ TEST_CASE("A per-variant project value maps onto its base preset's variant layou base_finder(&base, calls)); CHECK(config.option("print_extruder_variant")->values == std::vector{"Direct Drive Standard", "Direct Drive High Flow"}); - // The listed key keeps the project's Standard value and takes High Flow from the base. - check_double_vector(config.option("outer_wall_speed")->values, {100., 300.}); + // The listed key keeps the project's Standard value, and High Flow, which the project does not + // list, takes it too, as a user preset's value does. + check_double_vector(config.option("outer_wall_speed")->values, {100., 100.}); check_double_vector(config.option("inner_wall_speed")->values, {250., 350.}); }