diff --git a/src/libslic3r/Config.cpp b/src/libslic3r/Config.cpp index 427f6e66c9..baa2d81c1a 100644 --- a/src/libslic3r/Config.cpp +++ b/src/libslic3r/Config.cpp @@ -873,10 +873,10 @@ int ConfigBase::load_from_json(const std::string &file, ConfigSubstitutionContex CNumericLocalesSetter locales_setter; - std::function parse_str_arr = [&parse_str_arr](const json::const_iterator& it, const char single_sep,const char array_sep,const bool escape_string_style,std::string& value_str)->bool { + std::function parse_str_arr = [&parse_str_arr](const json& arr, const char single_sep,const char array_sep,const bool escape_string_style,std::string& value_str)->bool { // must have consistent type name std::string consistent_type; - for (auto iter = it.value().begin(); iter != it.value().end(); ++iter) { + for (auto iter = arr.begin(); iter != arr.end(); ++iter) { if (consistent_type.empty()) consistent_type = iter.value().type_name(); else { @@ -886,13 +886,13 @@ int ConfigBase::load_from_json(const std::string &file, ConfigSubstitutionContex } bool first = true; - for (auto iter = it.value().begin(); iter != it.value().end(); iter++) { + for (auto iter = arr.begin(); iter != arr.end(); iter++) { if (iter.value().is_array()) { if (!first) value_str += array_sep; else first = false; - bool success = parse_str_arr(iter, single_sep, array_sep,escape_string_style, value_str); + bool success = parse_str_arr(iter.value(), single_sep, array_sep,escape_string_style, value_str); if (!success) return false; } @@ -1038,8 +1038,39 @@ int ConfigBase::load_from_json(const std::string &file, ConfigSubstitutionContex } } + // BambuStudio and its forks save a nozzle variant that matches the parent preset as "nil". + // An option that can't hold nil gets its default in that slot, and the slot is reported so + // the merge onto the parent keeps the parent's value. All slots nil means the key is not set. + const json *values = &it.value(); + json values_with_defaults; + if (substitution_context.accept_nil && optdef && !optdef->nullable && optdef->default_value && + (optdef->type == coFloats || optdef->type == coPercents || optdef->type == coFloatsOrPercents || + optdef->type == coInts || optdef->type == coEnums || optdef->type == coBools)) { + auto is_nil = [](const json &v) { return v.is_string() && v.get() == "nil"; }; + std::vector nil_slots; + for (size_t i = 0; i < values->size(); ++i) + if (is_nil((*values)[i])) + nil_slots.push_back(i); + if (!nil_slots.empty()) { + BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << ": " << file << ": " << it.key() << " is nil in " + << nil_slots.size() << " of " << values->size() + << " slots, read as not set (the parent preset's value, or the default)"; + if (nil_slots.size() == values->size()) + continue; + // create_default_option() gives enums their names, which vserialize() needs. + std::unique_ptr default_option(optdef->create_default_option()); + const std::vector defaults = static_cast(default_option.get())->vserialize(); + values_with_defaults = *values; + const json first_value = *std::find_if_not(values->begin(), values->end(), is_nil); + for (size_t i : nil_slots) + values_with_defaults[i] = defaults.empty() ? first_value : json(defaults[i % defaults.size()]); + values = &values_with_defaults; + substitution_context.nil_slots[opt_key] = std::move(nil_slots); + } + } + // BBS: we only support 2 depth array - valid = parse_str_arr(it, single_sep, array_sep,escape_string_type, value_str); + valid = parse_str_arr(*values, single_sep, array_sep,escape_string_type, value_str); if (!valid) { BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": parse " << file << " error, invalid json array for " << it.key(); break; diff --git a/src/libslic3r/Config.hpp b/src/libslic3r/Config.hpp index 9818887c1d..4e8e505d16 100644 --- a/src/libslic3r/Config.hpp +++ b/src/libslic3r/Config.hpp @@ -267,6 +267,11 @@ struct ConfigSubstitutionContext ForwardCompatibilitySubstitutionRule rule; ConfigSubstitutions substitutions; std::vector unrecogized_keys; + // Read "nil" in an option that can't hold it as not set instead of failing. Set by callers that hand + // nil_slots to the merge onto the parent preset, or for which the option default is the right fallback. + bool accept_nil = false; + // Slots of options that can't hold nil but were "nil" in the file; they hold the option default. + std::map> nil_slots; }; // A generic value of a configuration option. diff --git a/src/libslic3r/Format/bbs_3mf.cpp b/src/libslic3r/Format/bbs_3mf.cpp index 160c169d69..3a9a1fe2f6 100644 --- a/src/libslic3r/Format/bbs_3mf.cpp +++ b/src/libslic3r/Format/bbs_3mf.cpp @@ -2696,6 +2696,8 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) } std::map key_values; std::string reason; + // No parent preset here: a nil slot keeps the option default. + config_substitutions.accept_nil = true; int ret = config.load_from_json(dest_file, config_substitutions, true, key_values, reason); if (ret) { add_error("Error load config from json:"+reason); @@ -2736,7 +2738,13 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) //ConfigSubstitutions config_substitutions = config.load_from_ini(dest_file, Enable); std::map key_values; std::string reason; - ConfigSubstitutions config_substitutions = use_json? config.load_from_json(dest_file, Enable, key_values, reason) : config.load_from_ini(dest_file, Enable); + ConfigSubstitutionContext load_context(Enable); + load_context.accept_nil = true; + if (use_json) + config.load_from_json(dest_file, load_context, true, key_values, reason); + else + load_context.substitutions = config.load_from_ini(dest_file, Enable); + ConfigSubstitutions config_substitutions = std::move(load_context.substitutions); if (!reason.empty()) { BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << boost::format(", load project embedded config from %1% failed\n") % dest_file; //skip this file @@ -2785,6 +2793,7 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) preset->is_project_embedded = true; preset->is_external = true; preset->is_dirty = false; + preset->nil_slots = std::move(load_context.nil_slots); std::string version_str = key_values[BBL_JSON_KEY_VERSION]; boost::optional version = Semver::parse(version_str); diff --git a/src/libslic3r/Preset.cpp b/src/libslic3r/Preset.cpp index cd02465fb0..f8ba891227 100644 --- a/src/libslic3r/Preset.cpp +++ b/src/libslic3r/Preset.cpp @@ -1726,9 +1726,11 @@ PresetCollection::UserPresetLoad PresetCollection::resolve_user_preset( //ConfigSubstitutions config_substitutions = config.load_from_ini(preset.file, substitution_rule); std::map key_values; std::string reason; - ConfigSubstitutions config_substitutions = config.load_from_json(preset.file, substitution_rule, key_values, reason); - if (! config_substitutions.empty()) - out.substitutions.push_back({ preset.name, m_type, PresetConfigSubstitutions::Source::UserFile, preset.file, std::move(config_substitutions) }); + ConfigSubstitutionContext load_context(substitution_rule); + load_context.accept_nil = true; + config.load_from_json(preset.file, load_context, true, key_values, reason); + if (! load_context.substitutions.empty()) + out.substitutions.push_back({ preset.name, m_type, PresetConfigSubstitutions::Source::UserFile, preset.file, std::move(load_context.substitutions) }); if (!reason.empty()) { out.discard_file = true; out.errors.push_back((boost::format("parse config %1% failed") % preset.file).str()); @@ -1764,7 +1766,7 @@ PresetCollection::UserPresetLoad PresetCollection::resolve_user_preset( preset.config = inherit_preset->config; preset.filament_id = inherit_preset->filament_id; extend_default_config_length(config, false, {}); - preset.config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2); + preset.config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2, &load_context.nil_slots); } else { auto inherits_config2 = dynamic_cast(inherits_config); @@ -2137,7 +2139,8 @@ void PresetCollection::load_project_embedded_presets(std::vector& proje BOOST_LOG_TRIVIAL(error) << boost::format("can not find parent for config %1%!")%preset->file; continue; } - preset->config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2); + preset->config.update_diff_values_to_child_config(config, extruder_id_name, extruder_variant_name, *key_set1, *key_set2, &preset->nil_slots); + preset->nil_slots.clear(); //preset->config.apply(std::move(config)); Preset::normalize(preset->config); // Report configuration fields, which are misplaced into a wrong group. diff --git a/src/libslic3r/Preset.hpp b/src/libslic3r/Preset.hpp index 418ba19fc6..9be3a8ab01 100644 --- a/src/libslic3r/Preset.hpp +++ b/src/libslic3r/Preset.hpp @@ -269,6 +269,8 @@ public: //BBS: add type for project-embedded bool is_project_embedded = false; ConfigSubstitutions *loading_substitutions{nullptr}; + // Slots that were nil in the embedded preset's file, see ConfigSubstitutionContext::nil_slots. + std::map> nil_slots; bool is_user() const { return ! this->is_default && ! this->is_system && ! this->is_project_embedded && ! this->is_from_bundle(); } bool can_overwrite() const { return ! this->is_default && ! this->is_system && ! this->is_from_bundle(); } //bool is_user() const { return ! this->is_default && ! this->is_system; } diff --git a/src/libslic3r/PrintConfig.cpp b/src/libslic3r/PrintConfig.cpp index 7d29da8da2..c30687b9b0 100644 --- a/src/libslic3r/PrintConfig.cpp +++ b/src/libslic3r/PrintConfig.cpp @@ -11596,7 +11596,8 @@ void DynamicPrintConfig::update_non_diff_values_to_base_config(DynamicPrintConfi return; } -void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig& new_config, std::string extruder_id_name, std::string extruder_variant_name, std::set& key_set1, std::set& key_set2) +void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig& new_config, std::string extruder_id_name, std::string extruder_variant_name, std::set& key_set1, std::set& key_set2, + const std::map>* nil_slots) { std::vector cur_extruder_ids, target_extruder_ids, variant_index; std::vector cur_extruder_variants, target_extruder_variants; @@ -11657,6 +11658,13 @@ void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig& if (opt_src && opt_target && (*opt_src != *opt_target)) { BOOST_LOG_TRIVIAL(debug) << __FUNCTION__ << boost::format(" change key %1% from base_value %2% to child's value %3%") %opt %(opt_src->serialize()) %(opt_target->serialize()); + const std::vector *unset_slots = nullptr; + if (nil_slots && opt_src->is_vector()) + if (auto it = nil_slots->find(opt); it != nil_slots->end()) + unset_slots = &it->second; + std::unique_ptr base_value(unset_slots ? opt_src->clone() : nullptr); + int stride = 1; + bool merged_by_variant = false; if (opt_target->is_scalar() || ((key_set1.find(opt) == key_set1.end()) && (key_set2.empty() || (key_set2.find(opt) == key_set2.end())))) { //nothing to do, keep the original one @@ -11665,7 +11673,6 @@ void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig& else { ConfigOptionVectorBase* opt_vec_src = static_cast(opt_src); const ConfigOptionVectorBase* opt_vec_dest = static_cast(opt_target); - int stride = 1; if (key_set2.find(opt) != key_set2.end()) stride = 2; // set_only_diff() requires the base vector length to equal variant_index.size()*stride, where @@ -11679,8 +11686,26 @@ void DynamicPrintConfig::update_diff_values_to_child_config(DynamicPrintConfig& if (opt_vec_src->size() != variant_index.size() * size_t(stride)) { opt_src->set(opt_target); } - else + else { opt_vec_src->set_only_diff(opt_vec_dest, variant_index, stride); + merged_by_variant = true; + } + } + // A slot that was nil in the child's file is not set there, so it keeps this config's value. + if (unset_slots) { + auto *merged = static_cast(opt_src); + const size_t base_size = static_cast(base_value.get())->size(); + for (size_t slot = 0; slot < merged->size() && slot < base_size; ++slot) { + size_t child_slot = slot; + if (merged_by_variant) { + const int child_variant = variant_index[slot / stride]; + if (child_variant == -1) + continue; + child_slot = size_t(child_variant) * stride + slot % stride; + } + if (std::find(unset_slots->begin(), unset_slots->end(), child_slot) != unset_slots->end()) + merged->set_at(base_value.get(), slot, slot); + } } } } diff --git a/src/libslic3r/PrintConfig.hpp b/src/libslic3r/PrintConfig.hpp index bbd92079c0..33d3709867 100644 --- a/src/libslic3r/PrintConfig.hpp +++ b/src/libslic3r/PrintConfig.hpp @@ -859,7 +859,10 @@ public: void update_non_diff_values_to_base_config(DynamicPrintConfig& new_config, const t_config_option_keys& keys, const std::set& different_keys, std::string extruder_id_name, std::string extruder_variant_name, std::set& key_set1, std::set& key_set2); - void update_diff_values_to_child_config(DynamicPrintConfig& new_config, std::string extruder_id_name, std::string extruder_variant_name, std::set& key_set1, std::set& key_set2); + // nil_slots: per option, the slots of new_config that were nil in its file (see ConfigSubstitutionContext::nil_slots); + // those variants keep this config's value. + void update_diff_values_to_child_config(DynamicPrintConfig& new_config, std::string extruder_id_name, std::string extruder_variant_name, std::set& key_set1, std::set& key_set2, + const std::map>* nil_slots = nullptr); int update_values_from_single_to_multi(DynamicPrintConfig& multi_config, std::set& key_set, std::string id_name, std::string variant_name); int update_values_from_multi_to_multi(DynamicPrintConfig& new_config, std::set& key_set, std::string id_name, std::string variant_name, std::vector& extruder_variants); diff --git a/tests/libslic3r/test_config.cpp b/tests/libslic3r/test_config.cpp index 10a852207b..9ea18f939e 100644 --- a/tests/libslic3r/test_config.cpp +++ b/tests/libslic3r/test_config.cpp @@ -585,6 +585,81 @@ TEST_CASE("A BambuStudio project config loads its renamed settings without subst CHECK(config.validate().count("raft_first_layer_expansion") == 0); } +TEST_CASE("load_from_json reads a BambuStudio nil slot as not set", "[Config]") { + ScopedTemporaryFile tmp(".json"); + { + boost::nowide::ofstream ofs(tmp.string()); + // Keys after retraction_length in file order must still load. + ofs << R"({"layer_height":"0.2","retraction_length":["0.8","nil"],"wall_loops":"3","z_hop":["nil","nil"],)" + R"("z_hop_types":["Spiral Lift","nil"]})"; + } + std::map key_values; + std::string reason; + + SECTION("a loader that doesn't merge onto a parent still rejects nil") { + DynamicPrintConfig config; + ConfigSubstitutionContext context(ForwardCompatibilitySubstitutionRule::Enable); + config.load_from_json(tmp.string(), context, true, key_values, reason); + CHECK_FALSE(reason.empty()); + } + + SECTION("a loader that opts in reads it as not set") { + DynamicPrintConfig config; + ConfigSubstitutionContext context(ForwardCompatibilitySubstitutionRule::Enable); + context.accept_nil = true; + REQUIRE(config.load_from_json(tmp.string(), context, true, key_values, reason) == 0); + CHECK(reason.empty()); + + const auto *default_length = static_cast(print_config_def.get("retraction_length")->default_value.get()); + const auto &retraction_length = config.option("retraction_length")->values; + REQUIRE(retraction_length.size() == 2); + CHECK_THAT(retraction_length[0], Catch::Matchers::WithinAbs(0.8, 1e-9)); + CHECK_THAT(retraction_length[1], Catch::Matchers::WithinAbs(default_length->get_at(1), 1e-9)); + + std::unique_ptr default_types(print_config_def.get("z_hop_types")->create_default_option()); + const std::vector type_defaults = static_cast(default_types.get())->vserialize(); + CHECK(static_cast(config.option("z_hop_types"))->vserialize() == + std::vector{"Spiral Lift", type_defaults[1 % type_defaults.size()]}); + + CHECK(context.nil_slots == std::map>{{"retraction_length", {1}}, {"z_hop_types", {1}}}); + CHECK_FALSE(config.has("z_hop")); + CHECK(config.opt_int("wall_loops") == 3); + } +} + +TEST_CASE("A nozzle variant that was nil keeps the parent preset's value", "[Config]") { + auto printer = [](std::vector nozzle_diameter, std::vector retraction_length, std::vector max_speed_x) { + DynamicPrintConfig config; + config.set_key_value("printer_extruder_variant", new ConfigOptionStrings({"Direct Drive Standard", "Direct Drive High Flow"})); + config.set_key_value("printer_extruder_id", new ConfigOptionInts({1, 1})); + // Not a per-variant key, so the merge copies it whole. + config.set_key_value("nozzle_diameter", new ConfigOptionFloats(nozzle_diameter)); + config.set_key_value("retraction_length", new ConfigOptionFloats(retraction_length)); + // Two values per variant: normal and silent mode. + config.set_key_value("machine_max_speed_x", new ConfigOptionFloats(max_speed_x)); + return config; + }; + auto check_values = [](const DynamicPrintConfig &config, const char *key, const std::vector &expected) { + const std::vector &values = config.option(key)->values; + INFO(key); + REQUIRE(values.size() == expected.size()); + for (size_t i = 0; i < expected.size(); ++i) + CHECK_THAT(values[i], Catch::Matchers::WithinAbs(expected[i], 1e-9)); + }; + DynamicPrintConfig parent = printer({0.4, 0.6}, {0.6, 0.5}, {500, 200, 400, 100}); + // The nil slots of the child hold the option default after loading. + DynamicPrintConfig child = printer({0.2, 0.4}, {0.8, 0.4}, {500, 200, 300, 90}); + const std::map> nil_slots{{"nozzle_diameter", {1}}, {"retraction_length", {1}}, {"machine_max_speed_x", {3}}}; + + parent.update_diff_values_to_child_config(child, "printer_extruder_id", "printer_extruder_variant", + printer_options_with_variant_1, printer_options_with_variant_2, &nil_slots); + + check_values(parent, "nozzle_diameter", {0.2, 0.6}); + check_values(parent, "retraction_length", {0.8, 0.5}); + // Only the silent-mode value of the second variant was nil. + check_values(parent, "machine_max_speed_x", {500, 200, 300, 100}); +} + TEST_CASE("save_to_json writes the same document to a stream as to a file", "[Config]") { DynamicPrintConfig config; config.set_key_value("layer_height", new ConfigOptionFloat(0.2));