Skip to content
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

Add custom dark theme for Windows. #12080

Merged
merged 11 commits into from Aug 12, 2023

Conversation

AdmiralCurtiss
Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss commented Jul 30, 2023

Hopefully the last PR for this...

This adds a custom theme (written from scratch by me, designed to mimic our default Windows theme but dark) for use on Windows since Qt lacks proper dark theme support by itself on Windows when using the QWindowsVistaStyle (which is the style we use on Windows). It also checks whether the system is using a dark theme and automatically switches Dolphin to the theme if it does. The style is embedded into the executable via Qt's resource compiler. Additionally, if it detects a Windows-wide dark theme, it changes the window decorations (ie, the title bar color for windows) to dark ones.

This PR was triggered by response from my comparison of available options for Windows dark themes here: #11446 (comment) We pretty much unanimously decided against Qt's Fusion theme and decided to either go with the theme from #11446 or my theme if I could finish it. So I finished it. There might still be some minor issues here and there which I'll fix up if I notice them or someone brings them to my attention. I might also add some more highlights and hover colors, but the basic look of the theme is done.

Screenshots: Overview | Controller Window | Menu

Note that the sliders are too bright right now. This stems from this ancient Qt bug that breaks the slider ticks if you try to change its color via stylesheet. There is a workaround, but I figured this PR is already big enough as-is and that fixing that would make more sense as a follow-up.

@mbc07
Copy link
Contributor

mbc07 commented Jul 31, 2023

Gave a try, it looks promising, but I noticed a few areas that might need a bit more work (by the way, my point of comparison is the Windows Explorer, which AFAICT is one of the only Win32 programs -- if not the only one-- from Windows that has proper dark mode):

Style

  • The mouse hover color for tabs, buttons, dropdown lists and game list entries is currently set to a dark blue, but that's not consistent with the hover color normally utilized in the main menus (File, Emulation, Options, Tools, etc.), which is some shade of gray (same on Windows Explorer). Could we change that to gray too?

    image
  • Some dialogs (About Dolphin, DSU Client, XLink Kai and TAP BBA Adapter settings) has blue clickable links which redirects users to the relevant pages on Dolphin's website (red arrow on the screenshot below). With the dark theme, this blue is set to a very dark shade which is hard to read. This should be adjusted to a lighter blue shade, to increase contrast between the hyperlink and the background and make them easier to read.
    Also, for some reason the dotted box around tab names (visible when you select them) is too small and cuts into the tab label when using the dark theme (yellow arrow). This doesn't happen with the light theme:

    image
  • The background of disabled radio buttons seems lighter than the background of the container, giving them a square-ish look. Is it possible to adjust that so they match?

    image
  • We have various custom indicators for our input settings (especially in Emulated Wiimote Settings). With the dark theme, they all retain a bit of their original color from the light theme, however, the C-Stick indicator of the GameCube controller becomes pitch black. Would it be possible to add a bit of yellow tint to it, to keep it consistent with the rest?

    image
  • The Advanced Input Editor seems to use hardcoded colors that make them hard to read with the dark theme (the text input here seems to share colors with the built-in GameINI editor, which also has the same problem):

    image

Behavior

  • Checkboxes from the Interface tab (from the main settings) for some reason are very laggy in this PR (both with the dark and light themes) Edit: seems to happen on master as well. They take up to 2 seconds to update when you interact with them. AFAICT other checkboxes are fine, only the ones from the Interface tab are showing this behavior.

  • I've mentioned this on Discord already, but I think the color scheme of the icons from the default Clean theme look a bit off when in Dark Mode. Would it be possible to switch them to Clean Lite when in dark mode? Clean Lite is a bit too light for the dark mode Edit: disregard this, it fits almost perfectly now that I've used it more -- but IMO it still looks a bit better than the default Clean theme. (probably out of the scope for this PR, but I had to ask)

  • When using the Fusion theme on master (by passing -style fusion via CLI), Dolphin is able to seamlessly change between dark and light themes if you change the setting on Windows while Dolphin is running, or if it's changed externally (e.g. by using Auto Dark Mode). With this PR, if Dolphin launches in dark mode, it'll remain in dark mode until you relaunch it (and vice versa). Would it be possible to re-implement that automatic theme switching when using this custom dark theme? (probably out of the scope of this PR as well)

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't look through the qtrcc.targets in detail but code wise I didn't see anything concerning. LGTM. Untested, I'll leave that to others.

@AdmiralCurtiss
Copy link
Contributor Author

AdmiralCurtiss commented Jul 31, 2023

The mouse hover color for tabs, buttons, dropdown lists and game list entries is currently set to a dark blue, but that's not consistent with the hover color normally utilized in the main menus (File, Emulation, Options, Tools, etc.), which is some shade of gray (same on Windows Explorer). Could we change that to gray too?

Yeah, I admittedly was a bit unsure what to do with those. I think I eventually ended up with gray for mouseover and dark blue for currently selected where possible, but I definitely wasn't entirely consistent there. I'll look over these again. fixed

Some dialogs (About Dolphin, DSU Client, XLink Kai and TAP BBA Adapter settings) has blue clickable links which redirects users to the relevant pages on Dolphin's website (red arrow on the screenshot below). With the dark theme, this blue is set to a very dark shade which is hard to read. This should be adjusted to a lighter blue shade, to increase contrast between the hyperlink and the background and make them easier to read.

Ah, good catch, yes that should be adjusted.

Also, for some reason the dotted box around tab names (visible when you select them) is too small and cuts into the tab label when using the dark theme (yellow arrow). This doesn't happen with the light theme:

Yeah Qt has some really weird ideas about how these dotted lines work, I had to manually adjust them elsewhere too. I'll try to see what's possible there...

The background of disabled radio buttons seems lighter than the background of the container, giving them a square-ish look. Is it possible to adjust that so they match?

Ah yup that's wrong, good catch. I'll adjust those. fixed

We have various custom indicators for our input settings (especially in Emulated Wiimote Settings). With the dark theme, they all retain a bit of their original color from the light theme, however, the C-Stick indicator of the GameCube controller becomes pitch black. Would it be possible to add a bit of yellow tint to it, to keep it consistent with the rest?

I believe that's intentional, I'll check how dark themes on Linux behave there.

The Advanced Input Editor seems to use hardcoded colors that make them hard to read with the dark theme (the text input here seems to share colors with the built-in GameINI editor, which also has the same problem):

Yes, I also noticed that but didn't look into why that happens. A few debugger widgets have the same issue. I'll check it out and compare with Linux.

Checkboxes from the Interface tab (from the main settings) for some reason are very laggy in this PR (both with the dark and light themes) Edit: seems to happen on master as well. They take up to 2 seconds to update when you interact with them. AFAICT other checkboxes are fine, only the ones from the Interface tab are showing this behavior.

Yes, this is a very long-standing issue that I've noticed before too, something in that tab takes a really long time to apply when a setting changes. I'd say this is out-of-scope here, but definitely something worth looking into. The general (non-Graphics) Config is pretty neglected in general, it doesn't even use the fancy tooltips...

I've mentioned this on Discord already, but I think the color scheme of the icons from the default Clean theme look a bit off when in Dark Mode. Would it be possible to switch them to Clean Lite when in dark mode? Clean Lite is a bit too light for the dark mode Edit: disregard this, it fits almost perfectly now that I've used it more -- but IMO it still looks a bit better than the default Clean theme. (probably out of the scope for this PR, but I had to ask)

Definitely possible, but we'd have to introduce a 'default' option for otherwise we can't distinguish between a user manually picking Clean or wanting the default theme. But yes, that's a good idea.

When using the Fusion theme on master (by passing -style fusion via CLI), Dolphin is able to seamlessly change between dark and light themes if you change the setting on Windows while Dolphin is running, or if it's changed externally (e.g. by using Auto Dark Mode). With this PR, if Dolphin launches in dark mode, it'll remain in dark mode until you relaunch it (and vice versa). Would it be possible to re-implement that automatic theme switching when using this custom dark theme? (probably out of the scope of this PR as well)

If I'm being honest, I consider this a fancy party trick and not actually anything meaningful to have. How often is a user going to switch between Light and Dark themes, really? But yeah it's probably doable, I guess you get a windows message about theme changes or something...? fixed, mostly

@MayImilae
Copy link
Contributor

MayImilae commented Jul 31, 2023

Regarding the clean theme, whether Clean Lite or just regular dark grey Clean is better depends on how dark the dark mode is. I designed Lite for Dolphin skins with ~30% brightness (#4d4d4d), where the dark grey clean icons blend in with the dark grey skin and disappear. However, the Lite theme can be too bright against REALLY dark dark skins, like, at near black (~10% brightness), lite is quite contrasty, and the regular dark grey clean icon theme can be "better". I mean, some people like the high contrast icons and some people don't, depends on the dark mode user really. But at < 10% brightness the lite theme is outside the contrast I designed it for.

Basically what is "better" for dark mode users will depend on how dark the skin we ship is, and how contrasty you want the buttons to be. And also how contrasty the average dark mode user wants the buttons to be I guess.

EDIT: For reference, the Qt theme in the screenshots currently have 13% brightness. That's really dark, and I'd argue that it is dark enough that we don't even need to worry about changing to Clean Lite. But the Lite theme isn't that bright so I don't think it would be bad or anything. Up to what you and dark mode users in general want I guess?

@mbc07
Copy link
Contributor

mbc07 commented Jul 31, 2023

If I'm being honest, I consider this a fancy party trick and not actually anything meaningful to have. How often is a user going to switch between Light and Dark themes, really? But yeah it's probably doable, I guess you get a windows message about theme changes or something...?

I normally use light mode during the day and dark mode during the night. Windows doesn't have automatic theme switching built-in, but whether you do it manually (on Windows' theme settings) or automatically with a 3rd party app (e.g. by using Auto Dark Mode -- my case), apps already running when the theme is changed just... changes themselves as appropriate, no message, notification, or anything else (that also includes most websites already loaded up in the browser).

There's certainly apps that have a dark mode but doesn't support automatic theme switching out there, but it's a nice to have. Anyway, not a blocker for this PR, I mentioned it assuming it's easy to do, since current master seems to already supports that if you launch Dolphin with the Fusion theme...

@AdmiralCurtiss
Copy link
Contributor Author

AdmiralCurtiss commented Jul 31, 2023

To be clear, I'm talking about the Windows Message Queue which is used to inform programs about various system events. I would expect theme change to be part of that, because otherwise I have no idea how to even notice that the theme has changed short of polling every once in a while.

e: Indeed, you apparently get a WM_SETTINGCHANGE with the value "ImmersiveColorSet" when the theme changes.

@AdmiralCurtiss
Copy link
Contributor Author

AdmiralCurtiss commented Aug 1, 2023

Also, for some reason the dotted box around tab names (visible when you select them) is too small and cuts into the tab label when using the dark theme (yellow arrow). This doesn't happen with the light theme:

I was unable to find a way to fix this, so I worked around it by disabling the dotted box (it's called 'outline' btw) and applied an underscore to the text instead.

seamlessly change between dark and light themes if you change the setting on Windows while Dolphin is running

This is now implemented. The title bar doesn't switch to dark properly on the light -> dark switch because Qt apparently resets it by itself in response to the message, which I'm not sure if we can do anything about unless we do something really hacky with intentional time delays.

@AdmiralCurtiss
Copy link
Contributor Author

Okay, I think I'm done, please re-check.

I have checked the C-Stick on a Linux dark theme and it's also black there, so I kept this for now. I'm open to change this though if we want this.

@mbc07
Copy link
Contributor

mbc07 commented Aug 2, 2023

Gave another try, it's almost perfect now, but...

  • I completely missed the Skylanders Portal in my initial review. It has custom colors for the Skylanders list based on the figure element, and they all look too bright when in dark mode:

    image
  • Can something be done in relation to the horizontal/vertical line separators found both in the Skylanders Portal and also in the Advanced Input Window? They probably should be some shade of gray instead of black, as black makes them practically invisible when in dark mode:

    image
  • I noticed an oddity with the dotted box that highlights the currently selected field in the game list. To reproduce it, click on a cell from a entry in the game list, then click on any empty space. In light mode, only the dotted box remain, but in dark mode, it also retains the light gray background from when the entire row was selected. I think that should be transparent (seems similar to the disabled radio button look before you fixed it):

    image

And that's pretty much it. I still think the C-Stick indicator should have a little bit of yellow, but I won't fight over it. Same about defaulting to Clean Lite when in dark mode, IMO it looks a bit better than Clean, but since May's reasoning about the design choices and considering it would require even more code just to handle switching between Clean/Clean Lite depending whether Dolphin is in light or dark mode, I'm not sure anymore that it's worth the hassle...

@AdmiralCurtiss
Copy link
Contributor Author

Fixed the Skylander colors, mapping window lines, and game list highlight (although that one feels like a Qt bug to me, I don't think an item should stay in the focus color when it's no longer in focus...?).

I'm not sure how to change the color of the the Skylander/Infinity separators, though I don't think that's really a big deal -- in fact, the more I stared at those windows the more I'm convinced they need an overhaul in general. Why are those separators even there if the entire item is one line? Why is the 'Skylander #' text a separate label instead of part of the radio button? Why does the slot list expand vertically when you increase the window size? Why are the textfields not aligned with eachother? What is going on with the spacing around the Emulate Infinity Base checkbox? Why is the Active Infinity Figures subpanel enclosed in two borders? Whatever. Maybe I'll do a follow-up.

I checked the C-Stick indicator and that color is chosen programmatically from the light theme base color, so it's nontrivial to pick a better color here without also changing the light theme one or changing all the colors that pass through this function in a dark theme. I think we're probably better off changing that so we pass two colors (for light and dark themes), but yeah I don't think that's super important so we can do this in another PR...

@mbc07
Copy link
Contributor

mbc07 commented Aug 3, 2023

There's one last thing, then this LGTM: the custom colors from the Skylanders list doesn't react to theme changes. E.g. if Dolphin launched in dark mode, the Skylanders list will retain their dark colors until Dolphin is relaunched, same thing if you've launched in light mode and later on Windows switches to dark mode, the list will retain the light colors until Dolphin is relaunched...

@AdmiralCurtiss
Copy link
Contributor Author

There's one last thing, then this LGTM: the custom colors from the Skylanders list doesn't react to theme changes. E.g. if Dolphin launched in dark mode, the Skylanders list will retain their dark colors until Dolphin is relaunched, same thing if you've launched in light mode and later on Windows switches to dark mode, the list will retain the light colors until Dolphin is relaunched...

Should be fixed.

@AdmiralCurtiss AdmiralCurtiss merged commit dfbc0e3 into dolphin-emu:master Aug 12, 2023
11 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the windark branch August 12, 2023 18:04
@dreamsyntax
Copy link
Member

dreamsyntax commented Nov 2, 2023

@AdmiralCurtiss FYI, was planning to use your custom theme in DME. Do you have any issues with that?
aldelaro5/dolphin-memory-engine#72

@AdmiralCurtiss
Copy link
Contributor Author

It's part of Dolphin so you can use it anywhere that respects the license. But yeah go ahead and use it there, I have no issues with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants