Add option for editor to follow system theme and accent colors

[Joseph-DiGiovanni]

As suggested by godotengine/godot-proposals#5678, I've implemented the following:

• Automatic editor theme switching on system theme change.
• Optional accent color matching (Windows and MacOS only).
• Individual theme preset selection for light and dark mode.

Before going into more detail, here are the results:

Screencast.from.2023-12-29.08-19-26.webm

I've only tested it on Linux and Windows, but it should work just fine on any OS. The editor settings now feature an option to have the editor theme follow the system dark mode settings. The light and dark themes default to 2 new presets called "Custom Light" and "Custom Dark" which can be configured individually to your liking, but any existing preset may be used (e.g., Solarized Light and Solarized Dark). You may also select a light and dark theme resource file which will override the function of the existing "Custom Theme" resource when the new presets are used.

I also took the opportunity to refactor a significant portion of the editor theme creation code for improved readability and to better facilitate adding these features.

The only questionable bit is my method of detecting the switch to and from dark mode. I'm currently storing the previous values of the dark mode and accent color check in EditorNode and checking for changes on process. I am absolutely open to any suggestions on how to better implement this.

[Calinou]

I've only tested it on Linux and Windows, but it should work just fine on any OS.

Note that the Android and web editors could also benefit from this feature, but this can be left for a future PR. This PR only touches the editor side and not any OS-specific code which was already previously added anyway.

[Joseph-DiGiovanni]

I used a timer node as @Calinou suggested checking for system theme changes only once per second.

I also changed the method of triggering the theme update from manually calling _update_theme() to simply changing the relevant settings as the user would, relying on the editor_settings_changed signal to call _update_theme() so it doesn't run twice. This eliminates the extra second for the theme to completely change automatically as seen in the video above.

Thanks for the feedback!

[Calinou]

Tested locally, it works as expected. Light/dark theme switching works on Fedora 39 KDE (X11).

There are some usability quirks though:

• Changing the editor theme preset listed at the top of the editor settings has no effect if Follow System Theme is enabled. This is because the theme preference takes priority over the theme preset.
• The default values for the dark/light theme presets are Custom Dark and Custom Light, but I'd expect them to be Default and Light respectively.
• With this PR, we now have 3 custom themes defined: Custom Dark, Custom Light and Custom:
• Black (OLED) is listed below the custom dark/light options, but it should be listed above.

To resolve these issues, I propose removing the existing (legacy) theme preset entirely and keeping only the Light Preset and Dark Preset themes. The theme in use is set by an enum property called Theme Mode that has the values Automatic (= follow system theme), Light and Dark (default). The Default theme is renamed to Default Dark and the Light theme is renamed to Default Light.

If you customize an existing theme, it switches the currently active theme mode to Custom and keeps the other one intact.

These changes may slightly break compatibility with existing themes, but they should be easy to update with a search-and-replace operation. Also, installing custom theme presets from a file is currently not really a supported use case, as it involves overwriting all your editor settings.

I'm aware this is a lot of work to get right, but it's important so we can ensure usability remains good on this aspect 🙂

[Joseph-DiGiovanni]

I agree with your proposals, I didn't want to step on anyone's toes which is why I implemented them as separate presets.

installing custom theme presets from a file is currently not really a supported use case, as it involves overwriting all your editor settings.

The option for custom theme files is there in the stable release. Are you implying it doesn't work and should be removed?

[Joseph-DiGiovanni]

I'm happy to report I was able to implement it almost exactly as you described. It will also read the old preset (even custom ones!) and set the new theme options accordingly. No users should notice a difference unless they open the theme settings.

Screencast.from.2024-01-02.00-27-28.webm

[Calinou]

Thanks! I'll test this ASAP.

The option for custom theme files is there in the stable release. Are you implying it doesn't work and should be removed?

Syntax highlighting themes can be shared as .tet files, but editor theme presets can't. Only custom editor themes (which have to overwrite everything or replicate the entire editor theme in some way) are supported in the form of .res/.tres files.

[Calinou]

It works great now, exactly as I expected. Thanks a ton for following through 🙂

[YuriSizov]

This needs to be reworked following the recent editor theme generation changes.

But more importantly, I find this solution to be overly complicated. I don't see the rationale for duplicating every theme property and having one set for each of dark and light modes. I don't expect people two change their system colors often, especially to change it often between light and dark modes.

Besides, on Linux at least we will probably want to try and pick the most fitting preset from our other options, not just our default light and default dark.

So all we need to add is a new "System" preset which will automatically resolve into either the Default (dark) or the Light preset on Windows and macOS. And on Linux, if we can, it can resolve to a more specific color mode, since this has been requested by users, or default to the same logic as on Windows and macOS.

[Joseph-DiGiovanni]

I don't see the rationale for duplicating every theme property

The custom themes need to be stored somewhere, so at minimum you will always need 2 sets of theme settings, and the original settings which are considered the "active" theme settings can be changed at any time and work as the user expects. It really is the simplest possible solution for the user.

So all we need to add is a new "System" preset which will automatically resolve into either the Default (dark) or the Light preset

What you suggest is the simplest implementation but you lose any ability to actually customize the theme.

[YuriSizov]

The custom themes need to be stored somewhere

What custom themes? Why? We don't do that right now, if you select a preset, all your custom settings get reset.

What you suggest is the simplest implementation but you lose any ability to actually customize the theme.

Color presets only affect 4 properties, which are expected to follow the preset. The rest you can customize.

[Joseph-DiGiovanni]

Why? We don't do that right now

Because we never had to switch between 2 different themes before this feature. Otherwise any customization would be lost the moment the system theme changes and it changes the preset. This would be very annoying for people who switch between light and dark mode throughout the day.

Color presets only affect 4 properties, which are expected to follow the preset. The rest you can customize.

Those 4 properties are arguably the most important part of the theme customization for the average user and should be separately configurable for dark and light mode.

[YuriSizov]

I think this is excessive. If you want the editor to follow your system theme, it is expected that editor theme's color properties will be updated automatically. If you want to customize the colors to fit your own preference, then you no longer follow the system theme.

People also don't generally update their system color mode often if at all, so most of what you suggest seems like a non-issue to me. As in, if you decide to customize your theme and then change the system preference at some point, it's not going to be an issue to adjust your editor settings once again. Doing this whole setup just for that rare occasion is too much and makes things much more convoluted to maintain.

[Joseph-DiGiovanni]

If you want the editor to follow your system theme

It doesn't follow the system theme as much as simply acknowledge the dark/light mode setting. Besides the original title of the PR the settings in the current state of the PR make this very clear. As a side note, the Default theme doesn't match any major OS so Gray would be more appropriate if this was the goal.

People also don't generally update their system color mode often

MacOS has an auto option when selecting the dark/light system preference that changes with the time of day. There are applications that add this feature to windows and linux as well.

makes things much more convoluted to maintain.

I think I addressed this pretty well. The "preset" option has just been replaced with a dark preset and light preset. The fact that there are a few clearly labeled duplicated settings only used by the theme generator doesn't realistically add anything extra to maintain. Not having them results in the total loss of customization. I think it's a fair trade off.

[YuriSizov]

MacOS has an auto option when selecting the dark/light system preference that changes with the time of day. There are applications that add this feature to windows and linux as well.

This is an interesting use case, but it's still a niche application for tripling the amount of settings, some of which don't even need to be associated with the color mode.

We can discuss the need to customize base and accent color on top of following dark/light polarity of the system theme in a follow-up. For the initial feature this is too much granularity.

As a side note, the Default theme doesn't match any major OS so Gray would be more appropriate if this was the goal.

If we can read values for the base and accent colors from the OS, we can automatically set them, like in case of Windows accent color. This doesn't need to be explicitly exposed as a setting, unless someone requests it. It can just work.

That's the main problem. This PR assumes a lot of customization is needed before we even implemented the base mechanic for following the system theme.

---

PS. I also noticed right now that you automatically update the editor theme when the system theme changes (and with a hack for some reason). This shouldn't be done. Regenerating the entire theme takes a noticeable amount of time. We don't want users to experience hitching in the middle of their work.

I suggest replacing it with some notification which can be clicked to confirm the change. But we also can simply keep the theme until the user restarts the application.

[Joseph-DiGiovanni]

We can discuss the need to customize base and accent color on top of following dark/light polarity of the system theme in a follow-up.

That's probably a good idea. I'm working on more basic PR right this moment.

If we can read values for the base and accent colors from the OS

We can only read accent color. I don't think reading any kind of "base" color of the OS is even possible.

you automatically update the editor theme when the system theme changes
We don't want users to experience hitching in the middle of their work.

As a user of auto dark/light mode, even I will concede every app flipping colors automatically is mildly jarring. This feature would not be enabled by default and if the user manually enabled it the second of delay once a day with an obvious reason for the hitch is frankly not an issue to me.

In reference to the hack in my implementation it was required due to a bug in the way settings changes were detected so the theme change was called twice (You can see this in the original PR video). I haven't checked but hopefully your PR has fixed that. In any case if I'm just modifying the preset value as you suggest this should thankfully not be necessary.

[YuriSizov]

In reference to the hack in my implementation it was required due to a bug in the way settings changes were detected so the theme change was called twice (You can see this in the original PR video). I haven't checked but hopefully your PR has fixed that.

Let me know if you need any help with that, here or on Rocket.Chat 👍

[bruvzg]

We can only read accent color. I don't think reading any kind of "base" color of the OS is even possible.

What's a base color? Control background color? It should be possible to get it the same way we are reading accent color.

It should be possible to get callback for the theme change (at least on Windows and macOS).

Edit: I'll make PR for system theme change callback and system control background color a bit later today.

[YuriSizov]

What's a base color? Control background color?

Let's say yes. Since we use it as a base for other colors, we may want to adjust it (because we have darker and lighter variations). But anything you can get from the OS should be useful.

[Joseph-DiGiovanni]

I've updated the PR to what I would consider the most basic implementation which we can hopefully further build upon later. I appreciate the input and assistance from everyone involved!

Note there is some kind of bug with inspector background colors not updating immediately. As far as I can tell this has nothing to do with my code so I'll leave it as is to be fixed at a later date. The theme update also seems to trigger the settings change notification causing _update_theme() to be called twice but this seems far simpler to solve and I may update the PR if I track it down.

It should be possible to get it the same way we are reading accent color.

I have my doubts about this. However it is dark mode is being detected is probably all that is provided from the OS since there is no way to further customize the "base" or "background" color on any platform outside of linux (which even then is likely not standardized in any useful way).

It should be possible to get callback for the theme change

This would be nice, however unless its implemented on every platform we will still need to rely on checking the dark mode state periodically.

[Joseph-DiGiovanni]

I've tracked down the settings changed signal to the set_initial_value() method called many times in the editor theme manager. I talked privately with @YuriSizov about it and it seems it is unintended behavior, however the inspector UI elements wrongfully still rely on this signal so they don't update properly without it. Until that is fixed I'd consider it an unrelated minor bug.

[bruvzg]

PR for callback and control bg color - #87384, base color might need some testing (some other color name might be better), but unfortunately immersive colors aren't documented at all.

[akien-mga]

IIUC this could be redone now to make use of #87384.

[Joseph-DiGiovanni]

I have implemented base color matching from #87384 and cleaned up my code a bit. I've tested it on Windows 11 and it works as expected. Thanks! Although as stated previously, without system theme change detection that triggers the callback being implemented on every platform it makes more sense to just manually check for system theme changes as was done previously.

While this probably should have started as a draft I appreciate all the help that has been given and believe this should be considered for merging. Thank you.

[Calinou]

Tested locally on Fedora 39 KDE X11, it works as expected.

However, the theme preset switching takes a fair amount of time as it requires two updates for some reason (notice how the Tree background color is initially not updated). This doesn't occur when manually switching theme presets as shown near the end of the video below.

simplescreenrecorder-2024-02-15_18.42.13.mp4

I'm sad to see the ability to choose different presents go away (I'd use Light + Black (OLED) personally), but it makes sense in the interest of simplicity.

[Joseph-DiGiovanni]

the theme preset switching takes a fair amount of time as it requires two updates for some reason

This is a bug unrelated to my code where any theme update not caused by a manual settings change signal actually emits a setting change signal during theme generation, resulting in 2 theme updates. This is easily fixed by making EditorSettings::set_initial_value not emit a signal but the inspector backgrounds do not update without it (which is why they only change on the second update).

These issues didn't present themselves until now because there was previously no way for a theme change to occur without changing settings manually. I'd be happy to open an issue with these details once this gets merged as the fix goes beyond the scope of this PR.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉