From d9684e1ceb83ef45dced68b20653e44f6bef4735 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 26 May 2019 23:17:47 +0200 Subject: [PATCH 1/2] Allow calling EnableSystemTheme(false) before creating the window This is important as enabling the system theme results in changes to the native ListView control appearance that are not undone later even if the system theme is disabled. Notably, the item rectangle width is reduced by 2 pixels when system theme is enabled and it's not increased to its original value even when it's disabled later, resulting in gaps between items through which the control background shows, for example. This also makes items drawn using our own HandleItemPaint() slightly, but noticeably, larger than the items using standard appearance, which looks bad. All these problems can be avoided if we skip enabling the system theme in the first place if EnableSystemTheme(false) had been called before creating the control, so support doing it like this and document that this is the preferred way of disabling the system theme use. Closes #17404, #18296. --- include/wx/systhemectrl.h | 29 ++++++++++++++++++++++++++++- interface/wx/systhemectrl.h | 21 +++++++++++++++++++++ src/generic/datavgen.cpp | 2 +- src/msw/listctrl.cpp | 2 +- src/msw/systhemectrl.cpp | 11 +++++++++-- src/msw/treectrl.cpp | 2 +- 6 files changed, 61 insertions(+), 6 deletions(-) diff --git a/include/wx/systhemectrl.h b/include/wx/systhemectrl.h index 269c361855..5881b6cbd1 100644 --- a/include/wx/systhemectrl.h +++ b/include/wx/systhemectrl.h @@ -21,7 +21,21 @@ class WXDLLIMPEXP_FWD_CORE wxWindow; class WXDLLIMPEXP_CORE wxSystemThemedControlBase { public: - wxSystemThemedControlBase() { } + wxSystemThemedControlBase() + { +#ifdef wxHAS_SYSTEM_THEMED_CONTROL + m_systemThemeDisabled = false; +#endif // wxHAS_SYSTEM_THEMED_CONTROL + } + + bool IsSystemThemeDisabled() const + { +#ifdef wxHAS_SYSTEM_THEMED_CONTROL + return m_systemThemeDisabled; +#else // !wxHAS_SYSTEM_THEMED_CONTROL + return false; +#endif // wxHAS_SYSTEM_THEMED_CONTROL/!wxHAS_SYSTEM_THEMED_CONTROL + } virtual ~wxSystemThemedControlBase() { } @@ -36,6 +50,11 @@ protected: (bool WXUNUSED(enable), wxWindow* WXUNUSED(window)) { } #endif // wxHAS_SYSTEM_THEMED_CONTROL +private: +#ifdef wxHAS_SYSTEM_THEMED_CONTROL + bool m_systemThemeDisabled; +#endif // wxHAS_SYSTEM_THEMED_CONTROL + wxDECLARE_NO_COPY_CLASS(wxSystemThemedControlBase); }; @@ -54,6 +73,14 @@ public: } protected: + void EnableSystemThemeByDefault() + { + // Check if the system theme hadn't been explicitly disabled before + // enabling it by default. + if ( !this->IsSystemThemeDisabled() ) + DoEnableSystemTheme(true, this); + } + wxDECLARE_NO_COPY_TEMPLATE_CLASS(wxSystemThemedControl, C); }; diff --git a/interface/wx/systhemectrl.h b/interface/wx/systhemectrl.h index 60a4405027..8d9a871e15 100644 --- a/interface/wx/systhemectrl.h +++ b/interface/wx/systhemectrl.h @@ -45,6 +45,27 @@ }; @endcode + Please also note that if you want to disable the system theme use in the + control that use it by default, it's best to do it before actually creating + the control as enabling the system theme can't always be completely undone + later. I.e. instead of + @code + // THIS CODE IS WRONG, DO NOT DO IT LIKE THIS + wxTreeCtrl* tree = new wxTreeCtrl(parent, wxID_ANY); + tree->EnableSystemTheme(false); + @endcode + prefer the following version: + @code + // Use default ctor to create the object, avoiding creating the window. + wxTreeCtrl* tree = new wxTreeCtrl(); + + // Then disable the system theme used by default. + tree->EnableSystemTheme(false); + + // And only then actually create the window. + tree->Create(parent, wxID_ANY); + @endcode + On non-MSW platforms this class currently does nothing but is still available, so that it can be used in portable code without any conditional compilation directives. diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 704e0ac4b3..12cc27d81c 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -5177,7 +5177,7 @@ bool wxDataViewCtrl::Create(wxWindow *parent, sizer->Add( m_clientArea, 1, wxGROW ); SetSizer( sizer ); - EnableSystemTheme(); + EnableSystemThemeByDefault(); #if wxUSE_ACCESSIBILITY wxAccessible::NotifyEvent(wxACC_EVENT_OBJECT_CREATE, this, wxOBJID_CLIENT, wxACC_SELF); diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 7b85bf262d..44a795ccdf 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -302,7 +302,7 @@ bool wxListCtrl::Create(wxWindow *parent, if ( !MSWCreateControl(WC_LISTVIEW, wxEmptyString, pos, size) ) return false; - EnableSystemTheme(); + EnableSystemThemeByDefault(); // explicitly say that we want to use Unicode because otherwise we get ANSI // versions of _some_ messages (notably LVN_GETDISPINFOA) diff --git a/src/msw/systhemectrl.cpp b/src/msw/systhemectrl.cpp index 8634368514..03c13271fb 100644 --- a/src/msw/systhemectrl.cpp +++ b/src/msw/systhemectrl.cpp @@ -24,8 +24,15 @@ void wxSystemThemedControlBase::DoEnableSystemTheme(bool enable, wxWindow* windo { if ( wxGetWinVersion() >= wxWinVersion_Vista && wxUxThemeIsActive() ) { - const wchar_t* const sysThemeId = enable ? L"EXPLORER" : NULL; - ::SetWindowTheme(GetHwndOf(window), sysThemeId, NULL); + // It's possible to call EnableSystemTheme(false) before creating the + // window, just don't do anything in this case. + if ( window->GetHandle() ) + { + const wchar_t* const sysThemeId = enable ? L"EXPLORER" : NULL; + ::SetWindowTheme(GetHwndOf(window), sysThemeId, NULL); + } + + m_systemThemeDisabled = !enable; } } diff --git a/src/msw/treectrl.cpp b/src/msw/treectrl.cpp index e9f9defff4..8e1025ab24 100644 --- a/src/msw/treectrl.cpp +++ b/src/msw/treectrl.cpp @@ -790,7 +790,7 @@ bool wxTreeCtrl::Create(wxWindow *parent, { // The Vista+ system theme uses rotating ("twist") buttons, so we map // this style to it. - EnableSystemTheme(); + EnableSystemThemeByDefault(); } return true; From 551dfea59aa1a70a9867af6842d95f5d69a909fc Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 26 May 2019 22:41:57 +0200 Subject: [PATCH 2/2] Fix wrong appearance of selected unfocused items in wxListCtrl Items using wxSYS_COLOUR_BTNFACE (light grey in the default theme) for their background are, for some reason, drawn using the colour of active selection (blue) even when the control doesn't have focus. This is wrong, but there just doesn't seem to be any way to prevent this from happening when using the native drawing logic other than not using wxSYS_COLOUR_BTNFACE for the background (modifying any colour channel by 1 is enough to work around the issue). So to draw the item ourselves in this case and hope that it remains indistinguishable from the native appearance when not using the system theme. Closes #17988. --- src/msw/listctrl.cpp | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 44a795ccdf..fc27322efc 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -2835,8 +2835,9 @@ bool HandleSubItemPrepaint(LPNMLVCUSTOMDRAW pLVCD, HFONT hfont, int colCount) const int col = pLVCD->iSubItem; const DWORD item = nmcd.dwItemSpec; - // the font must be valid, otherwise we wouldn't be painting the item at all - SelectInHDC selFont(hdc, hfont); + SelectInHDC selFont; + if ( hfont ) + selFont.Init(hdc, hfont); // get the rectangle to paint RECT rc; @@ -3053,6 +3054,22 @@ static WXLPARAM HandleItemPrepaint(wxListCtrl *listctrl, return CDRF_NEWFONT; } + // For some unknown reason, native control incorrectly uses the active + // selection colour for the background of the items using COLOR_BTNFACE as + // their custom background even when the control doesn't have focus (see + // #17988). To work around this, draw the item ourselves in this case. + // + // Note that the problem doesn't arise when using system theme, which is + // lucky as HandleItemPaint() doesn't result in the same appearance as with + // the system theme, so we should avoid using it in this case to ensure + // that all items appear consistently. + if ( listctrl->IsSystemThemeDisabled() && + pLVCD->clrTextBk == ::GetSysColor(COLOR_BTNFACE) ) + { + HandleItemPaint(pLVCD, NULL); + return CDRF_SKIPDEFAULT; + } + return CDRF_DODEFAULT; }