Improve wxRendererXP::DrawGauge() in wxMSW dark mode (PR #26812)
PB
Some things may need to be improved later (see the comments), when possible (i.e., dark mode colors interface extended), but IMO, it is better to push this now, the default appearance is not that good
Samples render and dataview before and after
Image: wx-gauge-before (view on web)
Image: wx-gauge-after (view on web)
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26812
Commit Summary
- d2da0a0 Improve wxRendererXP::DrawGauge() in wxMSW dark mode
File Changes
(1 file)
- M src/msw/renderer.cpp (49)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26812.patch
- https://github.com/wxWidgets/wxWidgets/pull/26812.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
@PBfordev pushed 1 commit.
- 0c9ef61 Improve wxRendererXP::DrawGauge() in wxMSW dark mode
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
@PBfordev pushed 1 commit.
- 65e85a8 Improve wxRendererXP::DrawGauge() in wxMSW dark mode
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
@PBfordev pushed 1 commit.
- 6a3d7af Improve wxRendererXP::DrawGauge() in wxMSW dark mode
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
@PBfordev pushed 1 commit.
- b3cc8f7 Improve wxRendererXP::DrawGauge() in wxMSW dark mode
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26812)I wonder if we perhaps could draw the light version into a memory DC and then remap its colours? Has anybody already tried this by chance?
And if we do have to draw it ourselves, could we reuse wxRendererGeneric::DrawGauge() for this?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Steve Cornett
stevecor left a comment (wxWidgets/wxWidgets#26812)When DarkMode_DarkTheme is available, we should use it as below, instead of explicitly drawing.
- wxUxThemeHandle hTheme(win, L"PROGRESS");
+ wxUxThemeHandle hTheme(win, L"PROGRESS", L"DarkMode_DarkTheme::Progress");
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
PBfordev left a comment (wxWidgets/wxWidgets#26812)When
DarkMode_DarkThemeis available, we should use it as below, instead of explicitly drawing.
That was one of the first things I tried but the border is too dark:
Image: image (view on web)
Perhaps I did something wrong, the code was
void wxRendererXP::DrawGauge(wxWindow* win, wxDC& dc, const wxRect& rect, int value, int max, int flags) { wxUxThemeHandle hTheme; // Windows theming engine doesn't draw the gauge correctly in dark mode, so we need to do it ourselves. if ( wxMSWDarkMode::IsActive() ) { if ( wxCheckOsVersion(10, 0, 26200) ) { hTheme = wxUxThemeHandle(win, L"PROGRESS", L"DarkMode_DarkTheme::Progress"); if ( !hTheme ) { wxMSWDarkModeDrawGauge(dc, rect, value, max, flags); return; } } } else { hTheme = wxUxThemeHandle(win, L"PROGRESS"); } ....
We could perhaps draw our border over the system-drawn gauge but...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
PBfordev left a comment (wxWidgets/wxWidgets#26812)I wonder if we perhaps could draw the light version into a memory DC and then remap its colours? Has anybody already tried this by chance?
I have not, seemed too complicated for just drawing a filled rectangle with a border.
And if we do have to draw it ourselves, could we reuse
wxRendererGeneric::DrawGauge()for this?
The generic renderer has too dark border (comes from its DrawTextCtrl()) and a different progress color (comes from wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT))
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26812)I wonder if we perhaps could draw the light version into a memory DC and then remap its colours? Has anybody already tried this by chance?
I have not, seemed too complicated for just drawing a filled rectangle with a border.
Well, the advantage is that it would draw something different from the filled rectangle if the theme draws something more elaborate.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
PBfordev left a comment (wxWidgets/wxWidgets#26812)Well, the advantage is that it would draw something different from the filled rectangle if the theme draws something more elaborate.
If the appearance of the progress bar changes in the future (e.g., gradient), we would not be able to map the colors correctly anwyay?
wxRendererXP::DrawGauge() uses a tiny subset of the theming API, no styles or states (e.g., error or paused) are supported.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Steve Cornett
stevecor left a comment (wxWidgets/wxWidgets#26812)We could perhaps draw our border over the system-drawn gauge but...
Drawing the border is not too difficult:
hTheme.DrawBackground(
GetHdcOf(dc.GetTempHDC()),
r,
flags & wxCONTROL_SPECIAL ? PP_BARVERT : PP_BAR
);
+ if ( wxMSWDarkMode::IsActive() && wxCheckOsVersion(10, 0, 26200) )
+ {
+ AutoHBRUSH hBrush(wxMSWDarkMode::GetBorderPen().GetColour().GetPixel());
+ ::FrameRect(GetHdcOf(dc.GetTempHDC()), &r, hBrush);
+ }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent commented on this pull request.
> + const wxPen oldPen = dc.GetPen(); + const wxBrush oldBrush = dc.GetBrush();
You could use helper functions like wxDCPenChanger and wxDCBrushChanger that automatically restore the pen/brush.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
PBfordev left a comment (wxWidgets/wxWidgets#26812)Drawing the border is not too difficult:
Yes, but the actual code needs to be longer, accounting for the possibility that GetBorderPen() can, according to the docs, return an invalid pen.
IMO, we should have a draw-simple-border function which could be called from everywhere in wxWidgets. Moreover, we should have a way to get the border width, it cannot be just 1 pixel even in 200% DPI scaling, right?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
@PBfordev commented on this pull request.
> + const wxPen oldPen = dc.GetPen(); + const wxBrush oldBrush = dc.GetBrush();
Thanks, I am aware of the changers, no idea why I did not use them here.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
PBfordev left a comment (wxWidgets/wxWidgets#26812)I am at a loss at what to do next:
- Do we need to have two pathways for the dark mode: (a) when DarkTheme is available (so we combine the system and our drawing) and (b) when it is not (draw fully ourselves).
- How to draw the gauge in (1b): simple drawing or attempt some magic with light theme color replacement?
- How to obtain the gauge color in (1b): Will there be the dark mode color API extended?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Steve Cornett
stevecor left a comment (wxWidgets/wxWidgets#26812)Yes, but the actual code needs to be longer, accounting for the possibility that GetBorderPen() can, according to the docs, return an invalid pen.
We have no provision for GetBorderPen() returning an invalid pen. The documentation should probably be updated. Generally you override a function for customized behavior, not to enable default or system behavior.
we should have a way to get the border width, it cannot be just 1 pixel even in 200% DPI scaling, right?
We do not take DPI scaling into account for border width. See wxWindowMSW::MSWGetBorderThickness().
- Do we need to have two pathways for the dark mode: (a) when DarkTheme is available (so we combine the system and our drawing) and (b) when it is not (draw fully ourselves).
Yes, something like this:
if ( wxMSWDarkMode::IsActive() && !wxMSWDarkMode::HasDarkTheme() )
{
wxMSWDarkModeDrawGauge(dc, rect, value, max, flags);
return;
}
wxUxThemeHandle hTheme(win, L"PROGRESS", L"DarkMode_DarkTheme::Progress");
- How to draw the gauge in (1b): simple drawing or attempt some magic with light theme color replacement?
Simple is better than complicated. Good enough is good enough.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
@PBfordev pushed 1 commit.
- 0509c63 Improve wxRendererXP::DrawGauge() in wxMSW dark mode
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
PBfordev left a comment (wxWidgets/wxWidgets#26812)@stevecor, thanks I mostly agree with what you said and the last version of the code reflects that.
But my question was mostly targeted at the maintainers.
There are still two issues:
- Can we rely on
wxDarMode::GetBorderPen()always returning a valid pen (contrary to the current docs). As I wrote elsewhere, perhaps we should have aDrawSimpleBorder()function (assuming we always use 1 physical pixel border width). - Getting the progress bar color from a central place, instead of hard-coding it locally.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
PB
@PBfordev pushed 1 commit.
- 769208e Improve wxRendererXP::DrawGauge() in wxMSW dark mode
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26812)Sorry, I don't know the answers to the questions above.
We do need to do something about the colours, it's a mess and becoming more and more so... Also, I'd probably move the code for drawing it manually into darkmode.cpp but it's a detail.
Do you think this is ready to be merged otherwise?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.