diff --git a/src/OrcaSlicer.cpp b/src/OrcaSlicer.cpp index 1939988261..7222947717 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 c5b78a8e0f..c93e418306 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -581,18 +581,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; @@ -601,7 +603,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; @@ -644,6 +646,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 0bf1bbd5a5..44f11c4120 100644 --- a/src/libslic3r/PresetBundle.hpp +++ b/src/libslic3r/PresetBundle.hpp @@ -274,6 +274,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. @@ -808,13 +812,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 49f0f90214..270c38cebb 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -5571,6 +5571,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); @@ -5684,3 +5713,75 @@ TEST_CASE("A project saved with pressure advance per filament applies it to ever check_double_vector(pla.opt("filament_flow_ratio")->values, { 0.95, 0.96 }); check_double_vector(petg.opt("filament_flow_ratio")->values, { 0.97 }); } + +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()); +}