From 845424a4779302437bc5388f94942cb2328f48ee Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 17 Sep 2026 18:19:29 +0000 Subject: [PATCH] fix(gui): show indexed coFloatsOrPercents in unsaved changes (Orca #15472 adapt) Port OrcaSlicer #15472 onto Edge without adding ConfigValueFormatter. Index coFloatsOrPercents in Search, add a 3-arg get_option with a safe miss (empty Option, variant_index=-2), and format absolute vs percent inline in UnsavedChangesDialog::get_string_value. Keep Edge #21 crash guards so copy/unsaved-changes still shows keys absent from the search index. deep_diff/apply_only gain per-slot coFloatsOrPercents transfer so Catch2 can assert 50 vs 50% per variant. Cereal is untouched. Co-authored-by: aceRage --- src/libslic3r/Config.cpp | 19 ++++++ src/libslic3r/Preset.cpp | 1 + src/slic3r/GUI/Search.cpp | 51 +++++++++++++++- src/slic3r/GUI/Search.hpp | 3 + src/slic3r/GUI/UnsavedChangesDialog.cpp | 79 +++++++++++++++++++------ tests/libslic3r/CMakeLists.txt | 1 + tests/libslic3r/test_preset_diff.cpp | 26 ++++++++ 7 files changed, 160 insertions(+), 20 deletions(-) create mode 100644 tests/libslic3r/test_preset_diff.cpp diff --git a/src/libslic3r/Config.cpp b/src/libslic3r/Config.cpp index ce1bc0724ece..5d6629d7f69c 100644 --- a/src/libslic3r/Config.cpp +++ b/src/libslic3r/Config.cpp @@ -5,6 +5,7 @@ #include "Preset.hpp" #include +#include #include #include #include @@ -457,6 +458,24 @@ void ConfigBase::apply_only(const ConfigBase &other, const t_config_option_keys if (my_opt == nullptr) { // opt_key does not exist in this ConfigBase and it cannot be created, because it is not defined by this->def(). // This is only possible if other is of DynamicConfig type. + // Orca #15472 / #13712: dirty_options may name a single vector slot as "key#index". + if (auto n = opt_key.find('#'); n != std::string::npos) { + auto opt_key2 = opt_key.substr(0, n); + auto my_opt2 = dynamic_cast(this->option(opt_key2)); + auto other_opt = other.option(opt_key2); + if (my_opt2 == nullptr && other_opt) { + my_opt2 = dynamic_cast(this->option(opt_key2, true)); + if (my_opt2 && my_opt2->empty()) { + my_opt2->resize(1, other_opt); + } + } + if (my_opt2) { + int index = std::atoi(opt_key.c_str() + n + 1); + if (other_opt) + my_opt2->set_at(other_opt, index, index); + continue; + } + } if (ignore_nonexistent) continue; throw UnknownOptionException(opt_key); diff --git a/src/libslic3r/Preset.cpp b/src/libslic3r/Preset.cpp index 0bae4b68e26b..5a6624b45f9c 100644 --- a/src/libslic3r/Preset.cpp +++ b/src/libslic3r/Preset.cpp @@ -3031,6 +3031,7 @@ inline t_config_option_keys deep_diff(const ConfigBase &config_this, const Confi case coFloats: add_correct_opts_to_diff(opt_key, diff, config_other, config_this); break; case coStrings: add_correct_opts_to_diff(opt_key, diff, config_other, config_this); break; case coPercents:add_correct_opts_to_diff(opt_key, diff, config_other, config_this); break; + case coFloatsOrPercents: add_correct_opts_to_diff(opt_key, diff, config_other, config_this); break; case coPoints: add_correct_opts_to_diff(opt_key, diff, config_other, config_this); break; // BBS case coEnums: add_correct_opts_to_diff(opt_key, diff, config_other, config_this); break; diff --git a/src/slic3r/GUI/Search.cpp b/src/slic3r/GUI/Search.cpp index 9906a2ea3bd9..7797f2075b95 100644 --- a/src/slic3r/GUI/Search.cpp +++ b/src/slic3r/GUI/Search.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -47,7 +48,7 @@ static char marker_by_type(Preset::Type type, PrinterTechnology pt) } } -std::string Option::opt_key() const { return into_u8(key).substr(2); } +std::string Option::opt_key() const { return key.size() < 2 ? std::string() : into_u8(key).substr(2); } void FoundOption::get_marked_label_and_tooltip(const char **label_, const char **tooltip_) const { @@ -101,12 +102,18 @@ void OptionsSearcher::append_options(DynamicPrintConfig *config, Preset::Type ty case coFloats: change_opt_key(opt_key, config, cnt); break; case coStrings: change_opt_key(opt_key, config, cnt); break; case coPercents: change_opt_key(opt_key, config, cnt); break; + case coFloatsOrPercents: change_opt_key>(opt_key, config, cnt); break; case coPoints: change_opt_key(opt_key, config, cnt); break; // BBS case coEnums: change_opt_key(opt_key, config, cnt); break; default: break; } + // Orca #15472: index print-level coFloatsOrPercents (e.g. per-variant speeds) without + // expanding Edge's search index to every print vector type (Orca does that via #13712). + if (type == Preset::TYPE_PRINT && config->option(opt_key)->type() == coFloatsOrPercents) + change_opt_key>(opt_key, config, cnt); + wxString label = opt.full_label.empty() ? opt.label : opt.full_label; std::string key = get_key(opt_key, type); @@ -311,6 +318,48 @@ const Option &OptionsSearcher::get_option(const std::string &opt_key, Preset::Ty return options[it - options.begin()]; } +const Option &OptionsSearcher::get_option(const std::string &opt_key, Preset::Type type, int &variant_index) const +{ + auto not_found = [&variant_index]() -> const Option & { + static const Option empty_option; + variant_index = -2; + return empty_option; + }; + + variant_index = -1; + std::string opt_key2 = opt_key; + if (auto n = opt_key.find('#'); n != std::string::npos) { + variant_index = std::atoi(opt_key.c_str() + n + 1); + opt_key2 = opt_key.substr(0, n); + } + + const std::wstring key = boost::nowide::widen(get_key(opt_key2, type)); + auto it = std::lower_bound(options.begin(), options.end(), Option({key})); + if (it == options.end()) + return not_found(); + + if (it->key == key) { + variant_index = -1; + } else { + const std::wstring prefix = key + L"#"; + it = std::lower_bound(it, options.end(), Option({prefix})); + if (it == options.end() || it->key.compare(0, prefix.length(), prefix) != 0) + return not_found(); + // Orca: Copy-parameters / unsaved-changes may request the base key without a vector index. + if (variant_index < 0) + return *it; + + // Edge has no print/printer/filament *_options_with_variant sets (Orca #13712). + // Look up the exact indexed key and keep variant_index for extruder labelling. + const std::wstring indexed_key = boost::nowide::widen(get_key(opt_key, type)); + it = std::lower_bound(it, options.end(), Option({indexed_key})); + if (it == options.end() || it->key != indexed_key) + return not_found(); + } + + return options[it - options.begin()]; +} + static Option create_option(const std::string &opt_key, const wxString &label, Preset::Type type, const GroupAndCategory &gc) { wxString suffix; diff --git a/src/slic3r/GUI/Search.hpp b/src/slic3r/GUI/Search.hpp index b827f5c3eb10..3bdccdb0fa04 100644 --- a/src/slic3r/GUI/Search.hpp +++ b/src/slic3r/GUI/Search.hpp @@ -133,6 +133,9 @@ class OptionsSearcher const FoundOption &operator[](const size_t pos) const noexcept { return found[pos]; } const Option & get_option(size_t pos_in_filter) const; const Option & get_option(const std::string &opt_key, Preset::Type type) const; + // Orca #15472: variant_index is -1 for a scalar, >=0 for an indexed vector entry, + // and -2 when the key is absent from the search index (empty Option, no options[0] fallback). + const Option & get_option(const std::string &opt_key, Preset::Type type, int &variant_index) const; Option get_option(const std::string &opt_key, const wxString &label, Preset::Type type) const; const std::vector &found_options() { return found; } diff --git a/src/slic3r/GUI/UnsavedChangesDialog.cpp b/src/slic3r/GUI/UnsavedChangesDialog.cpp index d2ff4990048c..e33602b385fa 100644 --- a/src/slic3r/GUI/UnsavedChangesDialog.cpp +++ b/src/slic3r/GUI/UnsavedChangesDialog.cpp @@ -1347,6 +1347,19 @@ static wxString get_string_value(std::string opt_key, const DynamicPrintConfig& out = double_to_string(opt->value) + (opt->percent ? "%" : ""); return out; } + case coFloatsOrPercents: { + const auto *values = config.opt(opt_key); + // Orca #15472: Preset comparison may request the entire vector instead of an indexed entry. + if (!values) + return _L("Undef"); + if (orig_opt_idx < 0) + return from_u8(values->serialize()); + if (opt_idx < (int) values->size()) { + const FloatOrPercent &value = values->get_at(opt_idx); + return double_to_string(value.value) + (value.percent ? "%" : ""); + } + return _L("Undef"); + } case coEnum: { return get_string_from_enum(opt_key, config, opt_key == "top_surface_pattern" || @@ -1672,6 +1685,9 @@ void UnsavedChangesDialog::update_tree(Preset::Type type, PresetCollection* pres else presets_list.emplace_back(presets_); + const auto *nozzle_diameter = wxGetApp().preset_bundle->printers.get_edited_preset().config.option("nozzle_diameter"); + const bool multiple_extruders = nozzle_diameter && nozzle_diameter->values.size() > 1; + // Display a dialog showing the dirty options in a human readable form. for (PresetCollection* presets : presets_list) { @@ -1705,32 +1721,57 @@ void UnsavedChangesDialog::update_tree(Preset::Type type, PresetCollection* pres } } + auto variant_key = Preset::get_iot_type_string(type) + "_extruder_variant"; + auto id_key = Preset::get_iot_type_string(type) + "_extruder_id"; + // Orca: Dirty indices belong to the edited config, which may contain newly added variants. + auto extruder_variant = dynamic_cast(new_config.option(variant_key)); + auto extruder_id = dynamic_cast(new_config.option(id_key)); + for (const std::string& opt_key : dirty_options) { - const Search::Option& option = searcher.get_option(opt_key, type); - if (option.opt_key() != opt_key) { - // Only show the fallback for user-facing option types - // (bool/float/int/enum). Internal keys like IDs and - // serialized blobs are coString — skip those silently. - const ConfigOption* o = old_config.option(opt_key); + int variant_index = -2; + Search::Option option = searcher.get_option(opt_key, type, variant_index); + if (variant_index == -2) { + // Edge #21 / Orca #15472: keep user-facing settings visible when they are + // absent from the search index. Do not fall back to options[0]. + const std::string pure_key = get_pure_opt_key(opt_key); + const ConfigOption *o = old_config.option(pure_key); + if (!o) o = new_config.option(pure_key); + if (!o) o = old_config.option(opt_key); if (!o) o = new_config.option(opt_key); if (!o || o->type() == coString || o->type() == coStrings) continue; - wxString label = from_u8(opt_key); - if (old_config.def()) { - const ConfigOptionDef* def = old_config.def()->get(opt_key); - if (def && !def->label.empty()) - label = def->label; + const ConfigOptionDef *def = print_config_def.get(pure_key); + if (!def && old_config.def()) + def = old_config.def()->get(pure_key); + const std::string def_label = def ? (def->full_label.empty() ? def->label : def->full_label) : std::string(); + option.label_local = (def_label.empty() ? from_u8(opt_key) : _L(def_label)).ToStdWstring(); + option.category_local = (def && !def->category.empty() ? + Tab::translate_category(from_u8(def->category), type) : _L("Other")).ToStdWstring(); + } + + wxString category = option.category_local; + wxString label = option.label_local; + if (type == Preset::TYPE_PRINTER && variant_index >= 0 && option.category == L"Machine limits") { + // Orca: silent_mode is obsolete on import, but its option and two-column UI still exist. + // Keep mode labels for configs that explicitly enable it; omit them in the default single-mode UI. + if (new_config.option("silent_mode") && new_config.opt_bool("silent_mode")) + label += " (" + (variant_index % 2 == 0 ? _L("Normal") : _L("Silent")) + ")"; + } + if (variant_index >= 0 && extruder_variant && variant_index < (int) extruder_variant->size()) { + // Orca: Match the untranslated category and use the same extruder names as the printer tabs. + if (option.category.compare(0, 9, L"Extruder ") == 0) + category = _L("Extruder"); + wxString variant_label = from_u8(extruder_variant->values[variant_index]); + // Orca: An extruder name only disambiguates variants on printers with multiple extruders. + if (multiple_extruders && extruder_id && variant_index < (int) extruder_id->size() && extruder_id->values[variant_index] > 0) { + const wxString extruder_name = Tab::translate_category( + wxString::Format("Extruder %d", extruder_id->values[variant_index]), Preset::TYPE_PRINTER); + variant_label = extruder_name + " (" + variant_label + ")"; } - PresetItem pi = {type, opt_key, - _L("Other"), wxEmptyString, - label, - get_string_value(opt_key, old_config), - get_string_value(opt_key, new_config)}; - m_presetitems.push_back(pi); - continue; + category = variant_label + ": " + category; } - PresetItem pi = {type, opt_key, option.category_local, option.group_local, option.label_local, get_string_value(opt_key, old_config), get_string_value(opt_key, new_config)}; + PresetItem pi = {type, opt_key, category, option.group_local, label, get_string_value(opt_key, old_config), get_string_value(opt_key, new_config)}; m_presetitems.push_back(pi); } diff --git a/tests/libslic3r/CMakeLists.txt b/tests/libslic3r/CMakeLists.txt index d8a5b7f10cff..112d2b3436ba 100644 --- a/tests/libslic3r/CMakeLists.txt +++ b/tests/libslic3r/CMakeLists.txt @@ -40,6 +40,7 @@ add_executable(${_TEST_NAME}_tests test_local_z_order_optimizer.cpp test_placeholder_parser.cpp test_polygon.cpp + test_preset_diff.cpp test_profile_load_util.cpp test_project_preset_switch.cpp test_mutable_polygon.cpp diff --git a/tests/libslic3r/test_preset_diff.cpp b/tests/libslic3r/test_preset_diff.cpp new file mode 100644 index 000000000000..2e53b6eb5fe0 --- /dev/null +++ b/tests/libslic3r/test_preset_diff.cpp @@ -0,0 +1,26 @@ +#include + +#include "libslic3r/Preset.hpp" +#include "libslic3r/PrintConfig.hpp" + +#include +#include + +using namespace Slic3r; + +TEST_CASE("deep_diff distinguishes absolute and percentage speeds for each variant", "[PresetDiff][Config]") +{ + const size_t changed_index = GENERATE(size_t(0), size_t(1)); + Preset reference(Preset::TYPE_PRINT, "ref"); + reference.config.set_key_value("small_perimeter_speed", new ConfigOptionFloatsOrPercents{{50., false}, {50., false}}); + + Preset edited = reference; + edited.config.option("small_perimeter_speed")->values[changed_index].percent = true; + + const auto diff = PresetCollection::dirty_options(&edited, &reference, /*deep_compare=*/true); + REQUIRE(diff == std::vector{"small_perimeter_speed#" + std::to_string(changed_index)}); + + DynamicPrintConfig transferred = reference.config; + transferred.apply_only(edited.config, diff); + REQUIRE(*transferred.option("small_perimeter_speed") == *edited.config.option("small_perimeter_speed")); +}