Share One Library Load Between Vendors in the CLI Preset Resolver

The manifest resolver loaded OrcaFilamentLibrary once per vendor it
resolved through, so a run that mixes vendors parsed the library tree
again for each of them. The library is now cached like any other vendor
tree, keyed on its root and substitution rule, and doubles as the base
every vendor under that root loads against. A vendor bundle only reads
from its base while loading, so sharing the instance is safe.

The cache key carries the substitution rule as its enum, and the lookup
lambdas take a const bundle since they only read.

src/libslic3r/PresetBundle.cpp

@@ -484,7 +484,7 @@ bool PresetBundle::resolve_preset_config(DynamicPrintConfig &config, Preset::Typ
     else if (compatibility_rule == ForwardCompatibilitySubstitutionRule::EnableSilentDisableSystem)
         compatibility_rule = ForwardCompatibilitySubstitutionRule::Disable;
 
-    auto collection_for_type = [](PresetBundle &bundle, Preset::Type preset_type) -> PresetCollection * {
+    auto collection_for_type = [](const PresetBundle &bundle, Preset::Type preset_type) -> const PresetCollection * {
         switch (preset_type) {
         case Preset::TYPE_PRINT:    return &bundle.prints;
         case Preset::TYPE_FILAMENT: return &bundle.filaments;
@@ -493,15 +493,15 @@ bool PresetBundle::resolve_preset_config(DynamicPrintConfig &config, Preset::Typ
         }
     };
 
-    PresetCollection *collection = collection_for_type(*this, type);
+    const PresetCollection *collection = collection_for_type(*this, type);
     if (collection == nullptr) {
         error = "Unsupported preset type";
         return false;
     }
 
     const boost::filesystem::path source_path = boost::filesystem::absolute(source_file).lexically_normal();
-    auto find_loaded = [&](PresetBundle &bundle) -> const Preset * {
+    auto find_loaded = [&](const PresetBundle &bundle) -> const Preset * {
-        PresetCollection *loaded_collection = collection_for_type(bundle, type);
+        const PresetCollection *loaded_collection = collection_for_type(bundle, type);
         const Preset *resolved = nullptr;
         for (const Preset &preset : loaded_collection->get_presets()) {
             if (preset.file.empty())
@@ -549,11 +549,11 @@ bool PresetBundle::resolve_preset_config(DynamicPrintConfig &config, Preset::Typ
             continue;
 
         try {
-            const SourceManifestBundles *loaded = load_source_manifest(root_dir, vendor_id, compatibility_rule, error);
+            const PresetBundle *loaded = load_source_vendor(root_dir, vendor_id, compatibility_rule, error);
             if (loaded == nullptr)
                 return false;
 
-            const Preset *resolved = find_loaded(*loaded->vendor);
+            const Preset *resolved = find_loaded(*loaded);
             if (resolved == nullptr) {
                 if (error.empty())
                     error = "Source file is not an instantiated preset in its vendor manifest";
@@ -572,37 +572,35 @@ bool PresetBundle::resolve_preset_config(DynamicPrintConfig &config, Preset::Typ
     return false;
 }
 
-const PresetBundle::SourceManifestBundles *PresetBundle::load_source_manifest(const boost::filesystem::path &root_dir,
+const PresetBundle *PresetBundle::load_source_vendor(const boost::filesystem::path &root_dir,
                                                     const std::string &vendor_id,
                                                     ForwardCompatibilitySubstitutionRule compatibility_rule,
                                                     std::string &error)
 {
-    auto key = std::make_tuple(root_dir.string(), vendor_id, static_cast<int>(compatibility_rule));
+    auto key = std::make_tuple(root_dir.string(), vendor_id, compatibility_rule);
-    if (auto it = m_source_manifest_bundles.find(key); it != m_source_manifest_bundles.end())
+    if (auto it = m_source_vendor_bundles.find(key); it != m_source_vendor_bundles.end())
-        return &it->second;
+        return it->second.get();
 
-    SourceManifestBundles loaded;
+    // 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;
     if (vendor_id != ORCA_FILAMENT_LIBRARY &&
         boost::filesystem::is_regular_file(root_dir / (std::string(ORCA_FILAMENT_LIBRARY) + ".json"))) {
-        loaded.library = std::make_unique<PresetBundle>();
+        library = load_source_vendor(root_dir, ORCA_FILAMENT_LIBRARY, compatibility_rule, error);
-        loaded.library->m_preserve_vendor_source_paths = true;
+        if (library == nullptr) {
-        loaded.library->load_vendor_configs_from_json(root_dir.string(), ORCA_FILAMENT_LIBRARY, LoadSystem,
-                                                      compatibility_rule, nullptr, false);
-        if (loaded.library->error_count() != 0) {
             error = "OrcaFilamentLibrary contains invalid presets";
             return nullptr;
         }
     }
 
-    loaded.vendor = std::make_unique<PresetBundle>();
+    auto bundle = std::make_unique<PresetBundle>();
-    loaded.vendor->m_preserve_vendor_source_paths = true;
+    bundle->m_preserve_vendor_source_paths = true;
-    loaded.vendor->load_vendor_configs_from_json(root_dir.string(), vendor_id, LoadSystem,
+    bundle->load_vendor_configs_from_json(root_dir.string(), vendor_id, LoadSystem, compatibility_rule, library, false);
-                                                 compatibility_rule, loaded.library.get(), false);
+    if (bundle->error_count() != 0) {
-    if (loaded.vendor->error_count() != 0) {
         error = "Vendor bundle contains invalid presets";
         return nullptr;
     }
-    return &m_source_manifest_bundles.emplace(std::move(key), std::move(loaded)).first->second;
+    return m_source_vendor_bundles.emplace(std::move(key), std::move(bundle)).first->second.get();
 }
 
 bool PresetBundle::resolve_preset_config_type(DynamicPrintConfig &config, Preset::Type &type,

src/libslic3r/PresetBundle.hpp

@@ -654,14 +654,12 @@ private:
     bool m_preserve_vendor_source_paths { false };
 
     // 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.
+    // resolved through this bundle shares one load per source root and vendor. The
-    struct SourceManifestBundles {
+    // filament library is one such tree, shared by every vendor under its root.
-        std::unique_ptr<PresetBundle> library;
+    std::map<std::tuple<std::string, std::string, ForwardCompatibilitySubstitutionRule>, std::unique_ptr<PresetBundle>>
-        std::unique_ptr<PresetBundle> vendor;
+        m_source_vendor_bundles;
-    };
-    std::map<std::tuple<std::string, std::string, int>, SourceManifestBundles> m_source_manifest_bundles;
 
-    const SourceManifestBundles *load_source_manifest(const boost::filesystem::path &root_dir,
+    const PresetBundle *load_source_vendor(const boost::filesystem::path &root_dir,
                                            const std::string &vendor_id,
                                            ForwardCompatibilitySubstitutionRule compatibility_rule,
                                            std::string &error);

tests/libslic3r/test_preset_bundle_loading.cpp

@@ -1118,6 +1118,62 @@ TEST_CASE("Manifest-backed resolution reuses the library base for type-probed fi
     CHECK_THAT(density(second), Catch::Matchers::WithinAbs(1.27, 1e-6));
 }
 
+TEST_CASE("Manifest-backed resolution shares the library between vendors under one root", "[Preset][Bundle][Regression]")
+{
+    ScopedTemporaryDir dir;
+    const fs::path      library_dir = dir.path() / PresetBundle::ORCA_FILAMENT_LIBRARY / "filament";
+
+    std::ofstream((dir.path() / (std::string(PresetBundle::ORCA_FILAMENT_LIBRARY) + ".json")).string())
+        << R"({"version":"1.0.0","name":"OrcaFilamentLibrary","filament_list":[)"
+        << R"({"name":"fdm_filament_pet","sub_path":"filament/pet.json","filament_id":"GFL99"},)"
+        << R"({"name":"Generic PETG","sub_path":"filament/generic_petg.json","filament_id":"GFL98"}]})";
+    fs::create_directories(library_dir);
+    auto write_library_pet = [&](double density) {
+        std::ofstream((library_dir / "pet.json").string())
+            << R"({"type":"filament","name":"fdm_filament_pet","from":"system",)"
+            << R"("filament_id":"GFL99","instantiation":"false",)"
+            << R"("filament_type":["PETG"],"filament_density":[")" << density << R"("]})";
+    };
+    write_library_pet(1.27);
+    std::ofstream((library_dir / "generic_petg.json").string())
+        << R"({"type":"filament","name":"Generic PETG","from":"system",)"
+        << R"("filament_id":"GFL98","instantiation":"true","inherits":"fdm_filament_pet"})";
+
+    auto write_vendor = [&](const std::string &vendor, const std::string &filament_id) {
+        const fs::path filament_dir = dir.path() / vendor / "filament";
+        fs::create_directories(filament_dir);
+        std::ofstream((dir.path() / (vendor + ".json")).string())
+            << R"({"version":"1.0.0","name":")" << vendor << R"(","filament_list":[)"
+            << R"({"name":")" << vendor << R"( PETG","sub_path":"filament/petg.json","filament_id":")" << filament_id << R"("}]})";
+        std::ofstream((filament_dir / "petg.json").string())
+            << R"({"type":"filament","name":")" << vendor << R"( PETG","from":"system",)"
+            << R"("filament_id":")" << filament_id << R"(","instantiation":"true","inherits":"fdm_filament_pet"})";
+        return filament_dir / "petg.json";
+    };
+    const fs::path acme_petg = write_vendor("Acme", "GFA00");
+    const fs::path beta_petg = write_vendor("Beta", "GFB00");
+
+    auto density = [&](PresetBundle &bundle, const fs::path &file) {
+        DynamicPrintConfig raw;
+        raw.option<ConfigOptionString>(BBL_JSON_KEY_INHERITS, true)->value = "fdm_filament_pet";
+        std::string error;
+        REQUIRE(bundle.resolve_preset_config(raw, Preset::TYPE_FILAMENT, file.string(),
+                                             ForwardCompatibilitySubstitutionRule::EnableSilent, error));
+        return raw.option<ConfigOptionFloats>("filament_density")->values.front();
+    };
+
+    PresetBundle bundle;
+    CHECK_THAT(density(bundle, acme_petg), Catch::Matchers::WithinAbs(1.27, 1e-6));
+
+    // Only a reload would see this change.
+    write_library_pet(1.5);
+    CHECK_THAT(density(bundle, beta_petg), Catch::Matchers::WithinAbs(1.27, 1e-6));
+    CHECK_THAT(density(bundle, library_dir / "generic_petg.json"), Catch::Matchers::WithinAbs(1.27, 1e-6));
+
+    PresetBundle fresh;
+    CHECK_THAT(density(fresh, beta_petg), Catch::Matchers::WithinAbs(1.5, 1e-6));
+}
+
 // Orca: a filament in the Orca Filament Library that names its compatible printers has to hide the generic
 // library filament sharing its alias, the same way a vendor owned filament does. Otherwise both are compatible
 // with that printer and the plater combo box lists the shared alias twice.