-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Qt: Set QStyleHints colorScheme property properly #13380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f33b9bc to
49ee550
Compare
|
Oh, macOS is still on an old Qt version. |
|
I could add a fallback, but there's been some talk of updating the version of Qt used on macOS, so I'll just leave it for now until some other members of the team decide what to do. |
cca43a5 to
f792dd3
Compare
|
Added the fallback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f792dd3 to
9e48fc3
Compare
This should be fixed now. It appears the problem was that some styles (e.g. the Windows Classic/Windows Vista style) just don't obey the new colorScheme property (but it's still reported to the application), |
9e48fc3 to
eeaa282
Compare
|
Removed the fallback again now that macOS has been updated. Needs testing again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- ✅ Palette based on whether the window text value is lighter or darker than the window value for the "fusion" theme seems more sensible than an arbitrary 0.5 text value cutoff.
- ✅ Changes to
QtUtilsbesides that are just a formatting fix and adding a comment for another unrelated option. - ✅ Color scheme property introduced in Qt 6.5 seems a lot better than trying to roll our own.
- ✅ Tested, and it fixes the outlines for me like it does for everyone else.
- ✅ Without an extensive read of the QStyleHints docs, I can't immediately think of a way this can be improved.
Aside: on line 202 of DropIndicators.cpp, it looks like a tab somehow got replaced in this PR with a bunch of spaces by accident.
|
Looks like this PR has a regression where certain on platforms (e.g. macOS) it breaks live switching of OS themes.
Some wild ideas are:
|
|
I think the problem might be that I'm explicitly settings the colorScheme property rather than unsetting it for the native themes. But I remember that unsetting it makes it return wrong results in some cases, so then it can't be used directly (and Qt uses it directly internally...). |
eeaa282 to
567fdce
Compare
|
Alright, I've changed how this works: Now I'm only setting the colorScheme property (including to Unknown where that makes sense, rather than Light or Dark), and not reading it, since if it's set to Unknown qApp->styleHints()->colorScheme() can return the wrong value (e.g. in the case of the Windows Classic theme), and if I never set it to Unknown (as I was doing before) we never get notified of OS colour scheme changes. |
b2854b4 to
b667326
Compare
|
Needs testing again. Sorry about that. |
b764e59 to
9a48b63
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for documentation sake, and so I remember..... I found that this conflicts with the cocoa theme change code and causes a feedback effect. Just waiting for this PR to remove that code.
9a48b63 to
9a2d9be
Compare
|
Removed the custom macOS theme changing code. Thanks for spotting that! |
9a2d9be to
a5f91ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine on macOS.









Description of Changes
Sets the colorScheme property of the qApp->styleHint() object whenever the theme is changed.
We're still using a palette heuristic to figure out if it's a light or dark theme since in some cases qApp->styleHints()->colourScheme() will return the wrong result (I've added comments explaining this in more detail).
This also removes some custom logic for detecting theme changes on macOS because it was conflicting with Qt's platform code (which now handles this automatically) which was causing an infinite loop.
Rationale behind Changes
This lets Qt know that we're using a light or dark theme, which will allow it to do a better job at drawing certain widgets. For example before checkboxes looked like this (with a dark OS theme set):
After these changes they look like this:
Suggested Testing Steps
Test on Windows and macOS with the native themes. Try changing between light and dark themes in the settings window, and try changing between light, dark and high contrast themes in the OS.
Did you use AI to help find, test, or implement this issue or feature?
No.