dolphin icon indicating copy to clipboard operation
dolphin copied to clipboard

Windows/DolphinQt: set the default style to fusion

Open shuffle2 opened this issue 1 year ago • 26 comments

shuffle2 avatar Apr 23 '23 18:04 shuffle2

Before:

image

After:

image

This looks kinda weird to me (it doesn't match the system styling at least).

Would it be possible to allow configuring it in the interface settings? As far as I can tell, this is unrelated to the currently available theme setting.

Pokechu22 avatar Apr 23 '23 23:04 Pokechu22

I really don’t want to make it configurable (note: the cmdline can still be used to override it). Also, if we move away from windowsvista style we can stop distributing the related DLL for that style (fusion is built in).

my understanding is that fusion is the default style on Linux and maybe macOS. It’s not windows-looking , but the style really grew on me after using it a bit, especially after seeing it at higher dpi scaling and dark mode, compared to the previous style - try it out for a bit and report back 😛.

shuffle2 avatar Apr 24 '23 01:04 shuffle2

Hmm. For reference, this is the windows style, which is definitely not what we want:

image

For some reason, I assumed that the windows style would be newer than the vista style. But clearly it's not.

Also, at first I wasn't sure what you meant by distributing the DLL for it, since I only see Qt6Core.dll/Qt6Gui.dll/Qt6Widgets.dll next to Dolphin.exe, but it looks like the DLL exists at QtPlugins\styles\qwindowsvistastyle.dll. It also seems like only fusion and windows are built-in.

Pokechu22 avatar Apr 24 '23 01:04 Pokechu22

Fwiw the build linked from https://github.com/dolphin-emu/dolphin/pull/11785#issue-1679985741 has qt6.5 and most of my prs integrated - not sure if there have been noticeable improvements to fusion styling, but that’s more representative of where I’m looking to end up.

shuffle2 avatar Apr 24 '23 01:04 shuffle2

TBH, I don't think fusion brings any noticeable improvement to the GUI styling, at least based on what I've seen so far. If the only argument for making it the default style (and not allowing users to change it without resorting to command-line parameters) is "there's one less DLL to ship", then yeah, that's a hard pass from me.

Currently, on master, Dolphin will match the system style of the OS it's running on and won't look out of place, as it'll render like any other native app from that same OS. By switching to fusion, it'll have a consistent style (identical across all OSes we support) but at the same time it'll feel out of place across all OSes we support, since it won't match the look and feel of any of them. I don't think this is the right approach.


I tested both this PR and dolphin-svg-dpi.7z you've linked on PR #11785 and something is wrong with the latter. The build from this PR renders like the screenshots posted earlier by Pokechu22, but it won't react to the light/dark mode setting from the OS.

Fusion on the light color theme on Windows doesn't match the system style but looks... usable. Accent color remains blue and it matches well with our Clean icon sets. I still prefer the Vista style, though. Didn't manage to make it switch to the dark color theme, so I can't evaluate that.

~~When testing dolphin-svg-dpi.7z, things look very weird, I'm almost certain something is broken. This time it reacts to the light/dark mode setting from the OS but the styling doesn't seem to match neither fusion, nor vista.~~ Nevermind, adding -style fusion makes it work as expected (missed the instructions at a first glance). The color scheme from dark mode fusion definitely needs to be tweaked, especially tab backgrounds (seems too light for a dark theme):

image

mbc07 avatar Apr 24 '23 05:04 mbc07

the reason to move to fusion is to support dark mode. for dolphin-svg-dpi.7z you need to pass the cmdline option -style fusion because it does not include the change in this PR

shuffle2 avatar Apr 24 '23 05:04 shuffle2

So, to get things straight: Qt Vista style doesn't support dark mode?

mbc07 avatar Apr 24 '23 05:04 mbc07

So, to get things straight: Qt Vista style doesn't support dark mode?

that's correct

shuffle2 avatar Apr 24 '23 05:04 shuffle2

I was also under the impression that fusion is already the default style on linux and maybe on macos. If that's not the case it would be nice to know. IMO having identical style across all OS could be nice (less platform-specific stuff to think about, theoretically), but we could make this PR only apply to windows builds if that's better.

shuffle2 avatar Apr 24 '23 05:04 shuffle2

that's correct

Okay, that makes the change far more reasonable.

Is it possible to at least tweak Fusion default colors? Fusion light looks fine out of the box, but Fusion dark is a bit jarring (especially the tab backgrounds)

I was also under the impression that fusion is already the default style on linux and maybe on macos

AFAICT Qt has a macOS style that matches the look and feel of the OS (similar to the Vista style on Windows). I have no idea if it already supports dark mode, though.

mbc07 avatar Apr 24 '23 05:04 mbc07

It is possible to tweak the style settings although I don't have any clue how to do it atm. I'm not totally convinced the theme "looks bad" by default - I'm not sure if it's working as intended (perhaps we could check against some other Qt app which uses similar dialog elements), or if Dolphin has just managed to layout the window elements in some technically-incorrect way, or what.

I know dark mode is already working on macOS before this change, but I'm not sure what style macOS is defaulting to. Someone should just try the macos buildbot build of this PR and let us know if dolphin looks different :p

shuffle2 avatar Apr 24 '23 06:04 shuffle2

Tried the macOS build, and it does look quite different.

Personally, I think I prefer the current style on macOS. It makes Dolphin look more native and takes into account user settings like the system's accent color.

5.0-19280

image

This PR

image

OatmealDome avatar Apr 24 '23 06:04 OatmealDome

Thanks for checking. I don't really have an opinion about how it looks on non-windows OS, so i'm fine with making this PR only apply to windows, if that's what people want.

shuffle2 avatar Apr 24 '23 06:04 shuffle2

I'm not totally convinced the theme "looks bad" by default - I'm not sure if it's working as intended (perhaps we could check against some other Qt app which uses similar dialog elements), or if Dolphin has just managed to layout the window elements in some technically-incorrect way, or what.

There is a tab dialog example: https://doc.qt.io/qt-6/qtwidgets-dialogs-tabdialog-example.html

Here's how it looks when compiled against Qt6.5 running in fusion dark mode: image image image so, what we are seeing in dolphin is the intended behavior.

Also, the Qt examples running with fusion style do not have have any problem reacting to dark/light mode changes while they are open. So that would appear to be a dolphin bug (which I think can be fixed in a separate PR ).

shuffle2 avatar Apr 24 '23 07:04 shuffle2

The fusion theme looks a bit dated honestly. If we are forced to use this for light theme as well, to get the dark mode functionality, then I'd honestly prefer shipping a dark theme seperately. But I'm obviously a bit biased since I made the original PR for that :)

FearlessTobi avatar Apr 24 '23 21:04 FearlessTobi

Also, the Qt examples running with fusion style do not have have any problem reacting to dark/light mode changes while they are open. So that would appear to be a dolphin bug (which I think can be fixed in a separate PR ).

Anyone more experienced with Qt have pointers about how to go about debugging this? I'm assuming there's some signal propagation issue but not sure if there's a standard/easy way to debug that.

shuffle2 avatar Apr 25 '23 03:04 shuffle2

OK, from trial and error I found that it's Settings::SetCurrentUserStyle which breaks it.

shuffle2 avatar Apr 25 '23 05:04 shuffle2

I don't really have an opinion about how it looks on non-windows OS, so i'm fine with making this PR only apply to windows, if that's what people want.

I think that's a good idea. The fusion style has a kind of "ten year old linux frontend" vibe (kind of gnome 3ish?) that's not great but just kind of fine, imo. On Windows it's a step up from the vista theme, but it's a step back on other platforms like macOS. Making this Windows only seems like a good choice.

MayImilae avatar Apr 25 '23 05:04 MayImilae

OK, from trial and error I found that it's Settings::SetCurrentUserStyle which breaks it.

It seems the problem occurs if setStyleSheet has been called with non-empty string. Calling it first with non-empty string (by default, just the the QToolTip setting), and then empty string does not fix the problem.

A hacky workaround will be to just avoid calling setStyleSheet if the QToolTip settings are the default values. So then the dynamic mode change will work fine, unless the user is using non-default settings. If a user-supplied stylesheet is used, it wouldn't be able to react nicely to dark/light mode change, anyway, so I don't think there's anything lost here.

edit: ah, the above can't work as-is, because dolphin wants to style the balloonTip lightly if the system theme is dark (which is probably, itself, an error).

See also https://www.kdab.com/say-no-to-qt-style-sheets/

shuffle2 avatar Apr 25 '23 05:04 shuffle2

because dolphin wants to style the balloonTip lightly if the system theme is dark (which is probably, itself, an error).

That was deliberate - it was done that way for contrast. https://dolphin-emu.org/blog/2020/12/10/dolphin-progress-report-october-2020/#50-13163-remove-description-box-in-graphics-tabs-and-use-custom-tooltips-instead-by-iwubcode

MayImilae avatar Apr 25 '23 06:04 MayImilae

To summarize the problem: Any Qt widget which has setStyleSheet called on itself or a parent will not be re-styled automatically when a user changes the Windows dark/light mode while dolphin is open. Someone will need to investigate a proper fix for this (possibly, we can trigger the styles of widgets using stylesheets to be re-evaluated or something, idk). But since it is only a bug that occurs if mode is changed while dolphin is open, I think it's OK to do in future PR.

Notably the Qt example widgets\itemviews\frozencolumn has the same problem. So I think it's inherent to style sheets and not a dolphin specific thing. Here's the example started in light mode and changed to dark: image compared to started in dark mode (the France header has retained Light style): image this is the related code:

frozenTableView->setStyleSheet("QTableView { border: none;"
                                     "background-color: #8EDE21;"
                                     "selection-background-color: #999}");

edit: updating the example to re-set the same style sheet upon QStyleHints::colorSchemeChanged does fix the problem:

void FreezeTableWidget::onColorSchemeChanged(Qt::ColorScheme colorScheme) {
      frozenTableView->setStyleSheet(frozenTableView->styleSheet());
}

It seems like a bit of spaghetti to implement though, as any class calling setStyleSheet on some widget it creates, should now also take care to connect to QStyleHints::colorSchemeChanged and re-set the style sheet. I guess it sort-of makes sense, if you want to take the opportunity to change the style sheet in reaction to ColorScheme changing, however the default behavior should just do that :/

p.s. does the same problem occur on other OS?

shuffle2 avatar Apr 25 '23 06:04 shuffle2

Out of curiosity, now that PR #11774 was merged, I tried running the most recent build of this PR with argument -platform windows:darkmode=2 (which became the default value for that parameter on Qt 6.5) and Fusion dark renders differently?

image

It doesn't apply the accent color (Qt 6.5 seems required for that) and the background is pitch black, however the tab background at least remains consistent. Any thoughts?

(by the way, the same theme gets applied on current master if you add arguments -platform windows:darkmode=2 -style fusion)

mbc07 avatar Apr 25 '23 21:04 mbc07

If that’s not applicable to qt6.5, then we don’t really need to talk about it.

however, personally I think it looks worse. The dark scheme is intended to be normal ui but grey. What your picture shows is something closer to black and widgets which should be slightly different color so as to delineate themselves from the background are the same color as the background so it’s flat looking.

From the blog, darkmode=2 means the colors are read from the system. Hence the use of grey, which is the proper and expected color! So I assume you’re mainly not liking the usage of the accent color?

again, I think the accent behavior looks fine. I would rather just do the default qt thing so it doesn’t need special casing across different OS , os versions, qt versions, user os and dolphin configurations, and maintains the default into future qt versions. Afaict the accent behavior is described on the qt dark mode blog. It sounds like they’ll tweak it in the future.

Another point: consider that ui elements in things like the Windows Settings app do use the accent color for things like sliders, switches, etc. So I'm not really sure why you don't like it so much 🙃 It is suboptimal that the accent isn't used consistently between light/dark modes in Qt, but ideally this is something that Qt will just fix themselves.

Having said that, someone else is free to make a separate pr doing the color tweaking.

shuffle2 avatar Apr 25 '23 22:04 shuffle2

I'm OK with the accent color usage, the only thing from Fusion dark on Qt 6.5 that's bugging me out is the gray background of tab contents and buttons, which is too "light" (e.g. the contrast between them and the window background is a bit too high). Fusion light, on the other hand, looks good out of the box, no tweaking required.

Back to Fusion dark, that's what I think needs tweaking, not to make the tab content and buttons the same #1E1E1E color of the windows background, but to make them a little more dark than the current #4B4B4B they default to...

(the pitch black screenshot I sent earlier was just to show that having closer colors between the window background and the tab content looks overall more uniform IMO -- again, they don't need to be the same, but should at least be more close to each other)

mbc07 avatar Apr 25 '23 23:04 mbc07

Here, I tweaked the background colors a bit with an user style sheet to show what I mean. First screenshot is default Fusion dark on Qt 6.5, second one is with slightly darker tab backgrounds (and related elements):

default tweaked

At least on the stylesheet, the affected elements seems all derived from the same base color, tweaking that color in the source would very likely fix that everywhere else. It doesn't even have to be a hardcoded color, we could just take the default base color returned by Qt and multiply it by 0.75 or other suitable factor to get it slightly darker...

(other interesting bit is that the preview feature of Qt Designer 6.5 with the default Fusion dark style uses a more uniform palette as well -- the tweaks I did in the mockup above comes mostly from that -- unsure why the final, compiled application, look more contrasty with the default Fusion dark style)

mbc07 avatar Apr 26 '23 01:04 mbc07

i understand what you're getting at, but i still think it should be another pr (which I won't be involved in)

and that's just because:

  • i'd like move to fusion before updating to qt6.5
  • i personally dont have any good input for the color tweaking anyway - i think it looks fine as-is and would prefer it stays default, but don't really care if someone who feels strongly about it makes it how they like

shuffle2 avatar Apr 26 '23 04:04 shuffle2