From 95e392e206108782c557b44009f5d620a333ea18 Mon Sep 17 00:00:00 2001 From: Lam Wei Lun Date: Fri, 21 Aug 2026 18:36:30 +0800 Subject: [PATCH] Code cleanup --- src/libslic3r/Format/bbs_3mf.cpp | 16 +++------- src/libslic3r/PresetBundle.cpp | 31 +++++++++++++------ src/libslic3r/PublishSettings.cpp | 28 +++++++---------- src/libslic3r/PublishSettings.hpp | 13 ++++++-- src/slic3r/GUI/ConfigValueFormatter.hpp | 5 +-- src/slic3r/GUI/Plater.cpp | 4 +-- src/slic3r/GUI/Plater.hpp | 2 +- src/slic3r/GUI/PublishSettingsDialog.cpp | 12 +++---- src/slic3r/GUI/PublishSettingsDialog.hpp | 5 +-- .../libslic3r/test_preset_bundle_loading.cpp | 10 ++++-- 10 files changed, 67 insertions(+), 59 deletions(-) diff --git a/src/libslic3r/Format/bbs_3mf.cpp b/src/libslic3r/Format/bbs_3mf.cpp index 06cc89de99..6415f29413 100644 --- a/src/libslic3r/Format/bbs_3mf.cpp +++ b/src/libslic3r/Format/bbs_3mf.cpp @@ -6987,17 +6987,11 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) stream << " <" << METADATA_TAG << " name=\"" << item.first << "\">" << xml_escape(item.second) << "\n"; if (item.first == BBL_APPLICATION_TAG) { - // The OrcaSlicer tag is the version receivers compare against their own to - // pick the import branch, and every graceful config-less branch of an - // Orca-classified file shows a baked-in "old OrcaSlicer version" popup. A - // minimal published 3MF omits the tag (together with the Application tag - // above): old receivers then classify it From_Other and import the geometry - // silently, while this build rebuilds the config from the published metadata - // payload before the branch runs. - if (!m_minimal_published) { - stream << " <" << METADATA_TAG << " name=\"" << ORCASLICER_TAG << "\">" - << xml_escape(SoftFever_VERSION) << "\n"; - } + // The OrcaSlicer tag is only written for files that carry the Application + // tag, which a minimal published 3MF omits (see the map assignment above): + // the branch below is unreachable in minimal mode. + stream << " <" << METADATA_TAG << " name=\"" << ORCASLICER_TAG << "\">" + << xml_escape(SoftFever_VERSION) << "\n"; } } diff --git a/src/libslic3r/PresetBundle.cpp b/src/libslic3r/PresetBundle.cpp index b6a3ed2d4b..fffd9c7fa4 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -43,6 +43,8 @@ namespace Slic3r { // Project-level options imported from a loaded 3MF into project_config. s_project_options_published // below is the reduced subset that crosses over in "published" 3MF mode; keep both in sync. +// s_project_options_published additionally carries wipe_tower_rotation_angle, which normal +// loads do not import (it is not listed here): published-only plate geometry. static std::vector s_project_options { "flush_volumes_vector", "flush_volumes_matrix", @@ -73,8 +75,9 @@ static std::vector s_project_options { "enable_filament_dynamic_map" }; -// Project options applied when loading a "published" 3MF project: the full s_project_options -// minus the filament/purge keys. A published file must not port the author's filament data +// Project options applied when loading a "published" 3MF project: s_project_options minus the +// filament/purge keys, plus wipe_tower_rotation_angle (plate geometry that only published +// loads import today). A published file must not port the author's filament data // (colors, colour types, filament/map/AMS slot state, purge/prime/flush volumes, nozzle // volume types, filament switcher state) to the receiver's project_config, which feeds the // scene colors, AMS slot colors and purge data. Only the plate/bed geometry keys cross over; @@ -4799,7 +4802,7 @@ void PresetBundle::load_config_file_config(const std::string &name_or_path, bool if (applied_keys.count(key) != 0) continue; // already applied // A '#' suffix denotes a variant key; resolve the base key. - const std::string base_key = key.substr(0, key.find('#')); + const std::string base_key = publish_base_key(key); // Structural keys are never applied (not "skipped due to mismatch"), so bail // out before the applied/skipped bookkeeping. if (structural_keys.count(base_key) != 0) @@ -4936,9 +4939,11 @@ void PresetBundle::load_config_file_config(const std::string &name_or_path, bool // type-only pick may surface an unrelated preset, e.g. a different vendor's // PLA). auto candidate_score = [](const Preset &candidate, const PublishedMaterialEntry &entry, const std::string &resolved_name) -> int { - // Exact preset name: raw and collection-resolved forms both score above the - // fuzzy bare/alias tier, so a receiver preset literally named "Generic PLA" - // can never beat the author's exact "Generic PLA @Vendor" preset on a tie. + // Exact preset name: raw and collection-resolved forms both outrank the + // fuzzy bare/alias tier by tier value, so a receiver preset literally named + // "Generic PLA" (tier 4) can never beat the author's exact + // "Generic PLA @Vendor" (tier 5). Within one tier the strict ">" + // comparison keeps the first candidate in collection order. if (!entry.preset_name.empty() && candidate.name == entry.preset_name) return 5; if (!resolved_name.empty() && candidate.name == resolved_name) @@ -5137,8 +5142,14 @@ void PresetBundle::load_config_file_config(const std::string &name_or_path, bool if (colours != nullptr && !colours->values.empty()) seed = colours->values.front(); } - if (seed.empty()) - seed = "#F2754E"; // filament_colour default + if (seed.empty()) { + // Fall back to the option's registered default instead of a + // duplicated literal; if the lookup fails the chip stays blank. + if (const ConfigOptionDef *colour_def = print_config_def.get("filament_colour")) + if (const auto *default_colours = dynamic_cast(colour_def->default_value.get())) + if (!default_colours->values.empty()) + seed = default_colours->values.front(); + } } if (proj_colour && slot < proj_colour->values.size()) proj_colour->values[slot] = seed; @@ -5153,7 +5164,7 @@ void PresetBundle::load_config_file_config(const std::string &name_or_path, bool auto apply_slot_keys = [&](DynamicPrintConfig &preset_config, const std::vector &slot_keys, int author_slot, const std::string &material_label) { for (const std::string &key : slot_keys) { - const std::string base_key = key.substr(0, key.find('#')); + const std::string base_key = publish_base_key(key); if (structural_keys.count(base_key) != 0) continue; const ConfigOption *src_opt = config.option(base_key); @@ -5375,7 +5386,7 @@ void PresetBundle::load_config_file_config(const std::string &name_or_path, bool for (const std::string &key : published_config->published_keys) { if (applied_keys.count(key) != 0) continue; - const std::string base_key = key.substr(0, key.find('#')); + const std::string base_key = publish_base_key(key); // Structural keys are silently ignored, never reported as skipped: a hand-crafted // 3MF must not trigger the "could not be applied" warning for them. if (structural_keys.count(base_key) != 0) diff --git a/src/libslic3r/PublishSettings.cpp b/src/libslic3r/PublishSettings.cpp index 8e2e58f5ee..e212770fd8 100644 --- a/src/libslic3r/PublishSettings.cpp +++ b/src/libslic3r/PublishSettings.cpp @@ -5,7 +5,6 @@ #include "PrintConfig.hpp" #include "MaterialType.hpp" -#include #include #include @@ -101,22 +100,18 @@ const std::set& publishable_printer_keys() std::vector collect_dirty_settings_keys(const PresetBundle& bundle) { - std::vector keys; - - auto append_dirty = [&keys](const std::vector& dirty) { - for (const std::string& key : dirty) { - if (std::find(keys.begin(), keys.end(), key) == keys.end()) - keys.push_back(key); - } - }; + std::set keys; // Union the dirty keys of each collection's edited preset (filaments may span multiple // slots); feeds only the Publish dialog's pre-check. - append_dirty(bundle.prints.current_dirty_options(true)); - append_dirty(bundle.printers.current_dirty_options(true)); - append_dirty(bundle.filaments.current_dirty_options(true)); + for (const std::string& key : bundle.prints.current_dirty_options(true)) + keys.insert(key); + for (const std::string& key : bundle.printers.current_dirty_options(true)) + keys.insert(key); + for (const std::string& key : bundle.filaments.current_dirty_options(true)) + keys.insert(key); - return keys; + return std::vector(keys.begin(), keys.end()); } DynamicPrintConfig filter_published_config( @@ -136,6 +131,7 @@ DynamicPrintConfig filter_published_config( std::map> slot_mask_map; // 1. Mandatory material identity & slot count keys for 3MF validation/normalization + // (filament_ids: exported for validation, denylisted on apply - see publish_structural_keys). static const std::vector s_material_identity_keys = { "filament_colour", "filament_type", @@ -163,7 +159,7 @@ DynamicPrintConfig filter_published_config( // 3. Process and printer published keys for (const std::string &key : published_keys) { - const std::string base_key = key.substr(0, key.find('#')); + const std::string base_key = publish_base_key(key); if (!base_key.empty()) { base_keys_to_include.insert(base_key); mask_exempt_keys.insert(base_key); @@ -175,7 +171,7 @@ DynamicPrintConfig filter_published_config( // slot-less entries (hand-crafted files) stay unmasked (whole vector). for (const PublishedMaterialEntry &entry : material_keys) { for (const std::string &key : entry.keys) { - const std::string base_key = key.substr(0, key.find('#')); + const std::string base_key = publish_base_key(key); if (base_key.empty()) continue; base_keys_to_include.insert(base_key); @@ -183,7 +179,7 @@ DynamicPrintConfig filter_published_config( slot_mask_map[base_key].insert(entry.slot); } for (const std::string &key : entry.full_keys) { - const std::string base_key = key.substr(0, key.find('#')); + const std::string base_key = publish_base_key(key); if (base_key.empty()) continue; base_keys_to_include.insert(base_key); diff --git a/src/libslic3r/PublishSettings.hpp b/src/libslic3r/PublishSettings.hpp index 0dc579c2a9..929c08dd51 100644 --- a/src/libslic3r/PublishSettings.hpp +++ b/src/libslic3r/PublishSettings.hpp @@ -6,8 +6,17 @@ namespace Slic3r { class PresetBundle; -// Structural keys that must never be published (single source of truth for the denylist): -// publishing them would rewrite the user's preset inheritance/structure. +// Strip a trailing "#N" variant suffix ("retraction_length#2" -> "retraction_length"). +inline std::string publish_base_key(const std::string &key) +{ + const size_t pos = key.find('#'); + return pos == std::string::npos ? key : key.substr(0, pos); +} + +// Structural keys that are never applied onto the receiver's presets when loading a published +// 3MF (single source of truth for the denylist): applying them would rewrite the user's preset +// inheritance/structure. filament_ids is nevertheless exported via the identity list in +// filter_published_config because 3MF validation needs it - exported, never applied. const std::set& publish_structural_keys(); // One row of the printer tab's "Retraction" / "Z-Hop" optgroups (key + tab icon id), kept diff --git a/src/slic3r/GUI/ConfigValueFormatter.hpp b/src/slic3r/GUI/ConfigValueFormatter.hpp index c51a8e8a17..67b16f9326 100644 --- a/src/slic3r/GUI/ConfigValueFormatter.hpp +++ b/src/slic3r/GUI/ConfigValueFormatter.hpp @@ -1,5 +1,4 @@ -#ifndef slic3r_ConfigValueFormatter_hpp_ -#define slic3r_ConfigValueFormatter_hpp_ +#pragma once #include @@ -25,5 +24,3 @@ wxString get_string_from_enum(const std::string& opt_key, const DynamicPrintConf } // namespace GUI } // namespace Slic3r - -#endif // slic3r_ConfigValueFormatter_hpp_ diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 908e4dc222..52d90623fe 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -16851,7 +16851,7 @@ void publish(Model &model, SaveStrategy strategy) { } // BBS: backup -int Plater::export_3mf(const boost::filesystem::path& output_path, SaveStrategy strategy, int export_plate_idx, Export3mfProgressFn proFn, const DynamicPrintConfig* override_config) +int Plater::export_3mf(const boost::filesystem::path& output_path, SaveStrategy strategy, int export_plate_idx, Export3mfProgressFn proFn) { int ret = 0; //if (p->model.objects.empty()) { @@ -16873,7 +16873,7 @@ int Plater::export_3mf(const boost::filesystem::path& output_path, SaveStrategy // modify model publish(p->model, strategy); - DynamicPrintConfig cfg = override_config ? *override_config : wxGetApp().preset_bundle->full_config_secure(); + DynamicPrintConfig cfg = wxGetApp().preset_bundle->full_config_secure(); const std::string path_u8 = into_u8(path); wxBusyCursor wait; diff --git a/src/slic3r/GUI/Plater.hpp b/src/slic3r/GUI/Plater.hpp index af599d6161..61e6bee15c 100644 --- a/src/slic3r/GUI/Plater.hpp +++ b/src/slic3r/GUI/Plater.hpp @@ -504,7 +504,7 @@ public: //void export_amf(); //BBS add extra param for exporting 3mf silence // BBS: backup - int export_3mf(const boost::filesystem::path& output_path = boost::filesystem::path(), SaveStrategy strategy = SaveStrategy::Default, int export_plate_idx = -1, Export3mfProgressFn proFn = nullptr, const DynamicPrintConfig* override_config = nullptr); + int export_3mf(const boost::filesystem::path& output_path = boost::filesystem::path(), SaveStrategy strategy = SaveStrategy::Default, int export_plate_idx = -1, Export3mfProgressFn proFn = nullptr); //BBS void publish_project(); diff --git a/src/slic3r/GUI/PublishSettingsDialog.cpp b/src/slic3r/GUI/PublishSettingsDialog.cpp index cd8b82a489..08adae903b 100644 --- a/src/slic3r/GUI/PublishSettingsDialog.cpp +++ b/src/slic3r/GUI/PublishSettingsDialog.cpp @@ -332,7 +332,7 @@ void PublishSettingsDialog::build_option_model() for (const auto& opt : optgroup->opt_map()) { // Row keys are base keys; the load side applies them positionally. const std::string& opt_id = opt.first; - std::string base = opt_id.substr(0, opt_id.find('#')); + std::string base = publish_base_key(opt_id); if (!material_added.insert(base).second) continue; // Show the value of this slot; fall back to slot 0 if out of range. @@ -388,15 +388,13 @@ void PublishSettingsDialog::build_option_model() // Pre-check the dirty (modified) settings and mark them bold (base-key match, across all // sections; collect_dirty_settings_keys unions the prints, printers and filaments). std::set dirty_base; - for (const std::string& key : collect_dirty_settings_keys(*wxGetApp().preset_bundle)) { - auto n = key.find('#'); - dirty_base.insert(n == std::string::npos ? key : key.substr(0, n)); - } + for (const std::string& key : collect_dirty_settings_keys(*wxGetApp().preset_bundle)) + dirty_base.insert(publish_base_key(key)); for (Row& row : m_rows) { // The Color/Type requirement rows are not "dirty overrides": never auto-checked. if (row.kind != RowKind::Setting) continue; - std::string base = row.key.substr(0, row.key.find('#')); + std::string base = publish_base_key(row.key); row.dirty = dirty_base.count(base) > 0; if (row.dirty) { row.check->SetValue(true); @@ -919,7 +917,7 @@ std::vector PublishSettingsDialog::GetPublishedKeys() const // build). Publish every extruder element so the load side can apply per-extruder // values even when the receiver has a different extruder count; a scalar printer // key is published as-is. - const std::string base_key = row.key.substr(0, row.key.find('#')); + const std::string base_key = publish_base_key(row.key); if (const ConfigOption* opt = full.option(base_key)) { if (const auto* vec = dynamic_cast(opt)) { for (size_t i = 0; i < vec->size(); ++i) diff --git a/src/slic3r/GUI/PublishSettingsDialog.hpp b/src/slic3r/GUI/PublishSettingsDialog.hpp index b474baa938..312931c0c1 100644 --- a/src/slic3r/GUI/PublishSettingsDialog.hpp +++ b/src/slic3r/GUI/PublishSettingsDialog.hpp @@ -1,5 +1,4 @@ -#ifndef slic3r_GUI_PublishSettingsDialog_hpp_ -#define slic3r_GUI_PublishSettingsDialog_hpp_ +#pragma once #include "GUI_Utils.hpp" #include "wxExtensions.hpp" @@ -200,5 +199,3 @@ private: }; }} // namespace Slic3r::GUI - -#endif // slic3r_GUI_PublishSettingsDialog_hpp_ diff --git a/tests/libslic3r/test_preset_bundle_loading.cpp b/tests/libslic3r/test_preset_bundle_loading.cpp index d3bf4a2f49..b68fafbc2a 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -584,6 +584,7 @@ TEST_CASE("Published 3MF overlays only the author-selected process keys onto the config.opt_string("print_settings_id", true) = "file process"; config.opt("flush_multiplier")->values = { 2., 2. }; // must NOT cross over config.opt("wipe_tower_x")->values = { 100. }; // plate geometry, does cross over + config.opt("wipe_tower_rotation_angle")->value = 45.; // published-only plate geometry, crosses over config.option("curr_bed_type")->setInt(BedType::btPC); // must NOT cross over return config; }; @@ -639,6 +640,7 @@ TEST_CASE("Published 3MF overlays only the author-selected process keys onto the CHECK(bundle.project_config.opt("flush_multiplier")->values == seed_flush_multiplier); CHECK(bundle.project_config.option("curr_bed_type")->getInt() == seed_bed_type); CHECK(bundle.project_config.opt("wipe_tower_x")->values == std::vector{ 100. }); + CHECK_THAT(bundle.project_config.opt("wipe_tower_rotation_angle")->value, Catch::Matchers::WithinAbs(45., 0.000001)); // e) The published path keeps the user's currently-selected presets: same preset, same size. CHECK(bundle.prints.get_edited_preset().name == pre_load_name); @@ -676,6 +678,9 @@ TEST_CASE("Published 3MF overlays only the allowlisted retraction and z-hop keys PresetBundle bundle; bundle.printers.get_edited_preset().config.opt("retraction_length")->values = { 0.8 }; + // A recognizable non-default value: the skipped mismatch below must leave it untouched + // (asserting the default instead would silently test PrintConfig's retraction_speed). + bundle.printers.get_edited_preset().config.opt("retraction_speed")->values = { 33. }; bundle.printers.get_edited_preset().config.opt_string("machine_start_gcode") = "G28 ; user"; PublishedConfig pub; @@ -683,9 +688,10 @@ TEST_CASE("Published 3MF overlays only the allowlisted retraction and z-hop keys pub.published_keys = { "retraction_length", "retraction_speed", "machine_start_gcode" }; bundle.load_config_model("test.3mf", std::move(config), Semver(), &pub); - // Matching-size retraction vector applied; mismatched vector reported as skipped. + // Matching-size retraction vector applied; mismatched vector reported as skipped and the + // receiver's own value survives. CHECK(bundle.printers.get_edited_preset().config.opt("retraction_length")->values == std::vector{ 1.4 }); - CHECK(bundle.printers.get_edited_preset().config.opt("retraction_speed")->values == std::vector{ 30. }); + CHECK(bundle.printers.get_edited_preset().config.opt("retraction_speed")->values == std::vector{ 33. }); CHECK(contains_key(pub.skipped_keys, "retraction_speed")); // Contract-excluded printer key: silently ignored, absent from skipped_keys. CHECK(bundle.printers.get_edited_preset().config.opt_string("machine_start_gcode") == "G28 ; user");