From cbd1bf2c37adda529489eab1101164be7879e498 Mon Sep 17 00:00:00 2001 From: Kris Austin Date: Thu, 27 Aug 2026 06:06:50 -0500 Subject: [PATCH] build: clear 295 more -Woverloaded-virtual warnings in GUI widgets (#15394) build: clear 295 -Woverloaded-virtual warnings in GUI widgets Turns three hidden base virtuals into real overrides, clearing 295 of the 553 -Woverloaded-virtual warnings and taking a full clang-cl build from 1,264 to 969. Part of #15374. Search.hpp: SearchDialog::Popup and SearchObjectDialog::Popup took a wxPoint that neither body ever read, hiding the virtual wxPopupTransientWindow::Popup(wxWindow*). Both bodies clear the input, call the base, set focus and refill the list, and SearchObjectDialog also guards re-entry, so hiding meant none of that ran when the window was popped through a base pointer. They now override and forward focus. LabeledStaticBox::SetFont and ScrolledWindow::SetBackgroundColour hid their base virtuals the same way, so the label metrics recompute and the child colour propagation only ran for callers holding the concrete type. Both now override. Marking a member override makes clang flag every other unmarked override in the same class, so seven sibling declarations needed the keyword too. Left unmarked they were worth 481 warnings, which would have made this a net loss. MSWDismissUnfocusedPopup is declared only inside #ifdef __WXMSW__ in wx/popupwin.h, so off Windows there is no base virtual to override and the keyword would not compile. Both the declarations and the definitions are guarded, which is how wxWidgets itself declares MSWWindowProc in wx/nativewin.h and how this repo already handles it in BBLTopbar, MainFrame, Button, ComboBox and TabCtrl. ScrolledWindow's constructor left m_userPanel and m_scroll_win uninitialised unless the style requested a vertical scrollbar, while SetBackgroundColour dereferences both. No caller hits that today since every instantiation passes wxVSCROLL, but the override widens who can reach them, so they are now initialised alongside their siblings. Co-authored-by: Rodrigo Faselli <162915171+RF47@users.noreply.github.com> --- src/slic3r/GUI/Search.cpp | 12 ++++++++---- src/slic3r/GUI/Search.hpp | 20 ++++++++++++-------- src/slic3r/GUI/Widgets/LabeledStaticBox.cpp | 3 ++- src/slic3r/GUI/Widgets/LabeledStaticBox.hpp | 2 +- src/slic3r/GUI/Widgets/ScrolledWindow.cpp | 7 +++++-- src/slic3r/GUI/Widgets/ScrolledWindow.hpp | 4 ++-- 6 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/slic3r/GUI/Search.cpp b/src/slic3r/GUI/Search.cpp index 1d5cf0b9e5..f8fc51ed02 100644 --- a/src/slic3r/GUI/Search.cpp +++ b/src/slic3r/GUI/Search.cpp @@ -681,7 +681,7 @@ SearchDialog::SearchDialog(OptionsSearcher *searcher, Preset::Type type, wxWindo SearchDialog::~SearchDialog() {} -void SearchDialog::Popup(wxPoint position /*= wxDefaultPosition*/) +void SearchDialog::Popup(wxWindow *focus /*= nullptr*/) { /* const std::string& line = searcher->search_string(); search_line->SetValue(line.empty() ? default_string : from_u8(line)); @@ -696,17 +696,19 @@ void SearchDialog::Popup(wxPoint position /*= wxDefaultPosition*/) search_line2->SetValue(wxString("")); //const std::string &line = searcher->search_string(); //searcher->search(into_u8(line), true); - PopupWindow::Popup(); + PopupWindow::Popup(focus); search_line2->SetFocus(); update_list(); } +#ifdef __WXMSW__ void SearchDialog::MSWDismissUnfocusedPopup() { Dismiss(); OnDismiss(); } +#endif // __WXMSW__ void SearchDialog::OnDismiss() { } @@ -926,7 +928,7 @@ SearchObjectDialog::SearchObjectDialog(GUI::ObjectList* object_list, wxWindow* p SearchObjectDialog::~SearchObjectDialog() {} -void SearchObjectDialog::Popup(wxPoint position /*= wxDefaultPosition*/) +void SearchObjectDialog::Popup(wxWindow *focus /*= nullptr*/) { if (m_is_dismissing || this->IsShown()) { return; @@ -937,7 +939,7 @@ void SearchObjectDialog::Popup(wxPoint position /*= wxDefaultPosition*/) // dropdown list, otherwise the text input won't be usable m_object_list->SetFocus(); #endif - PopupWindow::Popup(); + PopupWindow::Popup(focus); search_line2->SetFocus(); m_object_list->assembly_plate_object_name(); @@ -945,11 +947,13 @@ void SearchObjectDialog::Popup(wxPoint position /*= wxDefaultPosition*/) update_list(); } +#ifdef __WXMSW__ void SearchObjectDialog::MSWDismissUnfocusedPopup() { Dismiss(); OnDismiss(); } +#endif // __WXMSW__ void SearchObjectDialog::OnDismiss() {} diff --git a/src/slic3r/GUI/Search.hpp b/src/slic3r/GUI/Search.hpp index bdb4da83c4..4ae43dbca0 100644 --- a/src/slic3r/GUI/Search.hpp +++ b/src/slic3r/GUI/Search.hpp @@ -216,10 +216,12 @@ public: SearchDialog(OptionsSearcher *searcher, Preset::Type type, wxWindow *parent, TextInput *input, wxWindow *search_btn); ~SearchDialog(); - void MSWDismissUnfocusedPopup(); - void Popup(wxPoint position = wxDefaultPosition); - void OnDismiss(); - void Dismiss(); +#ifdef __WXMSW__ + void MSWDismissUnfocusedPopup() override; +#endif // __WXMSW__ + void Popup(wxWindow *focus = nullptr) override; + void OnDismiss() override; + void Dismiss() override; void Die(); void msw_rescale(); @@ -260,10 +262,12 @@ public: SearchObjectDialog(GUI::ObjectList* object_list, wxWindow* parent, TextInput* input); ~SearchObjectDialog(); - void MSWDismissUnfocusedPopup(); - void Popup(wxPoint position = wxDefaultPosition); - void OnDismiss(); - void Dismiss(); +#ifdef __WXMSW__ + void MSWDismissUnfocusedPopup() override; +#endif // __WXMSW__ + void Popup(wxWindow *focus = nullptr) override; + void OnDismiss() override; + void Dismiss() override; void Die(); void OnInputText(wxCommandEvent& event); diff --git a/src/slic3r/GUI/Widgets/LabeledStaticBox.cpp b/src/slic3r/GUI/Widgets/LabeledStaticBox.cpp index c8e054593f..a11839a8b8 100644 --- a/src/slic3r/GUI/Widgets/LabeledStaticBox.cpp +++ b/src/slic3r/GUI/Widgets/LabeledStaticBox.cpp @@ -98,7 +98,7 @@ void LabeledStaticBox::SetBorderColor(StateColor const &color) Refresh(); } -void LabeledStaticBox::SetFont(wxFont set_font) +bool LabeledStaticBox::SetFont(const wxFont &set_font) { m_font = set_font; @@ -109,6 +109,7 @@ void LabeledStaticBox::SetFont(wxFont set_font) m_label_width = tW; Refresh(); + return true; } bool LabeledStaticBox::Enable(bool enable) diff --git a/src/slic3r/GUI/Widgets/LabeledStaticBox.hpp b/src/slic3r/GUI/Widgets/LabeledStaticBox.hpp index f42175ae05..d3e7f2efce 100644 --- a/src/slic3r/GUI/Widgets/LabeledStaticBox.hpp +++ b/src/slic3r/GUI/Widgets/LabeledStaticBox.hpp @@ -42,7 +42,7 @@ public: void SetBorderColor(StateColor const &color); - void SetFont(wxFont set_font); + bool SetFont(const wxFont &set_font) override; bool Enable(bool enable) override; diff --git a/src/slic3r/GUI/Widgets/ScrolledWindow.cpp b/src/slic3r/GUI/Widgets/ScrolledWindow.cpp index 6aa6f5b600..90922f93f1 100644 --- a/src/slic3r/GUI/Widgets/ScrolledWindow.cpp +++ b/src/slic3r/GUI/Widgets/ScrolledWindow.cpp @@ -21,6 +21,8 @@ ScrolledWindow::ScrolledWindow(wxWindow *parent, wxWindowID id, wxPoint position m_bottomScrollbar = NULL; m_verticalSplitter = NULL; m_horizontalSplitter = NULL; + m_userPanel = NULL; + m_scroll_win = NULL; m_marginWidth = marginWidth; @@ -110,12 +112,13 @@ void ScrolledWindow::SetTipColor(wxColour color) if (m_bottomScrollbar) m_bottomScrollbar->SetTipColor(color); } -void ScrolledWindow::SetBackgroundColour(wxColour color) +bool ScrolledWindow::SetBackgroundColour(const wxColour &color) { - wxWindow::SetBackgroundColour(color); + const bool result = wxWindow::SetBackgroundColour(color); m_verticalSplitter->SetBackgroundColour(color); m_userPanel->SetBackgroundColour(color); m_scroll_win->SetBackgroundColour(color); + return result; } void ScrolledWindow::SetMarginColor(wxColour color) diff --git a/src/slic3r/GUI/Widgets/ScrolledWindow.hpp b/src/slic3r/GUI/Widgets/ScrolledWindow.hpp index 38409a19d4..5c2bc2f9e5 100644 --- a/src/slic3r/GUI/Widgets/ScrolledWindow.hpp +++ b/src/slic3r/GUI/Widgets/ScrolledWindow.hpp @@ -15,7 +15,7 @@ public: ScrolledWindow(wxWindow *parent, wxWindowID id, wxPoint position, wxSize size, long style, int marginWidth = 0, int scrollbarWidth = 4, int tipLength = 0); void OnMouseWheel(wxMouseEvent &event); void SetTipColor(wxColour color); - void SetBackgroundColour(wxColour color); + bool SetBackgroundColour(const wxColour &color) override; void SetMarginColor(wxColour color); void SetScrollbarColor(wxColour color); @@ -26,7 +26,7 @@ public: // wxSplitterWindow* GetVerticalSplitter() { return m_verticalSplitter; } // wxSplitterWindow* GetHorizontalSplitter() { return m_horizontalSplitter; } bool IsBothDirections() { return m_bothDirections; } - virtual void SetScrollbars(int pixelsPerUnitX, int pixelsPerUnitY, int noUnitsX, int noUnitsY, int xPos = 0, int yPos = 0, bool noRefresh = false); + virtual void SetScrollbars(int pixelsPerUnitX, int pixelsPerUnitY, int noUnitsX, int noUnitsY, int xPos = 0, int yPos = 0, bool noRefresh = false) override; private: wxPanel * m_userPanel; // the panel targeted by the scrolled window