wxMSW:darkmode - wxTreeCtrl, wxDataViewCtrl and wxListCtrl (Issue #25633)
herb52
herb52 created an issue (wxWidgets/wxWidgets#25633)Hello,
First of all: Thanks for the darkmode feature in wxMSW.
I have updated my Windows 10 project from wxWidgets 3.2.1 to 3.3.0 and
I started some tests with darkmode.
For my used controls I have seen the following in darkmode:
wxTreeCtrl
for selected and focused item it uses a light gray
for selected but unfocused item it uses a dark gray
which is nearly identcal to background
wxDataViewCtrl
for selected and focused item it uses a dark blue
for selected and unfocused item it uses black
which is also nearly identcal to used background
wxListCtrl
for selected item it uses blue
I also have retested this behaviour with the released samples for dataview, treeview and listview.
The only modification is: MSWEnableDarkMode(wxApp::DarkMode_Always, nullptr); inside OnInit()
Now I was asked whether it is possible
- to enhance the visibility of selected unfocused items and
- to harmonize the used colors
This leads to question:
Is it possible to change/modify ONLY these used colors?
If yes, any hint where is appreciated.
I know that wxWidgets uses native Windows controls when possible, so maybe some colors cannot be changed.
But I do not have an overview.
I think wxDataViewCtrl is not a native control.
Thanks for your answers/comments in advance
Best regards
herb
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
PB
PBfordev left a comment (wxWidgets/wxWidgets#25633)Back then when I was trying to fix some wxListCtrl issues, I thought that we need to introduce some pseudo-system named colors for Win32 dark mode, e.g., to wxDarkModeSettings or elsewhere. The system-provided ones may not be enough these days.
Now we may have them spread hardcoded all over the code base. I originally meant colors for grid lines (wxListView or wxGrid), but of course as in this Issue for various states of items in different controls too...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25633)As you notice, it should be possible to improve things for wxDVC as it's entirely under our control. Doing it for wx{Tree,List}Ctrl risks being trickier, especially for the list one, as I spent a lot of time on it and the current look was the best I could find, even if it has known problems, see this comment:
Of course, any improvements would be welcome.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Takasa Technologies
Takasa left a comment (wxWidgets/wxWidgets#25633)First, fantastic work on darkmode, it's great!
Rather than start a new ticket could I report a darkmode issue for wxMSW::wxlistctrl here?
I am setting a different text colour for each column using (as a test) the following
wxListItemAttr* cPointInfoList::OnGetItemColumnAttr(long item, long column) const
{
if (column == 0) m_attr.SetTextColour(wxColour(0xFF,0x00,0x00));
if (column == 1) m_attr.SetTextColour(wxColour(0x00,0xFF,0x00));
if (column == 2) m_attr.SetTextColour(wxColour(0x00,0x00,0xFF));
return const_cast<wxItemAttr*>(&m_attr);
}
In lightmode this works as you would expect. In darkmode all columns get the last colour from column2.
I've followed it through to line 3240 in src/msw/listctrl.cpp HandleItemPaint
// we could use CDRF_NOTIFYSUBITEMDRAW here but it results in weird repaint
// problems so just draw everything except the focus rect from here instead
const int colCount = Header_GetItemCount(ListView_GetHeader(hwndList));
for ( int col = 0; col < colCount; col++ )
{
pLVCD->iSubItem = col;
HandleSubItemPrepaint(listctrl, pLVCD, hfont, colCount);
}
::SetTextColor(hdc, colTextOld);
pLVCD->clrText - is correct just before the col loop. Even though HandleItemPaint is called per column, this loop at the end redraws all columns for each call, I think. Which is where it goes wrong.
I continue to investigate.
I was hoping to see an obvious difference between dark and lite mode but I cannot.
Thanks all - and apologies if jumping on this existing issue was the wrong call.
Mike.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#25633)pLVCD->clrText - is correct just before the col loop. Even though HandleItemPaint is called per column, this loop at the end redraws all columns for each call, I think. Which is where it goes wrong
I can see the problem too. Like you suspected, for each row, It is using the attribute of the column that is being updated to redraw all the columns.
I made this quick hack to get the attributes of each individual column when redrawing. It is a hack because I had to make DoGetItemColumnAttr public (this is not in the diff). And it is duplicating some code from HandleItemPrepaint.
And the improvements from #25950 would also need to be integrated into this.
diff --git "a/src/msw/listctrl.cpp" "b/src/msw/listctrl.cpp" index 0511836bf6e..8d78314c15e 100644 --- "a/src/msw/listctrl.cpp" +++ "b/src/msw/listctrl.cpp" @@ -3216,26 +3216,6 @@ void HandleItemPaint(wxListCtrl* listctrl, LPNMLVCUSTOMDRAW pLVCD, HFONT hfont) } HDC hdc = nmcd.hdc; - RECT rc = GetCustomDrawnItemRect(nmcd); - - if ( nmcd.uItemState & CDIS_SELECTED ) - { - pLVCD->clrText = wxColourToRGB(wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHTTEXT)); - pLVCD->clrTextBk = wxColourToRGB(wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT)); - } - else - { - // use normal colours from pLVCD - - // do not draw item background colour under the checkbox/image - RECT rcIcon; - wxGetListCtrlItemRect(nmcd.hdr.hwndFrom, nmcd.dwItemSpec, LVIR_ICON, rcIcon); - if ( !::IsRectEmpty(&rcIcon) ) - rc.left = rcIcon.right + listctrl->FromDIP(GAP_BETWEEN_CHECKBOX_AND_TEXT); - } - - COLORREF colTextOld = ::SetTextColor(hdc, pLVCD->clrText); - ::FillRect(hdc, &rc, AutoHBRUSH(pLVCD->clrTextBk));
// we could use CDRF_NOTIFYSUBITEMDRAW here but it results in weird repaint
// problems so just draw everything except the focus rect from here instead
@@ -3243,10 +3223,49 @@ void HandleItemPaint(wxListCtrl* listctrl, LPNMLVCUSTOMDRAW pLVCD, HFONT hfont) for ( int col = 0; col < colCount; col++ ) { pLVCD->iSubItem = col; - HandleSubItemPrepaint(listctrl, pLVCD, hfont, colCount); - } - ::SetTextColor(hdc, colTextOld); + RECT rcSubItem; + if (!wxGetListCtrlSubItemRect(nmcd.hdr.hwndFrom, item, col, LVIR_BOUNDS, rcSubItem)) + continue; + + wxItemAttr* attr = listctrl->DoGetItemColumnAttr(item, col); + wxFont font; + if ( attr && attr->HasFont() ) + { + font = attr->GetFont(); + font.WXAdjustToPPI(listctrl->GetDPI()); + } + pLVCD->clrText = attr && attr->HasTextColour() + ? wxColourToRGB(attr->GetTextColour()) + : wxColourToRGB(listctrl->GetTextColour()); + pLVCD->clrTextBk = attr && attr->HasBackgroundColour() + ? wxColourToRGB(attr->GetBackgroundColour()) + : wxColourToRGB(listctrl->GetBackgroundColour()); + + if ( nmcd.uItemState & CDIS_SELECTED ) + { + pLVCD->clrText = wxColourToRGB(wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHTTEXT)); + pLVCD->clrTextBk = wxColourToRGB(wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT)); + } + else + { + // use normal colours from pLVCD + + // do not draw item background colour under the checkbox/image + RECT rcIcon; + wxGetListCtrlSubItemRect(nmcd.hdr.hwndFrom, item, col, LVIR_ICON, rcIcon); + if ( !::IsRectEmpty(&rcIcon) ) + rcSubItem.left = rcIcon.right + listctrl->FromDIP(GAP_BETWEEN_CHECKBOX_AND_TEXT); + } + + ::FillRect(hdc, &rcSubItem, AutoHBRUSH(pLVCD->clrTextBk)); + + COLORREF colTextOld = ::SetTextColor(hdc, pLVCD->clrText); + + HandleSubItemPrepaint(listctrl, pLVCD, font.IsOk() ? GetHfontOf(font) : nullptr, colCount); + + ::SetTextColor(hdc, colTextOld); + } HandleItemPostpaint(nmcd); }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25633)Rather than start a new ticket could I report a darkmode issue for wxMSW::wxlistctrl here?
It doesn't look like the same problem as was originally reported here, unless I'm missing something, so I'd rather create a separate issue for it. Also, please use triple backticks around the code snippets for them to be formatted correctly/remain readable. TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Takasa Technologies
Takasa left a comment (wxWidgets/wxWidgets#25633)Amazing thanks MaartenBent, I will see if I can patch my copy.
Vadz will do in future thanks for the advice.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#25633)I created #25962 to fix this.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.