From 01b340dee16a327760211bb79cb4dc036a0d52bf Mon Sep 17 00:00:00 2001 From: SoftFever Date: Mon, 12 Oct 2026 14:54:17 +0800 Subject: [PATCH] Fix regressions in the AMS filament favorites dropdown --- src/slic3r/GUI/AMSMaterialsSetting.cpp | 126 +++++++++++++------------ src/slic3r/GUI/AMSMaterialsSetting.hpp | 8 +- src/slic3r/GUI/Widgets/ComboBox.cpp | 37 +++----- src/slic3r/GUI/Widgets/ComboBox.hpp | 2 +- src/slic3r/GUI/Widgets/DropDown.cpp | 56 +++++------ src/slic3r/GUI/Widgets/DropDown.hpp | 4 +- 6 files changed, 111 insertions(+), 122 deletions(-) diff --git a/src/slic3r/GUI/AMSMaterialsSetting.cpp b/src/slic3r/GUI/AMSMaterialsSetting.cpp index 671431dd2f..b2b5f46466 100644 --- a/src/slic3r/GUI/AMSMaterialsSetting.cpp +++ b/src/slic3r/GUI/AMSMaterialsSetting.cpp @@ -16,6 +16,7 @@ #include #include "slic3r/GUI/GUI_Utils.hpp" #include "slic3r/GUI/Widgets/Button.hpp" +#include "slic3r/GUI/Widgets/DropDown.hpp" #include "slic3r/GUI/Widgets/Label.hpp" #include #include "slic3r/GUI/wxExtensions.hpp" @@ -175,6 +176,24 @@ void AMSMaterialsSetting::create() m_comboBox_filament->Connect(wxEVT_COMMAND_COMBOBOX_SELECTED, wxCommandEventHandler(AMSMaterialsSetting::on_select_filament), NULL, this); m_comboBox_filament->Bind(EVT_DROPDOWN_ITEM_ACTION, &AMSMaterialsSetting::on_toggle_favorite_filament, this); + // Favorites toggled in the open list are regrouped just before it opens again, so rows never move + // under the cursor. These handlers run ahead of ComboBox's own, which then measure and show the + // rebuilt rows; wxEVT_COMBOBOX_DROPDOWN would be too late, as ComboBox sends it after showing the list. + auto regroup_favorites = [this]() { + if (!m_favorites_regroup || m_comboBox_filament->is_drop_down()) + return; + const FilamentInfos* info = selected_filament_info(); + const std::string filament_id = info ? info->filament_id : std::string(); + rebuild_filament_dropdown_items(filament_id); + }; + m_comboBox_filament->Bind(wxEVT_LEFT_DOWN, [regroup_favorites](wxMouseEvent& e) { regroup_favorites(); e.Skip(); }); + m_comboBox_filament->Bind(wxEVT_LEFT_DCLICK, [regroup_favorites](wxMouseEvent& e) { regroup_favorites(); e.Skip(); }); + m_comboBox_filament->Bind(wxEVT_KEY_DOWN, [regroup_favorites](wxKeyEvent& e) { + if (e.GetKeyCode() == WXK_RETURN || e.GetKeyCode() == WXK_SPACE) // the keys that open the list + regroup_favorites(); + e.Skip(); + }); + m_comboBox_cali_result->Connect(wxEVT_COMMAND_COMBOBOX_SELECTED, wxCommandEventHandler(AMSMaterialsSetting::on_select_cali_result), NULL, this); } @@ -193,6 +212,8 @@ void AMSMaterialsSetting::create_panel_normal(wxWindow* parent) m_sizer_filament->Add(0, 0, 0, wxEXPAND, 0); m_comboBox_filament = new ::ComboBox(parent, wxID_ANY, wxEmptyString, wxDefaultPosition, AMS_MATERIALS_SETTING_COMBOX_WIDTH, 0, nullptr, wxCB_READONLY); + m_star_filled = ScalableBitmap(this, "score_star_light", 16); + m_star_empty = ScalableBitmap(this, "score_star_dark", 16); m_sizer_filament->Add(m_comboBox_filament, 1, wxALIGN_CENTER, 0); @@ -988,12 +1009,10 @@ static const char* AMS_FAVORITE_FILAMENT_IDS_KEY = "ams_favorite_filament_ids"; const AMSMaterialsSetting::FilamentInfos* AMSMaterialsSetting::selected_filament_info() const { - int selection = m_comboBox_filament->GetSelection(); - if (selection >= 0 && selection < int(m_filament_list_items.size()) && m_filament_list_items[selection].is_filament) - return &m_filament_list_items[selection].filament; - - auto iter = map_filament_items.find(m_comboBox_filament->GetValue().ToStdString()); - return iter == map_filament_items.end() ? nullptr : &iter->second; + const int selection = m_comboBox_filament->GetSelection(); + if (selection < 0 || selection >= int(m_filament_list_items.size()) || !m_filament_list_items[selection].is_filament) + return nullptr; + return &m_filament_list_items[selection].filament; } std::set AMSMaterialsSetting::load_favorite_filament_ids() const @@ -1019,10 +1038,6 @@ void AMSMaterialsSetting::rebuild_filament_dropdown_items(const std::string& sel std::vector items; m_filament_list_items.clear(); - const std::set favorite_ids = load_favorite_filament_ids(); - wxBitmap star_filled = ScalableBitmap(this, "score_star_light", 16).bmp(); - wxBitmap star_empty = ScalableBitmap(this, "score_star_dark", 16).bmp(); - auto append_header = [&items, this](const wxString& label) { DropDown::Item item; item.text = label; @@ -1031,43 +1046,32 @@ void AMSMaterialsSetting::rebuild_filament_dropdown_items(const std::string& sel m_filament_list_items.push_back(FilamentListItem{}); }; - auto append_filament = [&items, this, &favorite_ids, &star_filled, &star_empty](const wxString& name) { - auto info_it = map_filament_items.find(name.ToStdString()); + auto append_filament = [&items, this](const wxString& name) { + auto info_it = map_filament_items.find(name); if (info_it == map_filament_items.end()) return; - const bool is_favorite = favorite_ids.find(info_it->second.filament_id) != favorite_ids.end(); DropDown::Item item; item.text = name; - item.action_icon = is_favorite ? star_filled : star_empty; - item.style = 0; items.push_back(item); - - FilamentListItem list_item; - list_item.filament = info_it->second; - list_item.is_filament = true; - m_filament_list_items.push_back(list_item); + m_filament_list_items.push_back(FilamentListItem{info_it->second, true}); }; - bool has_favorites = false; + // Favorites are listed again at the top under their own header. Without any, the list is the + // plain filament list. + const std::set favorite_ids = load_favorite_filament_ids(); + std::vector favorites; for (const wxString& name : m_sorted_filament_items) { - auto info_it = map_filament_items.find(name.ToStdString()); - if (info_it != map_filament_items.end() && favorite_ids.find(info_it->second.filament_id) != favorite_ids.end()) { - has_favorites = true; - break; - } + auto info_it = map_filament_items.find(name); + if (info_it != map_filament_items.end() && favorite_ids.count(info_it->second.filament_id) > 0) + favorites.push_back(name); } - - if (has_favorites) { + if (!favorites.empty()) { append_header(_L("Favorite filaments")); - for (const wxString& name : m_sorted_filament_items) { - auto info_it = map_filament_items.find(name.ToStdString()); - if (info_it != map_filament_items.end() && favorite_ids.find(info_it->second.filament_id) != favorite_ids.end()) - append_filament(name); - } + for (const wxString& name : favorites) + append_filament(name); + append_header(_L("All filaments")); } - - append_header(_L("All filaments")); for (const wxString& name : m_sorted_filament_items) append_filament(name); @@ -1081,8 +1085,25 @@ void AMSMaterialsSetting::rebuild_filament_dropdown_items(const std::string& sel } } + // Clear() first: SetItems() keeps the old selection, and SetSelection() ignores an index equal to it, + // which would leave the previous filament's name on the combo. + m_comboBox_filament->Clear(); m_comboBox_filament->SetItems(items); + update_favorite_stars(); m_comboBox_filament->SetSelection(selection_idx); + m_favorites_regroup = false; +} + +void AMSMaterialsSetting::update_favorite_stars() +{ + const std::set favorite_ids = load_favorite_filament_ids(); + for (size_t i = 0; i < m_filament_list_items.size(); ++i) { + const FilamentListItem& row = m_filament_list_items[i]; + if (row.is_filament) { + const bool favorite = favorite_ids.count(row.filament.filament_id) > 0; + m_comboBox_filament->SetItemActionBitmap(i, favorite ? m_star_filled.bmp() : m_star_empty.bmp()); + } + } } void AMSMaterialsSetting::toggle_favorite_filament(const std::string& filament_id) @@ -1106,12 +1127,9 @@ void AMSMaterialsSetting::on_toggle_favorite_filament(wxCommandEvent& evt) if (index < 0 || index >= int(m_filament_list_items.size()) || !m_filament_list_items[index].is_filament) return; - std::string selected_filament_id; - if (const FilamentInfos* info = selected_filament_info()) - selected_filament_id = info->filament_id; toggle_favorite_filament(m_filament_list_items[index].filament.filament_id); - rebuild_filament_dropdown_items(selected_filament_id); - m_comboBox_filament->RefreshDropdownIfOpen(); + update_favorite_stars(); + m_favorites_regroup = true; } void AMSMaterialsSetting::Popup(wxString filament, wxString sn, wxString temp_min, wxString temp_max, wxString k, wxString n) @@ -1127,16 +1145,12 @@ void AMSMaterialsSetting::Popup(wxString filament, wxString sn, wxString temp_mi m_input_k_val->GetTextCtrl()->SetValue(k); m_input_n_val->GetTextCtrl()->SetValue(n); - wxArrayString filament_items; - map_filament_items.clear(); - m_sorted_filament_items.clear(); - m_filament_list_items.clear(); wxString bambu_filament_name; - wxString hint_filament_name; // the hint type to be selected std::unordered_map query_filament_vendors;// some information for sort std::unordered_map query_filament_types; // std::set filament_id_set; + m_sorted_filament_items.clear(); // The alias keyed map has to start empty: it is a member, so a stale alias left by an earlier // popup (a different printer, a different nozzle) would resolve to that printer's filament_id. map_filament_items.clear(); @@ -1166,7 +1180,7 @@ void AMSMaterialsSetting::Popup(wxString filament, wxString sn, wxString temp_mi if (alias.empty()) continue; - filament_items.push_back(alias); + m_sorted_filament_items.push_back(alias); _collect_filament_info(alias, *filament_it, query_filament_vendors, query_filament_types); FilamentInfos filament_infos; @@ -1175,7 +1189,6 @@ void AMSMaterialsSetting::Popup(wxString filament, wxString sn, wxString temp_mi map_filament_items[alias] = filament_infos; if (filament_it->filament_id == ams_filament_id) { - hint_filament_name = from_u8(alias); bambu_filament_name = from_u8(alias); @@ -1346,20 +1359,10 @@ void AMSMaterialsSetting::Popup(wxString filament, wxString sn, wxString temp_mi return name_compare != 0 ? name_compare < 0 : left < right; }; - std::sort(filament_items.begin(), filament_items.end(), _filament_sorter); + std::sort(m_sorted_filament_items.begin(), m_sorted_filament_items.end(), _filament_sorter); } - std::string selection_filament_id; - for (int i = 0; i < int(filament_items.size()); ++i) { - m_sorted_filament_items.push_back(filament_items[i]); - if (hint_filament_name == filament_items[i]) { - auto info_it = map_filament_items.find(filament_items[i].ToStdString()); - if (info_it != map_filament_items.end()) - selection_filament_id = info_it->second.filament_id; - } - } - - rebuild_filament_dropdown_items(selection_filament_id); + rebuild_filament_dropdown_items(ams_filament_id); int selection_idx = m_comboBox_filament->GetSelection(); post_select_event(selection_idx); @@ -1645,6 +1648,11 @@ void AMSMaterialsSetting::on_dpi_changed(const wxRect &suggested_rect) m_button_reset->Rescale(); // ORCA m_button_confirm->Rescale(); // ORCA m_button_close->Rescale(); // ORCA + // An open list would keep the measurements of the old stars, so close it before replacing them. + m_comboBox_filament->CloseDropdown(); + m_star_filled.msw_rescale(); + m_star_empty.msw_rescale(); + update_favorite_stars(); this->Refresh(); } diff --git a/src/slic3r/GUI/AMSMaterialsSetting.hpp b/src/slic3r/GUI/AMSMaterialsSetting.hpp index 673ead26ea..6fa215063a 100644 --- a/src/slic3r/GUI/AMSMaterialsSetting.hpp +++ b/src/slic3r/GUI/AMSMaterialsSetting.hpp @@ -166,6 +166,7 @@ protected: bool is_virtual_tray(); void update_widgets(); void rebuild_filament_dropdown_items(const std::string& selected_filament_id); + void update_favorite_stars(); std::set load_favorite_filament_ids() const; void save_favorite_filament_ids(const std::set& favorite_ids); void toggle_favorite_filament(const std::string& filament_id); @@ -222,9 +223,14 @@ protected: }; const FilamentInfos* selected_filament_info() const; - std::map map_filament_items; + // Keyed by the same wxString the dropdown shows, so a row's name never has to be converted back. + std::map map_filament_items; std::vector m_sorted_filament_items; std::vector m_filament_list_items; + ScalableBitmap m_star_filled; + ScalableBitmap m_star_empty; + // Favorites changed while the list was open; their rows are regrouped before it opens again. + bool m_favorites_regroup{false}; }; wxDECLARE_EVENT(EVT_SELECTED_COLOR, wxCommandEvent); diff --git a/src/slic3r/GUI/Widgets/ComboBox.cpp b/src/slic3r/GUI/Widgets/ComboBox.cpp index 500b8c364a..55e261d04d 100644 --- a/src/slic3r/GUI/Widgets/ComboBox.cpp +++ b/src/slic3r/GUI/Widgets/ComboBox.cpp @@ -302,25 +302,10 @@ void ComboBox::ForceDropdownOpen() } } -void ComboBox::RefreshDropdownIfOpen() +void ComboBox::CloseDropdown() { - if (!drop_down) - return; - - drop.hover_item = -1; - drop.offset = wxPoint(); - if (drop.subDropDown) { - drop.subDropDown->group.clear(); - drop.subDropDown->hover_item = -1; - drop.subDropDown->offset = wxPoint(); - if (drop.subDropDown->IsShown()) - drop.subDropDown->Dismiss(); - } - - drop.need_sync = true; - drop.messureSize(); - drop.autoPosition(); - drop.paintNow(); + if (drop.IsShown()) + drop.DismissAndNotify(); } unsigned int ComboBox::GetCount() const { return items.size(); } @@ -452,16 +437,18 @@ void ComboBox::keyDown(wxKeyEvent& event) case WXK_UP: case WXK_DOWN: case WXK_LEFT: - case WXK_RIGHT: - if ((event.GetKeyCode() == WXK_UP || event.GetKeyCode() == WXK_LEFT) && GetSelection() > 0) { - SetSelection(GetSelection() - 1); - } else if ((event.GetKeyCode() == WXK_DOWN || event.GetKeyCode() == WXK_RIGHT) && GetSelection() + 1 < items.size()) { - SetSelection(GetSelection() + 1); - } else { + case WXK_RIGHT: { + // Step over disabled rows: the mouse cannot select them either. + const int step = (event.GetKeyCode() == WXK_UP || event.GetKeyCode() == WXK_LEFT) ? -1 : 1; + int n = GetSelection() + step; + while (n >= 0 && n < int(items.size()) && (items[n].style & DD_ITEM_STYLE_DISABLED)) + n += step; + if (n < 0 || n >= int(items.size())) break; - } + SetSelection(n); sendComboBoxEvent(); break; + } case WXK_TAB: HandleAsNavigationKey(event); break; diff --git a/src/slic3r/GUI/Widgets/ComboBox.hpp b/src/slic3r/GUI/Widgets/ComboBox.hpp index 1ab04c3a79..1fbc2e257d 100644 --- a/src/slic3r/GUI/Widgets/ComboBox.hpp +++ b/src/slic3r/GUI/Widgets/ComboBox.hpp @@ -101,7 +101,7 @@ public: void DeleteOneItem(unsigned int pos) { DoDeleteOneItem(pos); } void ForceDropdownOpen(); - void RefreshDropdownIfOpen(); + void CloseDropdown(); protected: virtual int DoInsertItems(const wxArrayStringsAdapter &items, diff --git a/src/slic3r/GUI/Widgets/DropDown.cpp b/src/slic3r/GUI/Widgets/DropDown.cpp index 3fb1607551..966a090526 100644 --- a/src/slic3r/GUI/Widgets/DropDown.cpp +++ b/src/slic3r/GUI/Widgets/DropDown.cpp @@ -243,27 +243,17 @@ static wxSize GetBmpSize(wxBitmap & bmp) #endif } -wxSize DropDown::actionIconSize() const +// The x-range [first, second) of the column at the right of the rows that holds the items' action +// icons, empty when no item has one. render() draws the icons in it and mouseReleased() hit-tests +// clicks against it, so the drawn icon and its clickable area always match. +std::pair DropDown::actionColumn() const { - wxSize size; - std::set groups; - for (const Item& item : items) { - if (group.IsEmpty()) { - if (!item.group_key.IsEmpty()) { - if (groups.find(item.group_key) != groups.end()) - continue; - groups.insert(item.group_key); - } - } else if (item.group_key != group) { - continue; - } - - wxBitmap icon = item.action_icon; - wxSize icon_size = GetBmpSize(icon); - size.x = std::max(size.x, icon_size.x); - size.y = std::max(size.y, icon_size.y); - } - return size; + if (actionIconWidth <= 0) + return {0, 0}; + int right = rowSize.x; + if (rowSize.y * count > GetSize().y) // position bar + right -= 6; + return {right - actionIconWidth - FromDIP(16), right}; } static void _DrawSplitItem(const wxWindow* w, wxDC& dc, wxString split_text, wxPoint start_pt, int item_width, int item_height) @@ -395,8 +385,9 @@ void DropDown::render(wxDC &dc) rcContent.width -= szBmp.x + 5; } - const wxSize action_icon_size = actionIconSize(); - const int action_column_width = action_icon_size.x > 0 ? action_icon_size.x + FromDIP(16) : 0; + const auto action_column = actionColumn(); + if (action_column.second > action_column.first) + rcContent.width = action_column.first - rcContent.x; std::set groups; // draw texts & icons @@ -436,7 +427,6 @@ void DropDown::render(wxDC &dc) } if (rcContent.y > size.y) break; wxPoint pt = rcContent.GetLeftTop(); - int rcContent_width = rcContent.width; if (item.style & DD_ITEM_STYLE_SPLIT_ITEM) { _DrawSplitItem(this, dc, item.text, pt, rowSize.GetWidth(), rowSize.GetHeight()); @@ -462,12 +452,9 @@ void DropDown::render(wxDC &dc) auto text = group.IsEmpty() ? (item.group_key.IsEmpty() ? item.text : item.group_label) : item.text; - if (action_column_width > 0) - rcContent.width -= action_column_width; - if (item.action_icon.IsOk()) { auto action_size = GetBmpSize(item.action_icon); - wxPoint action_pt(rcContent.x + rcContent_width - action_size.x - FromDIP(8), + wxPoint action_pt(action_column.second - action_size.x - FromDIP(8), rcContent.y + (rcContent.height - action_size.y) / 2); dc.DrawBitmap(item.action_icon, action_pt); } @@ -498,7 +485,6 @@ void DropDown::render(wxDC &dc) dc.DrawBitmap(arrow_bitmap.bmp(), pt); } } - rcContent.width = rcContent_width; rcContent.y += rowSize.y; } } @@ -572,6 +558,7 @@ void DropDown::messureSize() if (!need_sync) return; textSize = wxSize(); iconSize = wxSize(); + actionIconWidth = 0; count = 0; wxClientDC dc(GetParent() ? GetParent() : this); dc.SetFont(GetFont()); @@ -609,9 +596,12 @@ void DropDown::messureSize() } } if (item.action_icon.IsOk()) - size1.x += GetBmpSize(item.action_icon).x + FromDIP(16); + actionIconWidth = std::max(actionIconWidth, GetBmpSize(item.action_icon).x); if (size1.x > textSize.x) textSize = size1; } + // Every row leaves room for the action column, as render() and actionColumn() lay it out. + if (actionIconWidth > 0) + textSize.x += actionIconWidth + FromDIP(16); if (!align_icon) iconSize.x = 0; wxSize szContent = textSize; if (szContent.x < FromDIP(120) && !use_content_width) @@ -766,12 +756,8 @@ void DropDown::mouseReleased(wxMouseEvent& event) if (hover_item >= 0 && (subDropDown == nullptr || subDropDown->group.empty())) { // not moved int index = hoverIndex(); if (index >= 0 && items[index].action_icon.IsOk()) { - wxSize action_size = actionIconSize(); - int action_right = GetSize().x; - if (rowSize.y * count > GetSize().y) - action_right -= 6; - int action_left = action_right - action_size.x - FromDIP(16); - if (event.GetX() >= action_left && event.GetX() <= action_right) { + const auto action_column = actionColumn(); + if (event.GetX() >= action_column.first && event.GetX() < action_column.second) { sendDropDownActionEvent(index); paintNow(); return; diff --git a/src/slic3r/GUI/Widgets/DropDown.hpp b/src/slic3r/GUI/Widgets/DropDown.hpp index adbe0eb95c..a0821e6fff 100644 --- a/src/slic3r/GUI/Widgets/DropDown.hpp +++ b/src/slic3r/GUI/Widgets/DropDown.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include "slic3r/GUI/Widgets/StateColor.hpp" #include @@ -67,6 +68,7 @@ private: wxSize textSize; wxSize iconSize; + int actionIconWidth = 0; wxSize rowSize; StateHandler state_handler; @@ -137,7 +139,7 @@ private: int hoverIndex(); int selectedItem(); - wxSize actionIconSize() const; + std::pair actionColumn() const; friend class ComboBox; void messureSize();