Keep Per-Nozzle Values When a User Preset Syncs Through the Cloud

The cloud upload sent only the keys that differ from the parent, so a
variant layout equal to the parent's was left out. On download the
loader sized every per-variant value by the missing layout, cut it to
one value and kept the parent's values for the other nozzles and nozzle
volume types. On dual-nozzle printers the right nozzle's settings
silently reverted to the parent's after a sync.

Upload the variant layout always, as the local save already does, and
read copies stored without it against the parent's layout when they
carry one value per parent variant.

src/libslic3r/Preset.cpp

@@ -2164,6 +2164,16 @@ int PresetCollection::get_differed_values_to_update(Preset& preset, std::map<std
         DynamicPrintConfig temp_config;
         std::vector<std::string> dirty_options = preset.config.diff(parent_preset->config);
 
+        // Send the variant layout even when it matches the parent's, as the local save does: the loader
+        // sizes the per-variant values by it and would otherwise keep only the first one.
+        std::string extruder_id_name, extruder_variant_name;
+        std::set<std::string> *key_set1 = nullptr, *key_set2 = nullptr;
+        Preset::get_extruder_names_and_keysets(m_type, extruder_id_name, extruder_variant_name, &key_set1, &key_set2);
+        if (!extruder_id_name.empty())
+            dirty_options.emplace_back(extruder_id_name);
+        if (!extruder_variant_name.empty())
+            dirty_options.emplace_back(extruder_variant_name);
+
         for (auto option: dirty_options)
         {
             ConfigOption *opt_src = preset.config.option(option);
@@ -2628,15 +2638,34 @@ bool PresetCollection::load_user_preset(std::string name, std::map<std::string,
             new_config = default_preset.config;
         }
 
+        std::string extruder_id_name, extruder_variant_name;
+        std::set<std::string> *key_set1 = nullptr, *key_set2 = nullptr;
+        Preset::get_extruder_names_and_keysets(m_type, extruder_id_name, extruder_variant_name, &key_set1, &key_set2);
+
+        // Older uploads left out a variant layout equal to the parent's but still sent one value per
+        // parent variant. Read those values against the parent's layout instead of cutting them to one.
+        if (inherit_preset && !cloud_config.has(extruder_variant_name)) {
+            const auto  *parent_variants = inherit_preset->config.option<ConfigOptionStrings>(extruder_variant_name);
+            const size_t variant_count   = parent_variants ? parent_variants->size() : 0;
+            bool         values_per_variant = false;
+            for (const std::string &key : cloud_config.keys()) {
+                const size_t stride = key_set2->count(key) ? 2 : key_set1->count(key) ? 1 : 0;
+                const auto  *values = dynamic_cast<const ConfigOptionVectorBase *>(cloud_config.option(key));
+                if (stride > 0 && key != extruder_id_name && values && variant_count > 1 && values->size() == variant_count * stride) {
+                    values_per_variant = true;
+                    break;
+                }
+            }
+            if (values_per_variant)
+                for (const std::string &key : {extruder_id_name, extruder_variant_name})
+                    if (!key.empty() && !cloud_config.has(key) && inherit_preset->config.has(key))
+                        cloud_config.set_key_value(key, inherit_preset->config.option(key)->clone());
+        }
+
         extend_default_config_length(cloud_config, false, {});
 
-        if (inherit_preset) {
-            std::string extruder_id_name, extruder_variant_name;
-            std::set<std::string> *key_set1 = nullptr, *key_set2 = nullptr;
-            Preset::get_extruder_names_and_keysets(m_type, extruder_id_name, extruder_variant_name, &key_set1, &key_set2);
-
+        if (inherit_preset)
             new_config.update_diff_values_to_child_config(cloud_config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2);
-        }
         else{
             new_config.apply(std::move(cloud_config));
             extend_default_config_length(new_config, true, default_preset.config);

tests/libslic3r/test_preset_bundle_loading.cpp

@@ -34,6 +34,7 @@
 #include <utility>
 #include "libslic3r/Config.hpp"
 #include <map>
+#include <memory>
 #include "libslic3r/libslic3r.h"
 #include "libslic3r/PublishSettings.hpp"
 #include <set>
@@ -330,6 +331,105 @@ TEST_CASE("A directory of user presets loads with each one resolved against its
     }
 }
 
+TEST_CASE("A user preset synced to the cloud and back keeps its values on every nozzle variant", "[Preset][Bundle]")
+{
+    // Two extruders with a Standard and a High Flow variant each, as on a dual-nozzle printer.
+    auto add_parent = [](PresetCollection &coll) {
+        Preset &parent = add_inmemory_preset(coll, "Parent Process");
+        parent.config.set_key_value("print_extruder_variant",
+            new ConfigOptionStrings({"Direct Drive Standard", "Direct Drive High Flow", "Direct Drive Standard", "Direct Drive High Flow"}));
+        parent.config.set_key_value("print_extruder_id", new ConfigOptionInts({1, 1, 2, 2}));
+        parent.config.set_deserialize_strict("outer_wall_speed", "50,50,50,50");
+        parent.is_system  = true;
+        parent.setting_id = "parent-id";
+        return parent.config;
+    };
+
+    RenameTestCollection local;
+    const DynamicPrintConfig parent_config = add_parent(local);
+    Preset &child = add_inmemory_preset(local, "User Process", "Parent Process");
+    child.config.apply_only(parent_config, {"print_extruder_variant", "print_extruder_id"});
+    child.config.set_deserialize_strict("outer_wall_speed", "11,12,21,22");
+    child.version = Semver(1, 0, 0);
+
+    std::map<std::string, std::string> uploaded;
+    REQUIRE(local.get_differed_values_to_update(child, uploaded) == 0);
+    uploaded[BBL_JSON_KEY_SETTING_ID] = "child-id";
+    uploaded[BBL_JSON_KEY_USER_ID]    = "user";
+
+    // A cloud copy stored before uploads carried the variant layout holds the values alone.
+    std::map<std::string, std::string> stored_without_layout = uploaded;
+    stored_without_layout.erase("print_extruder_variant");
+    stored_without_layout.erase("print_extruder_id");
+    // One saved while the parent had a single variant covers the first extruder only.
+    std::map<std::string, std::string> stored_single_value = stored_without_layout;
+    stored_single_value["outer_wall_speed"] = "11";
+
+    struct Case { const char *label; std::map<std::string, std::string> values; const char *expected; };
+    for (const Case &c : {Case{"current upload", uploaded, "11,12,21,22"},
+                          Case{"stored without layout", stored_without_layout, "11,12,21,22"},
+                          Case{"stored with a single value", stored_single_value, "11,11,50,50"}}) {
+        DYNAMIC_SECTION(c.label) {
+            RenameTestCollection       downloaded;
+            PresetsConfigSubstitutions substitutions;
+            add_parent(downloaded);
+            REQUIRE(downloaded.load_user_preset("User Process", c.values, substitutions, ForwardCompatibilitySubstitutionRule::Disable));
+
+            const Preset *synced = downloaded.find_preset("User Process");
+            REQUIRE(synced != nullptr);
+            CHECK(synced->config.opt_serialize("outer_wall_speed") == c.expected);
+        }
+    }
+}
+
+TEST_CASE("A user filament synced to the cloud and back keeps its values and unset overrides on every variant", "[Preset][Bundle]")
+{
+    auto add_parent = [](PresetCollection &coll) {
+        Preset &parent = add_inmemory_preset(coll, "Parent Filament");
+        parent.config.set_key_value("filament_extruder_variant",
+            new ConfigOptionStrings({"Direct Drive Standard", "Direct Drive High Flow"}));
+        parent.config.set_deserialize_strict("nozzle_temperature", "220,220");
+        parent.config.set_deserialize_strict("filament_retraction_length", "nil,nil");
+        parent.is_system  = true;
+        parent.setting_id = "parent-id";
+        return parent.config;
+    };
+    auto make_collection = [] {
+        return std::make_unique<PresetCollection>(Preset::TYPE_FILAMENT, Preset::filament_options(),
+                                                  static_cast<const PrintRegionConfig &>(FullPrintConfig::defaults()));
+    };
+
+    auto local = make_collection();
+    add_parent(*local);
+    Preset &child = add_inmemory_preset(*local, "User Filament", "Parent Filament");
+    child.config.apply_only(add_parent(*make_collection()), {"filament_extruder_variant"});
+    child.config.set_deserialize_strict("nozzle_temperature", "200,210");
+    child.config.set_deserialize_strict("filament_retraction_length", "1.2,nil");
+    child.version = Semver(1, 0, 0);
+
+    std::map<std::string, std::string> uploaded;
+    REQUIRE(local->get_differed_values_to_update(child, uploaded) == 0);
+    uploaded[BBL_JSON_KEY_SETTING_ID] = "child-id";
+    uploaded[BBL_JSON_KEY_USER_ID]    = "user";
+    std::map<std::string, std::string> stored_without_layout = uploaded;
+    stored_without_layout.erase("filament_extruder_variant");
+
+    for (const auto &[label, values] : {std::make_pair("current upload", uploaded),
+                                        std::make_pair("stored without layout", stored_without_layout)}) {
+        DYNAMIC_SECTION(label) {
+            auto                       downloaded = make_collection();
+            PresetsConfigSubstitutions substitutions;
+            add_parent(*downloaded);
+            REQUIRE(downloaded->load_user_preset("User Filament", values, substitutions, ForwardCompatibilitySubstitutionRule::Disable));
+
+            const Preset *synced = downloaded->find_preset("User Filament");
+            REQUIRE(synced != nullptr);
+            CHECK(synced->config.opt_serialize("nozzle_temperature") == "200,210");
+            CHECK(synced->config.opt_serialize("filament_retraction_length") == "1.2,nil");
+        }
+    }
+}
+
 TEST_CASE("Repeated loads of a user preset directory produce the same presets", "[Preset][Bundle]")
 {
     ScopedTemporaryDir temp_dir;