-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Automatic light/dark mode switching of open STC plot windows #9182
Comments
This one to me seems like it will be a lot of work and/or potentially fragile (probably need to hook into some system notification thing) for very small added benefit (since theme changes are generally rare). |
Yeah, no idea, I thought maybe Qt can trigger a callback upon theme change, no idea. But the light title bar … would be super nice if we could make this one dark ;) |
cc @marsipu |
plus the STC views are typically not long-lived and can simply be recreated, unlike e.g. a text editor or terminal window, which you may want to keep open for days or weeks. |
Is the titlebar white when you start in darkmode? To dynamically change with the mode, one could maybe setup a QTimer and check periodically for a change from darkdetect. But it is maybe questionable as you already said, if it was worth it (and I don't know @GuillaumeFavelier, how that would influence the performance of for example the BackgroundPlotter). I read some forum-entries about the titlebar, it seems that the titlebar-color is not influenced by Qt but by the OS. There seem to be a workaround in cpp for MacOS, but I don't know if that could be translated to (MNE-)Python. And I found none for Windows or Linux. But there seems to be another possibility by replacing the whole window-surroundings including the titlebar. as qtmodern does it. If we'd be willing to add another dependency, this may be the best solution, which would also unify the look across platforms as a side-effect. What do you say? |
Take a look how I do it in MNELAB. It works on macOS and Linux, but unfortunately Qt doesn't support that on Windows (yet). |
Nice, what do you do? Is it the part with the NSBundle in main.py? |
No, I'm catching a This event is triggered whenever the OS theme changes. Therefore, you can use it to change themes on the fly, but as I said not on Windows. However, maybe this isn't that bad, because Windows doesn't have a dynamically changing dark/light theme (it's more like a setting you choose once). |
However, since you're using style sheets this will probably not work. I think the white title bar is also due to style sheets. |
Nice thank you, maybe it would work if we connect the Stylesheet-Change with your PaletteChange-Event? |
It's worth a try.
Exactly, I only change the icons manually. On Windows this doesn't work unfortunately, mainly because Qt still does not provide a native dark theme for Windows (not even Qt6).
They do (except for the title bar apparently), but style sheets never look like a native app (although I think it's very close). |
Ahh if it's based on some Qt event then ignore what I said about being much work or fragile, it should be neither of those. And even detection of the style name is two lines of Foundation code which seems okay. Personally my vote would be to use Qt's builtin dark support/styling as much as possible (OSX via icon change) and resort to the style sheet method where Qt does not support it or we don't know how to do it more natively (Windows and Linux). |
👍 but native styling works on both macOS and Linux, so style sheets should only be required on Windows. |
Even better! |
I wonder, why dark mode was not automatically applied already on MacOS and Linux before we added the stylesheets. |
I just tried this with MNELAB, and indeed it seems like 5.12 has not fully implemented theme switching. Only the title bar and icon bar adapt with the theme, but the main widget and status bar remain with the theme they had when I started the app. Therefore, it might be a good idea to use at least 5.15. There's already an open issue: conda-forge/pyqt-feedstock#98 |
Qt 5.14+ VTK does not render in HiDPI EDIT: and the controls / clicking are broken, which to me is (much) more important than properly setting the colors of the Qt controls / icons / theme: https://gitlab.kitware.com/vtk/vtk/-/issues/17953 If theme switching works on 5.13 then we can use that, though, since VTK works properly with that. Also on Linux I've never had my Qt be anything other than light despite using a dark theme (and latest PyQt5, 5.15+), but maybe that's because I use a GTK-based front-end (Gnome-shell and/or cinnamon) rather than KDE. @cbrnr where have you seen the Qt controls be automatically dark on Linux? |
Re Qt 5.14+, that's really a pity. Being stuck with an old version will only make things worse over time, so I really hope that VTK fixes this bug (BTW this is another reason why I would want to avoid VTK for a raw viewer and rather stick to pure Qt). I will test if theme switching works with Qt 5.13, but this would mean that we'd have to mix pip installs with conda, which is never a great idea. I use GNOME on Arch Linux (so very likely the latest versions of everything), and I'm pretty sure that it works. However, I might be wrong because it's been a while since I've worked on my Linux box (due to WFH), but I can test it next week. |
Yes, theme switching on macOS works with Qt 5.13! |
There is some suggestion that to trigger it you have to call setStyle, so maybe MNELAB always calls this, and if we do it (on 5.13+) it will work automagically on Qt and/or Linux as well: |
I'm not calling |
Ahh I thought you did, guess that's not it. Unfortunately for me even on Adaiwata-dark and Qt 5.15.3 I can perhaps investigate on my macOS laptop later. Presumably there is some important difference between what we do for |
I'd be very interested in anything you find out! It's weird that you always get a light theme, but maybe that's how it is. I'll let you know as soon as I get to my Linux box! |
On PyVistaQt, the https://github.com/pyvista/pyvistaqt/blob/master/pyvistaqt/window.py#L29-L34 I think it would be possible to open a PR there to emit a |
Can you not override the |
It would be hacky since we don't design the window itself in mne but just borrow the one created by |
Hm, to me reimplementing |
Also, I think you can connect the |
|
Another solution would be to drop
I don't understand, do you mean directly without intermediate signal? |
Or derive from
Yes. Wouldn't that work? You write a slot and connect it to this signal. |
Which signal though? By looking online, I found only some enums like |
|
To me, it's not a signal, just an enum. You use it in MNE-lab to filter events and process theme change directly. What I suggest in #9182 (comment) is to emit a signal instead, something like: def event(self, ev):
"""Catch system events."""
if ev.type() == QEvent.PaletteChange: # detect theme switches
self.palette_change.emit() And this will turn the event into a signal that can be connected. |
You're right, I mixed up events and signals. I still think translating In addition, what about implementing a custom In any case, I'm not familiar with PyQtVista, so maybe you can just go ahead and modify the main window upstream. |
This is a cleaner design for sure. It also gives more leeway.
I did not even know about this, it sounds really useful 😅 |
I would look into the automatic theme support (instead of using CSS) on macOS. Having custom signals and event handling functions just to take care of when a user switches a theme during use of these usually transient windows is starting to sound like more work to implement and maintain and think about than it's worth at the user end. |
+1 @ what @larsoner just said. Unless somebody is really willing to put that much effort and time into this -- which I would appreciate, but I can live without it. We've made a great step forward already! |
@larsoner you were right, Qt doesn't seem to support dark theming on Linux 😞 (only the title bar changes). |
Although I did manage to get it to use a dark theme with the |
Now that pyvista/pyvistaqt#152 is in, we can subclass |
Nice! Just to make sure I didn't miss anything, this still works only on macOS, right? I'd also prefer either processing the event directly (as I'm doing in MNELAB) or emitting a signal and connecting it to a method that does the processing. I don't think it makes a big difference. I'd not write a custom |
Good question, I will try a simple code snippet on my KDE desktop to find out 👍
I think it's just a misunderstanding. On
To clarify, I think we would end up doing the same as |
Nope, it did not work for me unfortunately. I'm really surprised considering everything KDE is basically Qt under the hood. output.mp4The rough code snippet I used: from qtpy.QtCore import QEvent
from qtpy.QtWidgets import QApplication, QMainWindow
class MainWindow(QMainWindow):
def event(self, ev):
"""Catch system events."""
print(ev)
if ev.type() == QEvent.PaletteChange: # detect theme switches
print("theme change")
return super().event(ev)
if __name__ == "__main__":
app = QApplication([])
win = MainWindow()
win.show()
app.exec() |
@GuillaumeFavelier OK, now I got it! I also confirmed that your example is working on macOS. I think that's unfortunately still the only OS, it doesn't work on Windows either. |
On macOS, apps that support light and dark themes switch between the two if the system theme is changed, without requiring a restart. It would be good if our STC plotters could do the same.
Screen.Recording.2021-03-22.at.21.18.53.mov
In this screencast you can also see that the color of the title bar is not correct in dark mode, but that might be a Qt issue?
cc @GuillaumeFavelier
The text was updated successfully, but these errors were encountered: