From 536ef758947b45fb9eb96f0c3296cc8c4a94bed8 Mon Sep 17 00:00:00 2001 From: Hanif Koh Date: Mon, 28 Sep 2026 18:01:00 +0800 Subject: [PATCH] Fill Settings Missing From a CLI Project From Its System Presets A project saved before a printer or process option existed has no value for it. The GUI takes such keys from the project's system preset; the CLI left them at the option default, so e.g. extruder_clearance_dist_to_rod sliced as 40 instead of the P1S's 33. The CLI now resolves the project's system printer and process presets by name and copies the keys the project lacks, skipping preset bookkeeping, print-host keys, the extruder variant layout and keys the legacy handler drops. PresetBundle::resolve_system_preset finds the vendor through its manifest or preset cache, so it also works in release builds, which ship vendors as caches only. --- src/OrcaSlicer.cpp | 58 ++++++++-- src/libslic3r/PresetBundle.cpp | 51 +++++++-- src/libslic3r/PresetBundle.hpp | 9 +- tests/cli/CMakeLists.txt | 8 ++ tests/cli/test_cli_project_missing_keys.sh | 89 +++++++++++++++ .../libslic3r/test_preset_bundle_loading.cpp | 101 ++++++++++++++++++ 6 files changed, 302 insertions(+), 14 deletions(-) create mode 100644 tests/cli/test_cli_project_missing_keys.sh diff --git a/src/OrcaSlicer.cpp b/src/OrcaSlicer.cpp index e7facbdbd8..4ad8beac94 100644 --- a/src/OrcaSlicer.cpp +++ b/src/OrcaSlicer.cpp @@ -2116,9 +2116,14 @@ int CLI::run(int argc, char **argv) // One resolver for the whole run, so presets from the same vendor tree share its load. std::unique_ptr system_preset_resolver; - auto resolve_preset = [&ensure_cli_preset_bundle, &system_preset_resolver](const std::string &file, DynamicPrintConfig &config, - std::string &config_type, const std::string &config_from, - bool probe_type, std::string &error) { + auto ensure_system_preset_resolver = [&system_preset_resolver]() -> PresetBundle & { + if (!system_preset_resolver) + system_preset_resolver = std::make_unique(); + return *system_preset_resolver; + }; + auto resolve_preset = [&ensure_cli_preset_bundle, &ensure_system_preset_resolver](const std::string &file, DynamicPrintConfig &config, + std::string &config_type, const std::string &config_from, + bool probe_type, std::string &error) { const auto *inherits = config.option(BBL_JSON_KEY_INHERITS); if (!probe_type && (inherits == nullptr || inherits->value.empty())) return true; @@ -2126,9 +2131,7 @@ int CLI::run(int argc, char **argv) PresetBundle *bundle = nullptr; bool allow_source_manifest = false; if (config_from == "system") { - if (!system_preset_resolver) - system_preset_resolver = std::make_unique(); - bundle = system_preset_resolver.get(); + bundle = &ensure_system_preset_resolver(); allow_source_manifest = true; } else { bundle = ensure_cli_preset_bundle(error); @@ -3120,6 +3123,49 @@ int CLI::run(int argc, char **argv) return 0; }; + // A project saved before a printer or process option existed has no value for it. The GUI takes such + // keys from the project's system preset (load_external_preset refreshes every key the project did not + // override), so fill them from there too instead of leaving them to the option default. + // The extruder variant keys describe the project's variant layout and are kept as they are, so an + // older project is not left with a variant list from one layout and ids from another. + auto fill_missing_project_keys = [this, &ensure_system_preset_resolver](const std::string &system_name, Preset::Type type) { + if (system_name.empty()) + return; + static const std::set skip_keys = { + "inherits", "compatible_printers", "compatible_prints", "compatible_printers_condition", "compatible_prints_condition", + "print_settings_id", "filament_settings_id", "printer_settings_id", + "print_host", "print_host_webui", "printhost_apikey", "printhost_cafile", "printhost_user", "printhost_password", "printhost_port", + "printer_extruder_id", "printer_extruder_variant", "print_extruder_id", "print_extruder_variant", "extruder_variant_list"}; + const std::vector &options = type == Preset::TYPE_PRINTER ? Preset::printer_options() : Preset::print_options(); + // Keys the legacy handler drops on load can never be in a project, so they do not count as missing. + auto dropped_on_load = [](std::string key) { + std::string value; + PrintConfigDef::handle_legacy(key, value); + return key.empty(); + }; + std::vector missing; + for (const std::string &key : options) + if (m_print_config.option(key) == nullptr && skip_keys.count(key) == 0 && !dropped_on_load(key)) + missing.push_back(key); + if (missing.empty()) + return; + DynamicPrintConfig system_config; + std::string error; + if (!ensure_system_preset_resolver().resolve_system_preset(system_config, type, system_name, config_substitution_rule, error)) { + BOOST_LOG_TRIVIAL(warning) << boost::format("CLI: system preset '%1%' not resolved (%2%); keys missing from the project keep their defaults") % system_name % error; + return; + } + for (const std::string &key : missing) + if (const ConfigOption *opt = system_config.option(key)) { + m_print_config.set_key_value(key, opt->clone()); + BOOST_LOG_TRIVIAL(info) << boost::format("CLI: %1% missing from the project, taken from '%2%': %3%") % key % system_name % opt->serialize(); + } + }; + if (new_printer_name.empty()) + fill_missing_project_keys(current_printer_system_name, Preset::TYPE_PRINTER); + if (new_process_name.empty()) + fill_missing_project_keys(current_process_system_name, Preset::TYPE_PRINT); + std::vector& different_settings = m_print_config.option("different_settings_to_system", true)->values; std::vector& inherits_group = m_print_config.option("inherits_group", true)->values; inherits_group.resize(filament_count + 2, std::string()); diff --git a/src/libslic3r/PresetBundle.cpp b/src/libslic3r/PresetBundle.cpp index c8fe00b17c..53430cb347 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -575,18 +575,20 @@ bool PresetBundle::resolve_preset_config(DynamicPrintConfig &config, Preset::Typ const PresetBundle *PresetBundle::load_source_vendor(const boost::filesystem::path &root_dir, const std::string &vendor_id, ForwardCompatibilitySubstitutionRule compatibility_rule, - std::string &error) + std::string &error, bool allow_cache) { - auto key = std::make_tuple(root_dir.string(), vendor_id, compatibility_rule); + auto key = std::make_tuple(root_dir.string(), vendor_id, compatibility_rule, allow_cache); if (auto it = m_source_vendor_bundles.find(key); it != m_source_vendor_bundles.end()) return it->second.get(); // The library loads with no base of its own, so the tree a vendor inherits from // is the same one that resolves the library's own presets. - const PresetBundle *library = nullptr; + const std::string library_file = std::string(ORCA_FILAMENT_LIBRARY); + const PresetBundle *library = nullptr; if (vendor_id != ORCA_FILAMENT_LIBRARY && - boost::filesystem::is_regular_file(root_dir / (std::string(ORCA_FILAMENT_LIBRARY) + ".json"))) { - library = load_source_vendor(root_dir, ORCA_FILAMENT_LIBRARY, compatibility_rule, error); + (boost::filesystem::is_regular_file(root_dir / (library_file + ".json")) || + (allow_cache && boost::filesystem::is_regular_file(root_dir / (library_file + ".opc"))))) { + library = load_source_vendor(root_dir, ORCA_FILAMENT_LIBRARY, compatibility_rule, error, allow_cache); if (library == nullptr) { error = "OrcaFilamentLibrary contains invalid presets"; return nullptr; @@ -595,7 +597,7 @@ const PresetBundle *PresetBundle::load_source_vendor(const boost::filesystem::pa auto bundle = std::make_unique(); bundle->m_preserve_vendor_source_paths = true; - bundle->load_vendor_configs_from_json(root_dir.string(), vendor_id, LoadSystem, compatibility_rule, library, false); + bundle->load_vendor_configs_from_json(root_dir.string(), vendor_id, LoadSystem, compatibility_rule, library, allow_cache); if (bundle->error_count() != 0) { error = "Vendor bundle contains invalid presets"; return nullptr; @@ -638,6 +640,43 @@ bool PresetBundle::resolve_preset_config_type(DynamicPrintConfig &config, Preset return true; } +bool PresetBundle::resolve_system_preset(DynamicPrintConfig &config, Preset::Type type, const std::string &name, + ForwardCompatibilitySubstitutionRule compatibility_rule, std::string &error) +{ + const std::string vendor_id = find_preset_vendor(name, type); + if (vendor_id.empty()) { + error = "No vendor lists the preset"; + return false; + } + // Release builds ship a vendor as its preset cache alone, without the profile JSONs. + auto installed = [&vendor_id](const fs::path &root) { + return fs::is_regular_file(root / (vendor_id + ".json")) || fs::is_regular_file(root / (vendor_id + ".opc")); + }; + fs::path root_dir = fs::path(data_dir()) / PRESET_SYSTEM_DIR; + if (!installed(root_dir)) + root_dir = fs::path(resources_dir()) / PRESET_PROFILES_DIR; + const bool cache_only = !fs::is_regular_file(root_dir / (vendor_id + ".json")); + + try { + const PresetBundle *vendor = load_source_vendor(root_dir, vendor_id, compatibility_rule, error, cache_only); + if (vendor == nullptr) + return false; + const PresetCollection &collection = type == Preset::TYPE_PRINTER ? vendor->printers : + type == Preset::TYPE_PRINT ? vendor->prints : vendor->filaments; + const Preset *preset = collection.find_preset(name, false); + if (preset == nullptr) { + error = "Preset was not found in its vendor bundle"; + return false; + } + config = preset->config; + } catch (const std::exception &ex) { + error = ex.what(); + return false; + } + error.clear(); + return true; +} + PresetBundle::PresetBundle(const PresetBundle &rhs) { *this = rhs; diff --git a/src/libslic3r/PresetBundle.hpp b/src/libslic3r/PresetBundle.hpp index 741f6069a4..220c9b8f37 100644 --- a/src/libslic3r/PresetBundle.hpp +++ b/src/libslic3r/PresetBundle.hpp @@ -273,6 +273,10 @@ public: const std::string &source_file, ForwardCompatibilitySubstitutionRule compatibility_rule, std::string &error, bool allow_source_manifest = true); + // Resolve a system preset by name. The vendor tree is read from data_dir()/system when installed + // there, as the GUI reads it, and from the bundled profiles otherwise. + bool resolve_system_preset(DynamicPrintConfig &config, Preset::Type type, const std::string &name, + ForwardCompatibilitySubstitutionRule compatibility_rule, std::string &error); // Load selections (current print, current filaments, current printer) from config.ini // This is done just once on application start up. @@ -663,13 +667,14 @@ private: // Vendor trees loaded by resolve_preset_config's manifest path, so every preset // resolved through this bundle shares one load per source root and vendor. The // filament library is one such tree, shared by every vendor under its root. - std::map, std::unique_ptr> + // A tree read from its preset cache is kept apart: its presets carry no source file. + std::map, std::unique_ptr> m_source_vendor_bundles; const PresetBundle *load_source_vendor(const boost::filesystem::path &root_dir, const std::string &vendor_id, ForwardCompatibilitySubstitutionRule compatibility_rule, - std::string &error); + std::string &error, bool allow_cache = false); // Orca: validation only - flag any printer with two or more compatible // filament presets sharing one filament_id (ambiguous AMS subtype match). diff --git a/tests/cli/CMakeLists.txt b/tests/cli/CMakeLists.txt index 6707159fb2..a0c9f2c078 100644 --- a/tests/cli/CMakeLists.txt +++ b/tests/cli/CMakeLists.txt @@ -15,3 +15,11 @@ set_tests_properties(cli_strict_mode PROPERTIES LABELS "CLI;RequiresApp" SKIP_RETURN_CODE 77 TIMEOUT 900) + +add_test(NAME cli_project_missing_keys + COMMAND bash ${CMAKE_CURRENT_SOURCE_DIR}/test_cli_project_missing_keys.sh $ ${ORCA_CLI_TEST_PYTHON} + ${CMAKE_SOURCE_DIR}/resources/profiles/BBL) +set_tests_properties(cli_project_missing_keys PROPERTIES + LABELS "CLI;RequiresApp" + SKIP_RETURN_CODE 77 + TIMEOUT 900) diff --git a/tests/cli/test_cli_project_missing_keys.sh b/tests/cli/test_cli_project_missing_keys.sh new file mode 100644 index 0000000000..d22ca6bec2 --- /dev/null +++ b/tests/cli/test_cli_project_missing_keys.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# End-to-end check that the CLI fills settings missing from a project from the project's system presets. +# +# A project saved before an option existed has no value for it. The GUI takes such keys from the +# project's system printer and process presets, not from the option defaults, and the CLI must slice +# the project with the same values. A project is exported from the shipped Bambu Lab P1S presets, one +# printer key and one process key are removed from it, one kept key is changed, and it is sliced again. +# +# usage: test_cli_project_missing_keys.sh +set -u + +BIN="${1:-}" +PY="${2:-python3}" +PROFILES="${3:-}" +# 77 is the test's SKIP_RETURN_CODE. +[ -x "$BIN" ] || { echo "SKIP: orca-slicer binary not found: $BIN"; exit 77; } +[ -d "$PROFILES" ] || { echo "FAIL: profiles directory not found: $PROFILES"; exit 1; } + +WORK="$(mktemp -d "${TMPDIR:-/tmp}/orca-cli-missing-keys.XXXXXX")" +trap 'rm -rf "$WORK"' EXIT + +"$PY" - "$WORK/cube.stl" <<'EOF' +import sys + +v = [(x, y, z) for z in (0, 10) for y in (0, 10) for x in (0, 10)] +with open(sys.argv[1], "w") as f: + f.write("solid cube\n") + for a, b, c, d in ((0, 2, 3, 1), (4, 5, 7, 6), (0, 1, 5, 4), (2, 6, 7, 3), (0, 4, 6, 2), (1, 3, 7, 5)): + for tri in ((v[a], v[b], v[c]), (v[a], v[c], v[d])): + f.write("facet normal 0 0 0\nouter loop\n") + for p in tri: + f.write("vertex %g %g %g\n" % p) + f.write("endloop\nendfacet\n") + f.write("endsolid cube\n") +EOF + +# slice [option...]: slice into $WORK//out.3mf with a fresh data directory. +slice() { + local out="$WORK/$1" input="$2"; shift 2 + mkdir -p "$out" + timeout 300 "$BIN" --datadir "$out/datadir" "$@" --slice 0 --outputdir "$out" --export-3mf out.3mf "$input" \ + > "$out/log" 2>&1 || { echo "FAIL: $1: orca-slicer exited $?"; tail -n 40 "$out/log"; exit 1; } +} + +slice base "$WORK/cube.stl" \ + --load-settings "$PROFILES/machine/Bambu Lab P1S 0.4 nozzle.json;$PROFILES/process/0.20mm Standard @BBL X1C.json" \ + --load-filaments "$PROFILES/filament/Bambu PLA Basic @BBL P1S 0.4 nozzle.json" + +# The removed keys, with their option defaults from PrintConfig.cpp, and a kept key with a new value. +"$PY" - "$WORK/base/out.3mf" "$WORK/old.3mf" <<'EOF' || exit $? +import json, sys, zipfile + +src, dst = sys.argv[1], sys.argv[2] +missing = {"extruder_clearance_dist_to_rod": "40", "sparse_infill_density": "20%"} +with zipfile.ZipFile(src) as zin, zipfile.ZipFile(dst, "w", zipfile.ZIP_DEFLATED) as zout: + for item in zin.infolist(): + data = zin.read(item.filename) + if item.filename == "Metadata/project_settings.config": + config = json.loads(data) + for key, default in missing.items(): + if config[key] == default: + print("SKIP: %s is %s in the system preset, the option default, so the test cannot tell them apart" % (key, default)) + sys.exit(77) + expected = {key: config.pop(key) for key in missing} + expected["wall_loops"] = str(int(config["wall_loops"]) + 1) + config["wall_loops"] = expected["wall_loops"] + data = json.dumps(config, indent=4) + zout.writestr(item, data) +with open(dst + ".expected.json", "w") as f: + json.dump(expected, f) +EOF + +slice project "$WORK/old.3mf" + +"$PY" - "$WORK/project/out.3mf" "$WORK/old.3mf.expected.json" <<'EOF' +import json, sys, zipfile + +with zipfile.ZipFile(sys.argv[1]) as z: + config = json.loads(z.read("Metadata/project_settings.config")) +with open(sys.argv[2]) as f: + expected = json.load(f) +errors = ["%s is %r, want %r" % (key, config.get(key), want) for key, want in expected.items() if config.get(key) != want] +for e in errors: + print("FAIL: " + e) +sys.exit(1 if errors else 0) +EOF +status=$? +[ "$status" -eq 0 ] || { tail -n 40 "$WORK/project/log"; exit 1; } +echo "PASS" diff --git a/tests/libslic3r/test_preset_bundle_loading.cpp b/tests/libslic3r/test_preset_bundle_loading.cpp index 6ec6ba9b1e..291be98ee8 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -5405,6 +5405,35 @@ struct ScopedDataDir ~ScopedDataDir() { set_data_dir(previous); } }; +// resources_dir() is process-wide too; system preset lookups scan its profiles directory. +struct ScopedResourcesDir +{ + std::string previous = resources_dir(); + explicit ScopedResourcesDir(const fs::path &dir) { set_resources_dir(dir.string()); } + ~ScopedResourcesDir() { set_resources_dir(previous); } +}; + +// An "Acme" vendor under root whose "Acme Printer" inherits extruder_clearance_dist_to_rod from an +// abstract base, with the printer in a nested sub_path so the name cannot be derived from the file. +void write_acme_printer_vendor(const fs::path &root, double dist_to_rod) +{ + const fs::path machine_dir = root / "Acme" / "machine"; + fs::create_directories(machine_dir / "nested"); + std::ofstream((root / "Acme.json").string()) + << R"({"version":"1.0.0","name":"Acme",)" + << R"("machine_model_list":[{"name":"Acme One","sub_path":"machine/model.json"}],"machine_list":[)" + << R"({"name":"fdm_acme_common","sub_path":"machine/base.json"},)" + << R"({"name":"Acme Printer","sub_path":"machine/nested/printer.json"}]})"; + std::ofstream((machine_dir / "model.json").string()) + << R"({"type":"machine_model","name":"Acme One","nozzle_diameter":"0.4"})"; + std::ofstream((machine_dir / "base.json").string()) + << R"({"type":"machine","name":"fdm_acme_common","from":"system","instantiation":"false",)" + << R"("extruder_clearance_dist_to_rod":")" << dist_to_rod << R"("})"; + std::ofstream((machine_dir / "nested" / "printer.json").string()) + << R"({"type":"machine","name":"Acme Printer","from":"system","instantiation":"true","inherits":"fdm_acme_common",)" + << R"("printer_model":"Acme One","printer_variant":"0.4"})"; +} + std::string read_file(const fs::path &file) { std::ifstream in(file.string(), std::ios::binary); @@ -5482,3 +5511,75 @@ TEST_CASE("Config import confines zip entries, preset names and bundle ids to th CHECK_FALSE(any_filename_contains(temp_dir.path(), "bundle-escape")); } } + +TEST_CASE("A system preset resolves by name from the bundled profiles", "[Preset][Bundle]") +{ + ScopedTemporaryDir temp_dir; + ScopedDataDir data(temp_dir.path() / "data"); + ScopedResourcesDir resources(temp_dir.path() / "resources"); + write_acme_printer_vendor(temp_dir.path() / "resources" / "profiles", 33.); + + PresetBundle bundle; + DynamicPrintConfig config; + std::string error; + REQUIRE(bundle.resolve_system_preset(config, Preset::TYPE_PRINTER, "Acme Printer", + ForwardCompatibilitySubstitutionRule::EnableSilent, error)); + CHECK(error.empty()); + CHECK_THAT(config.opt_float("extruder_clearance_dist_to_rod"), Catch::Matchers::WithinAbs(33., 1e-6)); +} + +TEST_CASE("A system preset resolves from the data directory copy of its vendor", "[Preset][Bundle]") +{ + ScopedTemporaryDir temp_dir; + ScopedDataDir data(temp_dir.path() / "data"); + ScopedResourcesDir resources(temp_dir.path() / "resources"); + write_acme_printer_vendor(temp_dir.path() / "resources" / "profiles", 33.); + write_acme_printer_vendor(temp_dir.path() / "data" / PRESET_SYSTEM_DIR, 35.); + + PresetBundle bundle; + DynamicPrintConfig config; + std::string error; + REQUIRE(bundle.resolve_system_preset(config, Preset::TYPE_PRINTER, "Acme Printer", + ForwardCompatibilitySubstitutionRule::EnableSilent, error)); + CHECK_THAT(config.opt_float("extruder_clearance_dist_to_rod"), Catch::Matchers::WithinAbs(35., 1e-6)); +} + +TEST_CASE("A system preset resolves from a vendor shipped as its preset cache alone", "[Preset][Bundle]") +{ + ScopedTemporaryDir temp_dir; + ScopedDataDir data(temp_dir.path() / "data"); + ScopedResourcesDir resources(temp_dir.path() / "resources"); + const fs::path profiles = temp_dir.path() / "resources" / "profiles"; + write_acme_printer_vendor(profiles, 33.); + + PresetBundle writer; + writer.set_generate_vendor_caches(true); + writer.load_vendor_configs_from_json(profiles.string(), "Acme", PresetBundle::LoadSystem, + ForwardCompatibilitySubstitutionRule::EnableSilent); + REQUIRE(fs::exists(profiles / "Acme.opc")); + // Release builds ship the cache and drop the profile JSONs, manifest included. + fs::remove(profiles / "Acme.json"); + fs::remove_all(profiles / "Acme"); + + PresetBundle bundle; + DynamicPrintConfig config; + std::string error; + REQUIRE(bundle.resolve_system_preset(config, Preset::TYPE_PRINTER, "Acme Printer", + ForwardCompatibilitySubstitutionRule::EnableSilent, error)); + CHECK_THAT(config.opt_float("extruder_clearance_dist_to_rod"), Catch::Matchers::WithinAbs(33., 1e-6)); +} + +TEST_CASE("A system preset no vendor lists is not resolved", "[Preset][Bundle]") +{ + ScopedTemporaryDir temp_dir; + ScopedDataDir data(temp_dir.path() / "data"); + ScopedResourcesDir resources(temp_dir.path() / "resources"); + write_acme_printer_vendor(temp_dir.path() / "resources" / "profiles", 33.); + + PresetBundle bundle; + DynamicPrintConfig config; + std::string error; + CHECK_FALSE(bundle.resolve_system_preset(config, Preset::TYPE_PRINTER, "Unknown Printer", + ForwardCompatibilitySubstitutionRule::EnableSilent, error)); + CHECK_FALSE(error.empty()); +}