From 5ecb596b1c8ce4f05665b81ceff7914e629fc2c4 Mon Sep 17 00:00:00 2001 From: Hanif Koh Date: Fri, 2 Oct 2026 18:00:54 +0800 Subject: [PATCH] Read BambuStudio Nil Slots as Not Set Instead of Aborting the Load BambuStudio and its forks save a nozzle variant that matches the parent preset as "nil", including in keys OrcaSlicer can't leave empty, such as retraction, z-hop and nozzle temperature. Reading one threw, which ended the rest of the settings file: a project kept only the keys before it alphabetically, and an embedded or user preset was dropped. load_from_json now reads such a slot as not set when the caller opts in (project settings, embedded presets, user presets). If every slot is nil, the key is left out. Otherwise the slot holds the option default and is recorded, and update_diff_values_to_child_config gives it the parent preset's value. Other loaders still reject nil. --- src/libslic3r/Config.cpp | 41 ++++++++++++++--- src/libslic3r/Config.hpp | 5 +++ src/libslic3r/Format/bbs_3mf.cpp | 11 ++++- src/libslic3r/Preset.cpp | 13 +++--- src/libslic3r/Preset.hpp | 2 + src/libslic3r/PrintConfig.cpp | 31 +++++++++++-- src/libslic3r/PrintConfig.hpp | 5 ++- tests/libslic3r/test_config.cpp | 75 ++++++++++++++++++++++++++++++++ 8 files changed, 168 insertions(+), 15 deletions(-) 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));