diff --git a/docs/HLSD/preset-cache.md b/docs/HLSD/preset-cache.md index c774910637..aa39d522a0 100644 --- a/docs/HLSD/preset-cache.md +++ b/docs/HLSD/preset-cache.md @@ -170,7 +170,7 @@ merged in a stable order: ```mermaid flowchart LR - lib["1 · OrcaFilamentLibrary loaded;
meanwhile every other vendor read
from its cache or its JSONs"] --> par["2 · every other vendor installed
in parallel, each into its own bundle,
filaments resolving against the library"] --> merge["3 · bundles merged into one,
sequentially, in stable vendor order"] + lib["1 · OrcaFilamentLibrary loaded;
meanwhile every other vendor read
from its cache or its JSONs"] --> par["2 · every other vendor installed
in parallel, each into its own bundle,
filaments resolving against the library"] --> merge["3 · bundles merged into one,
in one pass per collection,
in stable vendor order"] ``` `PresetBundle::load_vendors` runs these steps for startup and for the setup wizard, diff --git a/src/libslic3r/Preset.cpp b/src/libslic3r/Preset.cpp index e00ffa5eea..3fc7de5a53 100644 --- a/src/libslic3r/Preset.cpp +++ b/src/libslic3r/Preset.cpp @@ -3903,28 +3903,53 @@ bool PresetCollection::select_preset_by_name_strict(const std::string &name) return false; } -// Merge one vendor's presets with the other vendor's presets, report duplicates. -std::vector PresetCollection::merge_presets(PresetCollection &&other, const VendorMap &new_vendors) +std::vector> PresetCollection::merge_presets(const std::vector &others, const VendorMap &new_vendors) { - std::vector duplicates; - for (Preset &preset : other.m_presets) { - if (preset.is_default || preset.is_external) - continue; - Preset key(m_type, preset.name); - auto it = (m_type == Preset::TYPE_FILAMENT) - ? std::lower_bound(m_presets.begin() + m_num_default_presets, m_presets.end(), key, filament_preset_less) - : std::lower_bound(m_presets.begin() + m_num_default_presets, m_presets.end(), key); - if (it == m_presets.end() || it->name != preset.name) { + auto less = [this](const Preset &a, const Preset &b) { + return m_type == Preset::TYPE_FILAMENT ? filament_preset_less(a, b) : a < b; + }; + struct Incoming { Preset *preset; size_t source; }; + auto incoming_less = [&less](const Incoming &a, const Incoming &b) { return less(*a.preset, *b.preset); }; + // Each of `others` is sorted, so its presets form one sorted run. + std::vector incoming; + std::vector run_ends { 0 }; + for (size_t source = 0; source < others.size(); ++ source) { + for (Preset &preset : others[source]->m_presets) + if (! preset.is_default && ! preset.is_external) + incoming.push_back({ &preset, source }); + assert(std::is_sorted(incoming.begin() + run_ends.back(), incoming.end(), incoming_less)); + run_ends.push_back(incoming.size()); + } + // Merged pairwise and stably, so equal names stay in the order of `others`. + const size_t runs = others.size(); + for (size_t width = 1; width < runs; width *= 2) + for (size_t i = 0; i + width < runs; i += 2 * width) + std::inplace_merge(incoming.begin() + run_ends[i], incoming.begin() + run_ends[i + width], + incoming.begin() + run_ends[std::min(i + 2 * width, runs)], incoming_less); + + std::vector> duplicates(others.size()); + std::deque merged; + auto own = m_presets.begin() + m_num_default_presets; + std::move(m_presets.begin(), own, std::back_inserter(merged)); + // On equal names this collection's preset is kept, else the earliest of `others`, + // and each repeat is listed under the collection it came from. + for (auto next = incoming.begin(); own != m_presets.end() || next != incoming.end();) { + if (next == incoming.end() || (own != m_presets.end() && ! less(*next->preset, *own))) + merged.emplace_back(std::move(*own ++)); + else { + Preset &preset = *(next ++)->preset; if (preset.vendor != nullptr) { // Re-assign a pointer to the vendor structure in the new PresetBundle. auto it = new_vendors.find(preset.vendor->id); assert(it != new_vendors.end()); preset.vendor = &it->second; } - m_presets.emplace(it, std::move(preset)); - } else - duplicates.emplace_back(std::move(preset.name)); + merged.emplace_back(std::move(preset)); + } + for (; next != incoming.end() && next->preset->name == merged.back().name; ++ next) + duplicates[next->source].emplace_back(next->preset->name); } + m_presets = std::move(merged); return duplicates; } diff --git a/src/libslic3r/Preset.hpp b/src/libslic3r/Preset.hpp index 42b1cdb664..329e835afe 100644 --- a/src/libslic3r/Preset.hpp +++ b/src/libslic3r/Preset.hpp @@ -879,8 +879,10 @@ protected: // This is a temporary state, which shall be fixed immediately by the following step. bool select_preset_by_name_strict(const std::string &name); - // Merge one vendor's presets with the other vendor's presets, report duplicates. - std::vector merge_presets(PresetCollection &&other, const VendorMap &new_vendors); + // Move the presets of `others` into this collection in one pass. A name this + // collection or an earlier one of `others` already has is left out, and reported + // in the list of the collection that repeats it. + std::vector> merge_presets(const std::vector &others, const VendorMap &new_vendors); // Update m_map_alias_to_profile_name from loaded system profiles. void update_map_alias_to_profile_name(); diff --git a/src/libslic3r/PresetBundle.cpp b/src/libslic3r/PresetBundle.cpp index b8035618ad..9da2e88d21 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -2651,6 +2651,12 @@ std::pair PresetBundle::load_vendors(co // Merged in the original vendor order, so any duplicate-warning output stays // stable across runs. + std::vector bundles; + for (VendorLoad& load : loads) + if (load.bundle) + bundles.push_back(load.bundle.get()); + const std::vector> duplicates = this->merge_presets(bundles); + size_t merged = 0; for (VendorLoad& load : loads) { if (! load.error.empty()) { if (validation_mode) @@ -2664,18 +2670,18 @@ std::pair PresetBundle::load_vendors(co if (! load.bundle) continue; - const std::string& vendor_name = load.source->name; + const std::string& vendor_name = load.source->name; + const std::vector& vendor_duplicates = duplicates[merged ++]; append(substitutions, std::move(load.substitutions)); - std::vector duplicates = this->merge_presets(std::move(*load.bundle)); first = false; - if (!duplicates.empty()) { + if (!vendor_duplicates.empty()) { errors_cummulative += "Found duplicated settings in vendor " + vendor_name + "'s json file lists: "; - for (size_t j = 0; j < duplicates.size(); ++j) { - if (j > 0) + for (size_t k = 0; k < vendor_duplicates.size(); ++k) { + if (k > 0) errors_cummulative += ", "; - errors_cummulative += duplicates[j]; + errors_cummulative += vendor_duplicates[k]; ++m_errors; - BOOST_LOG_TRIVIAL(error) << "Found duplicated preset: " + duplicates[j] + " in vendor: " + vendor_name + ": "; + BOOST_LOG_TRIVIAL(error) << "Found duplicated preset: " + vendor_duplicates[k] + " in vendor: " + vendor_name + ": "; } } } @@ -2756,7 +2762,7 @@ std::pair PresetBundle::load_system_fil // Report duplicate profiles. PresetBundle other; append(substitutions, other.load_vendor_configs_from_json(dir.string(), vendor_name, PresetBundle::LoadSystem | PresetBundle::LoadFilamentOnly, compatibility_rule).first); - std::vector duplicates = this->merge_presets(std::move(other)); + std::vector duplicates = std::move(this->merge_presets({ &other }).front()); if (!duplicates.empty()) { errors_cummulative += "Found duplicated settings in vendor " + vendor_name + "'s json file lists: "; for (size_t i = 0; i < duplicates.size(); ++i) { @@ -2802,26 +2808,33 @@ VendorProfile PresetBundle::get_custom_vendor_models() const return vendor; } -// Merge one vendor's presets with the other vendor's presets, report duplicates. -std::vector PresetBundle::merge_presets(PresetBundle &&other) +std::vector> PresetBundle::merge_presets(const std::vector &others) { - this->vendors.insert(other.vendors.begin(), other.vendors.end()); - std::vector duplicate_prints = this->prints .merge_presets(std::move(other.prints), this->vendors); - std::vector duplicate_sla_prints = this->sla_prints .merge_presets(std::move(other.sla_prints), this->vendors); - std::vector duplicate_filaments = this->filaments .merge_presets(std::move(other.filaments), this->vendors); - std::vector duplicate_sla_materials = this->sla_materials.merge_presets(std::move(other.sla_materials), this->vendors); - std::vector duplicate_printers = this->printers .merge_presets(std::move(other.printers), this->vendors); - append(this->obsolete_presets.prints, std::move(other.obsolete_presets.prints)); - append(this->obsolete_presets.sla_prints, std::move(other.obsolete_presets.sla_prints)); - append(this->obsolete_presets.filaments, std::move(other.obsolete_presets.filaments)); - append(this->obsolete_presets.sla_materials, std::move(other.obsolete_presets.sla_materials)); - append(this->obsolete_presets.printers, std::move(other.obsolete_presets.printers)); - append(duplicate_prints, std::move(duplicate_sla_prints)); - append(duplicate_prints, std::move(duplicate_filaments)); - append(duplicate_prints, std::move(duplicate_sla_materials)); - append(duplicate_prints, std::move(duplicate_printers)); - m_errors += other.m_errors; - return duplicate_prints; + for (PresetBundle *other : others) + this->vendors.insert(other->vendors.begin(), other->vendors.end()); + std::vector> duplicates(others.size()); + auto merge = [&](auto collection) { + std::vector other_collections; + for (PresetBundle *other : others) + other_collections.push_back(&(other->*collection)); + std::vector> collection_duplicates = (this->*collection).merge_presets(other_collections, this->vendors); + for (size_t i = 0; i < others.size(); ++ i) + append(duplicates[i], std::move(collection_duplicates[i])); + }; + merge(&PresetBundle::prints); + merge(&PresetBundle::sla_prints); + merge(&PresetBundle::filaments); + merge(&PresetBundle::sla_materials); + merge(&PresetBundle::printers); + for (PresetBundle *other : others) { + append(this->obsolete_presets.prints, std::move(other->obsolete_presets.prints)); + append(this->obsolete_presets.sla_prints, std::move(other->obsolete_presets.sla_prints)); + append(this->obsolete_presets.filaments, std::move(other->obsolete_presets.filaments)); + append(this->obsolete_presets.sla_materials, std::move(other->obsolete_presets.sla_materials)); + append(this->obsolete_presets.printers, std::move(other->obsolete_presets.printers)); + m_errors += other->m_errors; + } + return duplicates; } void PresetBundle::update_system_maps() diff --git a/src/libslic3r/PresetBundle.hpp b/src/libslic3r/PresetBundle.hpp index 403cb89e6c..0bf1bbd5a5 100644 --- a/src/libslic3r/PresetBundle.hpp +++ b/src/libslic3r/PresetBundle.hpp @@ -641,8 +641,10 @@ public: const std::atomic* cancel = nullptr, std::vector* failed = nullptr); private: - // Merge one vendor's presets with the other vendor's presets, report duplicates. - std::vector merge_presets(PresetBundle &&other); + // Move the presets and vendor profiles of `others` into this bundle, in one pass + // over each collection. A preset whose name this bundle or an earlier one of + // `others` already has is left out and listed under the bundle that repeats it. + std::vector> merge_presets(const std::vector &others); // What parsing one entry's JSON sub-file reported. Its errors and warnings are // logged when the entry installs, so they come out in listing order with the diff --git a/tests/libslic3r/test_vendor_cache.cpp b/tests/libslic3r/test_vendor_cache.cpp index 4d42a5b0d4..45172404fa 100644 --- a/tests/libslic3r/test_vendor_cache.cpp +++ b/tests/libslic3r/test_vendor_cache.cpp @@ -904,6 +904,64 @@ TEST_CASE("a canceled vendor load starts no vendor", "[VendorCache]") CHECK(bundle.prints.find_preset("0.20mm Standard @Acme", false) == nullptr); } +TEST_CASE("a preset two vendors both define is kept from the first listed and reported under the others", "[VendorCache]") +{ + InstallDirs dirs; + for (const std::string vendor : { "Acme", "Mira", "Zeta" }) + write_process_vendor(dirs.system, vendor, { + { "Shared", process_json("Shared", R"("instantiation":"true",)") }, + { "Own @" + vendor, process_json("Own @" + vendor, R"("instantiation":"true",)") } }); + PresetBundle bundle; + const std::string errors = bundle.load_vendors({ { "Mira", dirs.system }, { "Acme", dirs.system }, { "Zeta", dirs.system } }, + ForwardCompatibilitySubstitutionRule::EnableSilent, + /*allow_cache=*/false).second; + + const Preset* shared = bundle.prints.find_preset("Shared", false); + REQUIRE(shared != nullptr); + REQUIRE(shared->vendor != nullptr); + CHECK(shared->vendor->id == "Mira"); + CHECK(errors.find("vendor Mira") == std::string::npos); + CHECK(errors.find("Found duplicated settings in vendor Acme's json file lists: Shared") != std::string::npos); + CHECK(errors.find("Found duplicated settings in vendor Zeta's json file lists: Shared") != std::string::npos); + CHECK(bundle.error_count() == 2); + for (const std::string vendor : { "Acme", "Mira", "Zeta" }) { + const Preset* own = bundle.prints.find_preset("Own @" + vendor, false); + REQUIRE(own != nullptr); + CHECK(own->vendor == &bundle.vendors.at(vendor)); + } +} + +TEST_CASE("filaments merged from several vendors come out generic first, then by name", "[VendorCache]") +{ + InstallDirs dirs; + auto write_filaments = [&](const std::string& vendor, const std::vector& names) { + fs::create_directories(dirs.system / vendor / "filament"); + std::ofstream index((dirs.system / (vendor + ".json")).string()); + index << R"({"version":"1.0.0","name":")" << vendor << R"(","filament_list":[)"; + for (size_t i = 0; i < names.size(); ++ i) { + const std::string sub_path = "filament/f" + std::to_string(i) + ".json"; + index << (i ? "," : "") << R"({"name":")" << names[i] << R"(","sub_path":")" << sub_path << R"("})"; + std::ofstream((dirs.system / vendor / sub_path).string()) + << R"({"type":"filament","name":")" << names[i] << R"(","from":"system","instantiation":"true",)" + << R"("filament_id":"GF)" << vendor << i << R"("})"; + } + index << "]}"; + }; + write_filaments("Zeta", { "Zeta PLA @0.4", "Generic PETG @Zeta" }); + write_filaments("Acme", { "Acme PLA @0.4", "Generic PLA @Acme" }); + PresetBundle bundle; + bundle.load_vendors({ { "Zeta", dirs.system }, { "Acme", dirs.system } }, + ForwardCompatibilitySubstitutionRule::EnableSilent, /*allow_cache=*/false); + + std::vector names; + for (const Preset& preset : bundle.filaments.get_presets()) + if (! preset.is_default) + names.push_back(preset.name); + CHECK(names == std::vector{ "Generic PETG @Zeta", "Generic PLA @Acme", "Acme PLA @0.4", "Zeta PLA @0.4" }); + for (const std::string& name : names) + CHECK(bundle.filaments.find_preset(name, false) != nullptr); +} + TEST_CASE("a vendor read while the filament library loads resolves against it, from JSON and from its cache", "[VendorCache]") { InstallDirs dirs;