From 122d7c85fc0cc5dfeb32c6405f05dc3f386eea97 Mon Sep 17 00:00:00 2001 From: heffneil Date: Sun, 26 Apr 2026 11:48:53 -0400 Subject: [PATCH 1/5] Add filter to Edit Display Elements Available list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Large shows can have hundreds of models/groups/timings; finding one to add to a view via scroll-and-eye is tedious. Add a wxSearchCtrl above the Available list (left side of Edit Display Elements) using the same pattern as the Controllers tab visualizer filter and the ControllerModelDialog model filter — descriptive 'Filter models...' text, cancel button, case-insensitive substring match against the model/timing/group name. The filter clears automatically when items are moved into the view (Add Selected, Add All, drag-to-right, key-right). After moving, the user almost always wants to see the full list again to pick the next thing to add; if they need the same filter they can retype. Implementation notes: - All UI/state lives outside the wxSmith block, so the .wxs file does not need to change. - ListCtrlNonModels was at GBPosition(1,0) span(3,1); we detach it, drop the search ctrl at (1,0), and re-attach the list at (2,0) span(2,1). The growable row stays row 3, so the list still fills. - The filter is enforced in AddModelToNotList / AddTimingToNotList (both gateways into the visible list), so PopulateModels rebuilds with the filter applied without any other code path needing to know about the filter. --- README.txt | 3 ++ src-ui-wx/layout/ViewsModelsPanel.cpp | 58 ++++++++++++++++++++++++++- src-ui-wx/layout/ViewsModelsPanel.h | 11 +++++ 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/README.txt b/README.txt index a3edce917b..836ea2835c 100644 --- a/README.txt +++ b/README.txt @@ -86,6 +86,9 @@ XLIGHTS/NUTCRACKER RELEASE NOTES -enh (dkulp) Shader effect: dynamic uniforms emit JSON matching the effect-panel schema; JsonEffectPanel gains a reusable point2d control type, so iPad and desktop build the dynamic rows from the same description. + -enh (Neil) Edit Display Elements: add a filter box above the Available list so large shows can + quickly find a model/timing/group to add. Filter clears automatically when items are + moved into the view. -bug (dkulp) Replaced throwing std::stoi calls in OutputManager / xxxSerialOutput / HinksPix with non-throwing std::strtol so corrupt config or controller responses no longer crash. -bug (dkulp) State / Faces / Shockwave / VUMeter / Lyric / MatrixModel: added div-by-zero and diff --git a/src-ui-wx/layout/ViewsModelsPanel.cpp b/src-ui-wx/layout/ViewsModelsPanel.cpp index c3122b1002..26eae34fd1 100644 --- a/src-ui-wx/layout/ViewsModelsPanel.cpp +++ b/src-ui-wx/layout/ViewsModelsPanel.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include "model-16.xpm" #include "timing-16.xpm" @@ -252,7 +253,25 @@ ViewsModelsPanel::ViewsModelsPanel(xLightsFrame *frame, wxWindow* parent, wxWind _gridBagSizer = GridBagSizer1; _viewButtonsSizer = FlexGridSizer8; - + // Insert a filter control above the "Available" (non-models) list. + // The wxSmith block put ListCtrlNonModels at (1, 0) spanning rows 1-3. + // Detach it, drop the filter into row 1, and re-attach the list to + // rows 2-3 so the bottom row stays the growable one. + GridBagSizer1->Detach(ListCtrlNonModels); + TextCtrl_NonModelsFilter = new wxSearchCtrl(this, wxID_ANY, wxEmptyString, + wxDefaultPosition, wxDefaultSize, + wxTE_PROCESS_ENTER); + TextCtrl_NonModelsFilter->SetDescriptiveText(_("Filter models...")); + TextCtrl_NonModelsFilter->ShowCancelButton(true); + GridBagSizer1->Add(TextCtrl_NonModelsFilter, wxGBPosition(1, 0), wxDefaultSpan, + wxALL | wxEXPAND, 2); + GridBagSizer1->Add(ListCtrlNonModels, wxGBPosition(2, 0), wxGBSpan(2, 1), + wxALL | wxEXPAND, 2); + TextCtrl_NonModelsFilter->Bind(wxEVT_TEXT, + &ViewsModelsPanel::OnNonModelsFilterText, this); + TextCtrl_NonModelsFilter->Bind(wxEVT_SEARCHCTRL_CANCEL_BTN, + &ViewsModelsPanel::OnNonModelsFilterCancel, this); + GridBagSizer1->AddGrowableCol(0, 2); GridBagSizer1->AddGrowableCol(2, 1); GridBagSizer1->AddGrowableRow(3); @@ -889,6 +908,14 @@ void ViewsModelsPanel::AddSelectedModels(int pos) } MarkViewsChanged(); + + // Clear the Available filter so the user sees the full list again + // after moving items to the right (consistent with what we agreed — + // filter served its purpose, reset state). + if (TextCtrl_NonModelsFilter != nullptr && !_nonModelFilter.empty()) { + TextCtrl_NonModelsFilter->ChangeValue(wxEmptyString); + _nonModelFilter.clear(); + } PopulateModels(wxJoin(addedModels, ',').ToStdString()); // Update Grid @@ -1438,9 +1465,22 @@ void ViewsModelsPanel::OnLeftUp(wxMouseEvent& event) #pragma region Non Models +bool ViewsModelsPanel::IsFilteredOutOfNonModels(const std::string& name) const +{ + if (_nonModelFilter.empty()) { + return false; + } + // Case-insensitive substring match. _nonModelFilter is already lower. + wxString lname = wxString::FromUTF8(name).Lower(); + return lname.Find(wxString::FromUTF8(_nonModelFilter)) == wxNOT_FOUND; +} + void ViewsModelsPanel::AddTimingToNotList(Element* timing) { if (timing != nullptr) { + if (IsFilteredOutOfNonModels(timing->GetName())) { + return; + } wxListItem li; li.SetId(_numNonModels); li.SetText(_("")); @@ -1455,6 +1495,9 @@ void ViewsModelsPanel::AddTimingToNotList(Element* timing) void ViewsModelsPanel::AddModelToNotList(Element* model) { if (model != nullptr) { + if (IsFilteredOutOfNonModels(model->GetName())) { + return; + } wxListItem li; li.SetId(_numNonModels); li.SetText(_("")); @@ -1471,6 +1514,19 @@ void ViewsModelsPanel::AddModelToNotList(Element* model) } } +void ViewsModelsPanel::OnNonModelsFilterText(wxCommandEvent& /*event*/) +{ + _nonModelFilter = TextCtrl_NonModelsFilter->GetValue().Lower().ToStdString(); + PopulateModels(); +} + +void ViewsModelsPanel::OnNonModelsFilterCancel(wxCommandEvent& /*event*/) +{ + TextCtrl_NonModelsFilter->ChangeValue(wxEmptyString); + _nonModelFilter.clear(); + PopulateModels(); +} + void ViewsModelsPanel::OnListCtrlNonModelsItemSelect(wxListEvent& event) { ValidateWindow(); diff --git a/src-ui-wx/layout/ViewsModelsPanel.h b/src-ui-wx/layout/ViewsModelsPanel.h index 5a369b29f3..55a9ab36ee 100644 --- a/src-ui-wx/layout/ViewsModelsPanel.h +++ b/src-ui-wx/layout/ViewsModelsPanel.h @@ -25,6 +25,9 @@ #include "render/SequenceData.h" #include #include +#include + +class wxSearchCtrl; class SequenceElements; class xLightsFrame; @@ -256,5 +259,13 @@ class ViewsModelsPanel : public wxPanel void OnModelsPopup(wxCommandEvent& event); void OnImportBtnPopup(wxCommandEvent& event); + // Filter for the "Available" (non-models) list. Created outside the + // wxSmith block so the .wxs file does not need to know about it. + wxSearchCtrl* TextCtrl_NonModelsFilter = nullptr; + std::string _nonModelFilter; + void OnNonModelsFilterText(wxCommandEvent& event); + void OnNonModelsFilterCancel(wxCommandEvent& event); + bool IsFilteredOutOfNonModels(const std::string& name) const; + DECLARE_EVENT_TABLE() }; From f4508a0facd1b92fa78d0cc09668dd1a3e74a568 Mon Sep 17 00:00:00 2001 From: heffneil Date: Sun, 26 Apr 2026 16:03:56 -0400 Subject: [PATCH 2/5] Fix filter layout gap and EnsureVisible assert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups after first round of testing: 1. Layout: row 1 of the GridBagSizer is stretched by ListCtrlViews on the right (the tall view picker), so putting the search ctrl in its own GB row left a tall empty gap between filter and list. Wrap the search ctrl + ListCtrlNonModels in a vertical wxBoxSizer and put that single sizer back at the original GBPosition(1, 0) span(3, 1). Filter now sits flush above the list. 2. Asserting in EnsureVisible. PopulateModels saves the previous top index and tries to restore it after rebuilding. With a filter applied the list shrinks, so the saved topN may be >= the new GetItemCount, tripping wx's listctrl bounds assert. Bounds-check topM/topN before calling EnsureVisible — the existing code already bounds-checked topM+visible-1 / topN+visible-1, but the second plain call was unguarded. --- src-ui-wx/layout/ViewsModelsPanel.cpp | 28 +++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src-ui-wx/layout/ViewsModelsPanel.cpp b/src-ui-wx/layout/ViewsModelsPanel.cpp index 26eae34fd1..d971dbf135 100644 --- a/src-ui-wx/layout/ViewsModelsPanel.cpp +++ b/src-ui-wx/layout/ViewsModelsPanel.cpp @@ -254,18 +254,23 @@ ViewsModelsPanel::ViewsModelsPanel(xLightsFrame *frame, wxWindow* parent, wxWind _viewButtonsSizer = FlexGridSizer8; // Insert a filter control above the "Available" (non-models) list. - // The wxSmith block put ListCtrlNonModels at (1, 0) spanning rows 1-3. - // Detach it, drop the filter into row 1, and re-attach the list to - // rows 2-3 so the bottom row stays the growable one. + // The wxSmith block put ListCtrlNonModels at (1, 0) span(3, 1). + // Detach it, build a vertical box sizer holding [filter, listctrl], + // and put the whole sizer back at the original GB position. We need + // to wrap them in a sizer (not put the filter in its own GB row) + // because row 1 of the GridBagSizer is stretched by ListCtrlViews + // on the right, which would leave a tall gap between the filter + // and the list. GridBagSizer1->Detach(ListCtrlNonModels); TextCtrl_NonModelsFilter = new wxSearchCtrl(this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, wxTE_PROCESS_ENTER); TextCtrl_NonModelsFilter->SetDescriptiveText(_("Filter models...")); TextCtrl_NonModelsFilter->ShowCancelButton(true); - GridBagSizer1->Add(TextCtrl_NonModelsFilter, wxGBPosition(1, 0), wxDefaultSpan, - wxALL | wxEXPAND, 2); - GridBagSizer1->Add(ListCtrlNonModels, wxGBPosition(2, 0), wxGBSpan(2, 1), + auto* nonModelsSizer = new wxBoxSizer(wxVERTICAL); + nonModelsSizer->Add(TextCtrl_NonModelsFilter, 0, wxBOTTOM | wxEXPAND, 2); + nonModelsSizer->Add(ListCtrlNonModels, 1, wxEXPAND, 0); + GridBagSizer1->Add(nonModelsSizer, wxGBPosition(1, 0), wxGBSpan(3, 1), wxALL | wxEXPAND, 2); TextCtrl_NonModelsFilter->Bind(wxEVT_TEXT, &ViewsModelsPanel::OnNonModelsFilterText, this); @@ -575,13 +580,20 @@ void ViewsModelsPanel::PopulateModels(const std::string& selectModels) if (topM + visibileM - 1 < ListCtrlModels->GetItemCount()) { ListCtrlModels->EnsureVisible(topM + visibileM - 1); } - ListCtrlModels->EnsureVisible(topM); + if (topM >= 0 && topM < ListCtrlModels->GetItemCount()) { + ListCtrlModels->EnsureVisible(topM); + } } if (ListCtrlNonModels->GetItemCount() > 0) { if (topN + visibileN - 1 < ListCtrlNonModels->GetItemCount()) { ListCtrlNonModels->EnsureVisible(topN + visibileN - 1); } - ListCtrlNonModels->EnsureVisible(topN); + // Filtering can shrink the non-models list below topN, so + // clamp before calling EnsureVisible to avoid the wx + // generic listctrl assert. + if (topN >= 0 && topN < ListCtrlNonModels->GetItemCount()) { + ListCtrlNonModels->EnsureVisible(topN); + } } } From 2e6415158ee7ae062b245082169e5c2c92130139 Mon Sep 17 00:00:00 2001 From: heffneil Date: Mon, 27 Apr 2026 15:01:52 -0400 Subject: [PATCH 3/5] Address Copilot review on #6241 Three issues raised, all valid: 1. Memory leak in PopulateModels. Filter check was inside AddModelToNotList, but the model-groups / regular-models loops allocate a fresh ModelElement* up front and pass it in. When the item was filtered out, AddModelToNotList returned without taking ownership, leaking per keystroke on a filtered list. Move the filter check up to the call site so we never allocate when we would just drop the result. 2. Encoding consistency. _nonModelFilter was stored via wxString::ToStdString() but later compared with wxString::FromUTF8(_nonModelFilter), which only matches when the wxString build is UTF-8. Switch _nonModelFilter to wxString throughout (already lower-cased) so non-ASCII names work on every wx build. Also flip the few .empty()/.clear() callsites to .IsEmpty()/.Clear(). 3. SetEffectSequenceMode AddGrowableRow(1) confusion. Row 1 is now shared with the [filter + ListCtrlNonModels] box sizer on col 0. Making row 1 growable still does the right thing because the filter sits at proportion 0 inside the box sizer and only the listctrl below it (proportion 1) absorbs the row growth. Add a comment so future readers don't have to derive that. --- src-ui-wx/layout/ViewsModelsPanel.cpp | 38 ++++++++++++++++++--------- src-ui-wx/layout/ViewsModelsPanel.h | 4 ++- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src-ui-wx/layout/ViewsModelsPanel.cpp b/src-ui-wx/layout/ViewsModelsPanel.cpp index d971dbf135..2b537428a3 100644 --- a/src-ui-wx/layout/ViewsModelsPanel.cpp +++ b/src-ui-wx/layout/ViewsModelsPanel.cpp @@ -374,6 +374,12 @@ void ViewsModelsPanel::SetEffectSequenceMode(bool effectSeq) // Expand ListCtrlModels to cover the right side _gridBagSizer->SetItemPosition(ListCtrlModels, wxGBPosition(0, 2)); _gridBagSizer->SetItemSpan(ListCtrlModels, wxGBSpan(4, 1)); + // Row 1 is shared with the [filter + ListCtrlNonModels] box sizer + // on the left (col 0). Making row 1 growable lets the right-side + // ListCtrlModels (rows 0-3) get a fair vertical share. The filter + // itself does NOT stretch because inside the box sizer it sits at + // proportion 0; the listctrl below it (proportion 1) absorbs all + // row growth. _gridBagSizer->AddGrowableRow(1); _gridBagSizer->RemoveGrowableCol(2); _gridBagSizer->AddGrowableCol(2, 2); @@ -555,10 +561,14 @@ void ViewsModelsPanel::PopulateModels(const std::string& selectModels) } } - // Add model groups not already in the sequence elements list + // Add model groups not already in the sequence elements list. + // Check the Available filter BEFORE allocating the ModelElement - + // AddModelToNotList drops filtered items without inserting, so an + // up-front allocation would leak per keystroke on a filtered list. for (const auto& it : _xlFrame->AllModels) { if (it.second->GetDisplayAs() == DisplayAsType::ModelGroup) { - if (!_sequenceElements->ElementExists(it.first, 0)) { + if (!_sequenceElements->ElementExists(it.first, 0) && + !IsFilteredOutOfNonModels(it.first)) { ModelElement* me = new ModelElement(it.first); if (me != nullptr) AddModelToNotList(me); } @@ -568,7 +578,8 @@ void ViewsModelsPanel::PopulateModels(const std::string& selectModels) // Add regular models not already in the sequence elements list for (const auto& it : _xlFrame->AllModels) { if (it.second->GetDisplayAs() != DisplayAsType::ModelGroup) { - if (!_sequenceElements->ElementExists(it.first, 0)) { + if (!_sequenceElements->ElementExists(it.first, 0) && + !IsFilteredOutOfNonModels(it.first)) { ModelElement* me = new ModelElement(it.first); if (me != nullptr) AddModelToNotList(me); } @@ -922,11 +933,11 @@ void ViewsModelsPanel::AddSelectedModels(int pos) MarkViewsChanged(); // Clear the Available filter so the user sees the full list again - // after moving items to the right (consistent with what we agreed — - // filter served its purpose, reset state). - if (TextCtrl_NonModelsFilter != nullptr && !_nonModelFilter.empty()) { + // after moving items to the right (filter served its purpose, reset + // state). + if (TextCtrl_NonModelsFilter != nullptr && !_nonModelFilter.IsEmpty()) { TextCtrl_NonModelsFilter->ChangeValue(wxEmptyString); - _nonModelFilter.clear(); + _nonModelFilter.Clear(); } PopulateModels(wxJoin(addedModels, ',').ToStdString()); @@ -1479,12 +1490,15 @@ void ViewsModelsPanel::OnLeftUp(wxMouseEvent& event) bool ViewsModelsPanel::IsFilteredOutOfNonModels(const std::string& name) const { - if (_nonModelFilter.empty()) { + if (_nonModelFilter.IsEmpty()) { return false; } - // Case-insensitive substring match. _nonModelFilter is already lower. + // Case-insensitive substring match. _nonModelFilter is already + // lower-cased and held as wxString so we stay encoding-consistent + // (avoids std::string round-trips through ToStdString/FromUTF8 that + // can break non-ASCII names on non-UTF-8 wx builds). wxString lname = wxString::FromUTF8(name).Lower(); - return lname.Find(wxString::FromUTF8(_nonModelFilter)) == wxNOT_FOUND; + return lname.Find(_nonModelFilter) == wxNOT_FOUND; } void ViewsModelsPanel::AddTimingToNotList(Element* timing) @@ -1528,14 +1542,14 @@ void ViewsModelsPanel::AddModelToNotList(Element* model) void ViewsModelsPanel::OnNonModelsFilterText(wxCommandEvent& /*event*/) { - _nonModelFilter = TextCtrl_NonModelsFilter->GetValue().Lower().ToStdString(); + _nonModelFilter = TextCtrl_NonModelsFilter->GetValue().Lower(); PopulateModels(); } void ViewsModelsPanel::OnNonModelsFilterCancel(wxCommandEvent& /*event*/) { TextCtrl_NonModelsFilter->ChangeValue(wxEmptyString); - _nonModelFilter.clear(); + _nonModelFilter.Clear(); PopulateModels(); } diff --git a/src-ui-wx/layout/ViewsModelsPanel.h b/src-ui-wx/layout/ViewsModelsPanel.h index 55a9ab36ee..9865f181a7 100644 --- a/src-ui-wx/layout/ViewsModelsPanel.h +++ b/src-ui-wx/layout/ViewsModelsPanel.h @@ -261,8 +261,10 @@ class ViewsModelsPanel : public wxPanel // Filter for the "Available" (non-models) list. Created outside the // wxSmith block so the .wxs file does not need to know about it. + // Held as wxString (already lower-cased) so encoding stays consistent + // with the wxString-based names we compare against. wxSearchCtrl* TextCtrl_NonModelsFilter = nullptr; - std::string _nonModelFilter; + wxString _nonModelFilter; void OnNonModelsFilterText(wxCommandEvent& event); void OnNonModelsFilterCancel(wxCommandEvent& event); bool IsFilteredOutOfNonModels(const std::string& name) const; From a320273362a922093479e66da19ffe6b8e611983 Mon Sep 17 00:00:00 2001 From: heffneil Date: Mon, 27 Apr 2026 16:46:23 -0400 Subject: [PATCH 4/5] Debounce filter rebuilds in Edit Display Elements Address Copilot follow-up on #6241: PopulateModels rebuilds both list controls and re-allocates ModelElement objects for every model not already in the sequence. Running it on every keystroke is fine on small shows but visibly stutters past ~500 models. Add a 150 ms one-shot wxTimer: - wxEVT_TEXT stores the new filter text immediately, then restarts the timer - The rebuild fires only when the user pauses, collapsing fast typing into a single rebuild - The cancel button (X) still rebuilds immediately - explicit user action shouldn't wait - AddSelectedModels also stops any pending timer so the clear-on-add rebuild isn't shadowed by a stale debounce firing right after Freeze/Thaw on both list controls is already in PopulateModels, so paint batching is already covered. Caches the filter text on every keystroke so callers that read _nonModelFilter (e.g. PopulateModels via IsFilteredOutOfNonModels) always see the current value, even before the debounce fires. --- src-ui-wx/layout/ViewsModelsPanel.cpp | 37 +++++++++++++++++++++++++-- src-ui-wx/layout/ViewsModelsPanel.h | 8 ++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src-ui-wx/layout/ViewsModelsPanel.cpp b/src-ui-wx/layout/ViewsModelsPanel.cpp index 2b537428a3..4f1c57b75b 100644 --- a/src-ui-wx/layout/ViewsModelsPanel.cpp +++ b/src-ui-wx/layout/ViewsModelsPanel.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include "model-16.xpm" #include "timing-16.xpm" @@ -277,6 +278,12 @@ ViewsModelsPanel::ViewsModelsPanel(xLightsFrame *frame, wxWindow* parent, wxWind TextCtrl_NonModelsFilter->Bind(wxEVT_SEARCHCTRL_CANCEL_BTN, &ViewsModelsPanel::OnNonModelsFilterCancel, this); + // Debounce keystrokes so PopulateModels (full rebuild of both lists) + // doesn't run on every character on large shows. + _filterDebounceTimer = new wxTimer(this); + Bind(wxEVT_TIMER, &ViewsModelsPanel::OnFilterDebounceTimer, this, + _filterDebounceTimer->GetId()); + GridBagSizer1->AddGrowableCol(0, 2); GridBagSizer1->AddGrowableCol(2, 1); GridBagSizer1->AddGrowableRow(3); @@ -419,6 +426,12 @@ ViewsModelsPanel::~ViewsModelsPanel() //(*Destroy(ViewsModelsPanel) //*) + if (_filterDebounceTimer != nullptr) { + _filterDebounceTimer->Stop(); + delete _filterDebounceTimer; + _filterDebounceTimer = nullptr; + } + //for (int i = 0; i < ListCtrlNonModels->GetItemCount(); ++i) //{ // Element* e = (Element*)ListCtrlNonModels->GetItemData(i); @@ -934,8 +947,12 @@ void ViewsModelsPanel::AddSelectedModels(int pos) // Clear the Available filter so the user sees the full list again // after moving items to the right (filter served its purpose, reset - // state). + // state). Also stop any pending debounce so the upcoming + // PopulateModels call doesn't get re-run a moment later. if (TextCtrl_NonModelsFilter != nullptr && !_nonModelFilter.IsEmpty()) { + if (_filterDebounceTimer != nullptr) { + _filterDebounceTimer->Stop(); + } TextCtrl_NonModelsFilter->ChangeValue(wxEmptyString); _nonModelFilter.Clear(); } @@ -1542,17 +1559,33 @@ void ViewsModelsPanel::AddModelToNotList(Element* model) void ViewsModelsPanel::OnNonModelsFilterText(wxCommandEvent& /*event*/) { + // Capture the current text immediately so the cached filter stays + // in sync with what the user sees, but debounce the expensive + // PopulateModels rebuild — restart the one-shot timer on each + // keystroke and only rebuild after the user pauses for ~150 ms. _nonModelFilter = TextCtrl_NonModelsFilter->GetValue().Lower(); - PopulateModels(); + if (_filterDebounceTimer != nullptr) { + _filterDebounceTimer->Start(kFilterDebounceMs, wxTIMER_ONE_SHOT); + } } void ViewsModelsPanel::OnNonModelsFilterCancel(wxCommandEvent& /*event*/) { + // Explicit cancel — rebuild immediately so the user sees the full + // list without waiting for the debounce window. + if (_filterDebounceTimer != nullptr) { + _filterDebounceTimer->Stop(); + } TextCtrl_NonModelsFilter->ChangeValue(wxEmptyString); _nonModelFilter.Clear(); PopulateModels(); } +void ViewsModelsPanel::OnFilterDebounceTimer(wxTimerEvent& /*event*/) +{ + PopulateModels(); +} + void ViewsModelsPanel::OnListCtrlNonModelsItemSelect(wxListEvent& event) { ValidateWindow(); diff --git a/src-ui-wx/layout/ViewsModelsPanel.h b/src-ui-wx/layout/ViewsModelsPanel.h index 9865f181a7..e6ec4df878 100644 --- a/src-ui-wx/layout/ViewsModelsPanel.h +++ b/src-ui-wx/layout/ViewsModelsPanel.h @@ -263,10 +263,18 @@ class ViewsModelsPanel : public wxPanel // wxSmith block so the .wxs file does not need to know about it. // Held as wxString (already lower-cased) so encoding stays consistent // with the wxString-based names we compare against. + // + // PopulateModels is O(N+M) and rebuilds both list controls plus the + // model-group heap allocations. wxEVT_TEXT fires on every keystroke, + // so we debounce: keystrokes restart a one-shot timer; the rebuild + // only fires once the user pauses for kFilterDebounceMs. wxSearchCtrl* TextCtrl_NonModelsFilter = nullptr; wxString _nonModelFilter; + wxTimer* _filterDebounceTimer = nullptr; + static constexpr int kFilterDebounceMs = 150; void OnNonModelsFilterText(wxCommandEvent& event); void OnNonModelsFilterCancel(wxCommandEvent& event); + void OnFilterDebounceTimer(wxTimerEvent& event); bool IsFilteredOutOfNonModels(const std::string& name) const; DECLARE_EVENT_TABLE() From 68c7eed7e93c02a39f5a6b1bdd0ae37fdd066222 Mon Sep 17 00:00:00 2001 From: heffneil Date: Tue, 28 Apr 2026 20:38:52 -0400 Subject: [PATCH 5/5] Address Copilot review on #6241 (round 2) AddSelectedModels was only stopping the filter debounce timer when _nonModelFilter was non-empty. If the user typed into the box, backspaced to empty, and then immediately added items, the one-shot timer was still pending and would fire shortly after our PopulateModels(selectModels) call, triggering a second PopulateModels() that clobbered the selection/highlight just set. Stop the timer unconditionally before the rebuild. --- src-ui-wx/layout/ViewsModelsPanel.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src-ui-wx/layout/ViewsModelsPanel.cpp b/src-ui-wx/layout/ViewsModelsPanel.cpp index 4f1c57b75b..22d4bc75ac 100644 --- a/src-ui-wx/layout/ViewsModelsPanel.cpp +++ b/src-ui-wx/layout/ViewsModelsPanel.cpp @@ -945,14 +945,19 @@ void ViewsModelsPanel::AddSelectedModels(int pos) MarkViewsChanged(); + // Stop any pending debounce regardless of whether the filter + // currently looks empty. The user could have typed into the box, + // backspaced to empty, then immediately added items — that leaves + // a pending one-shot timer that would fire after our PopulateModels + // call below and re-rebuild the list, clobbering the selection / + // highlight we're about to set. Always stop first. + if (_filterDebounceTimer != nullptr) { + _filterDebounceTimer->Stop(); + } // Clear the Available filter so the user sees the full list again // after moving items to the right (filter served its purpose, reset - // state). Also stop any pending debounce so the upcoming - // PopulateModels call doesn't get re-run a moment later. + // state). if (TextCtrl_NonModelsFilter != nullptr && !_nonModelFilter.IsEmpty()) { - if (_filterDebounceTimer != nullptr) { - _filterDebounceTimer->Stop(); - } TextCtrl_NonModelsFilter->ChangeValue(wxEmptyString); _nonModelFilter.Clear(); }