Fix various Dark/Light Theme inconsistencies

[bhennion]

Fixes #4336 Fixes #5422 Fixes #5445

This PR changes the "Dark theme" setting to become a combo box with 3 options: USE_SYSTEM, FORCE_LIGHT or FORCE_DARK.
With this, the gtk theme is always consistent with the icon set.

There are themes out there for which this does not work: e.g. breeze-gtk does not seem to allow the app to control its variant (dark/light) via gtk_settings::gtk-application-prefer-dark-theme.
For those users, the bug should be reported to the theme's maintainer or packager maybe.

[rolandlo]

This looks like a great attempt to get dark/light theme working. Unfortunately even on a widely popular distro like Ubuntu it doesn't work (with the theme variant). In my case I get:

(which is kind of self-contradicting) and I can't change that from the Ubuntu settings to Theme name: Ubuntu-Yaru-dark, Theme variant: dark. If I want to change the setting I have to edit ~/.config/gtk-3.0/settings.ini

I'm wondering if we could use the color-scheme gsetting instead to determine the theme variant like the AppImage does, see https://github.com/linuxdeploy/linuxdeploy-plugin-gtk/pull/58/files.

[rolandlo]

Or maybe consider the system preference as "dark" when the theme name contains the substring "dark" or the theme variant is dark.

[bhennion]

Yeah, I don't quite understand why it works with the Adwaita theme and not with others.
We could try editing the theme name itself. I'll give in it another try.

[bhennion]

Does it work for you if your session-wide theme is set to yaru-purple (and not yaru-purple-dark)?
It works for me. There seems to be conflicting ways of setting the theme to dark mode.

[rolandlo]

Well yes, with Yaru-purple and gtk-application-prefer-dark-theme=1 in ~/.config/gtk-3.0/settings.ini (or the "Force dark" option in the Xournal++ settings) it works. The trouble is I can't follow the system theme and simply use the Dark style switch here:

[bhennion]

Now it should be working for themes like yaru (who have a -dark variant).

[rolandlo]

Testing on MacOS I found that with the default theme (Adwaita) it works fine with "Force dark"/"Force light". The "Follow system theme" doesn't work (and I get "Adwaita", "light" all the time). Also when running with "GTK_THEME=Adwaita:dark" there is an inconsistency between icons and (dark) background when "Force dark theme" is set. Still it's a nice improvement over master. By the way the gsettings color-scheme is not set on MacOS. I can retrieve the setting via

osascript -l JavaScript -e "Application('System Events').appearancePreferences.darkMode()"

on MacOS.

[bhennion]

I think the Adwaita:dark can be fixed easily. The follow system theme issue, I have no idea. What's the terminal output is this case?

[rolandlo]

I think the Adwaita:dark can be fixed easily. The follow system theme issue, I have no idea. What's the terminal output is this case?

I always get Theme name: Adwaita, Theme Variant: light when following the system theme, regardless of whether I use GTK_THEME=Adwaita:dark or not. When using that option everything except the icons use dark theme.

[rolandlo]

One question: Is it possible (in Linux) to react to theme (and dark-preference) changes, so that when a user changes the theme the appearence of Xournal++ changes automatically (without opening the preferences)?

[bhennion]

I think the Adwaita:dark can be fixed easily. The follow system theme issue, I have no idea. What's the terminal output is this case?

I always get Theme name: Adwaita, Theme Variant: light when following the system theme, regardless of whether I use GTK_THEME=Adwaita:dark or not. When using that option everything except the icons use dark theme.

Hmm... So we probably can't get the dark/light information from the GtkSettings content on MacOS. Don't know where else to look. Do you know of a way to get access to the color-scheme?

[bhennion]

One question: Is it possible (in Linux) to react to theme (and dark-preference) changes, so that when a user changes the theme the appearence of Xournal++ changes automatically (without opening the preferences)?

Probably, lemme try.

[bhennion]

One question: Is it possible (in Linux) to react to theme (and dark-preference) changes, so that when a user changes the theme the appearence of Xournal++ changes automatically (without opening the preferences)?

Probably, lemme try.

This is now done. I also added a bit more terminal output to see if something can be done on MacOS.

[rolandlo]

One question: Is it possible (in Linux) to react to theme (and dark-preference) changes, so that when a user changes the theme the appearence of Xournal++ changes automatically (without opening the preferences)?

Probably, lemme try.

This is now done.

That's fantastic. Our users will love it! It works great on Ubuntu 24.04.

I also added a bit more terminal output to see if something can be done on MacOS.

I will test it when I'm back home (on Saturday or Sunday).

[rolandlo]

From https://gitlab.gnome.org/GNOME/gtk/blob/90d84a2af8b367bd5a5312b3fa3b67563462c0ef/gtk/gtksettings.c#L1567-L1622
when the css provider loads a theme, it first checks the environment variable GTK_THEME for determining theme name and theme variant. When it is not set it uses the "gtk-theme-name" and "gtk-application-prefer-dark-theme" properties or fallsback to the default theme. These two properties are not updated by the GTK_THEME variable.

So when we want to handle the case where the user (or e.g. a script in the AppImage) sets GTK_THEME we should first read that variable as well and use it to decide theme name and theme variant.

[bhennion]

From https://gitlab.gnome.org/GNOME/gtk/blob/90d84a2af8b367bd5a5312b3fa3b67563462c0ef/gtk/gtksettings.c#L1567-L1622 when the css provider loads a theme, it first checks the environment variable GTK_THEME for determining theme name and theme variant. When it is not set it uses the "gtk-theme-name" and "gtk-application-prefer-dark-theme" properties or fallsback to the default theme. These two properties are not updated by the GTK_THEME variable.

So when we want to handle the case where the user (or e.g. a script in the AppImage) sets GTK_THEME we should first read that variable as well and use it to decide theme name and theme variant.

Nice find!!
I just pushed a commit doing that

[rolandlo]

From https://gitlab.gnome.org/GNOME/gtk/blob/90d84a2af8b367bd5a5312b3fa3b67563462c0ef/gtk/gtksettings.c#L1567-L1622 when the css provider loads a theme, it first checks the environment variable GTK_THEME for determining theme name and theme variant. When it is not set it uses the "gtk-theme-name" and "gtk-application-prefer-dark-theme" properties or fallsback to the default theme. These two properties are not updated by the GTK_THEME variable.
So when we want to handle the case where the user (or e.g. a script in the AppImage) sets GTK_THEME we should first read that variable as well and use it to decide theme name and theme variant.

Nice find!! I just pushed a commit doing that

It works nicely on Linux. I will test on MacOS and Windows as well when I'm home.

[rolandlo]

One question: Is it possible (in Linux) to react to theme (and dark-preference) changes, so that when a user changes the theme the appearence of Xournal++ changes automatically (without opening the preferences)?

Probably, lemme try.

This is now done. I also added a bit more terminal output to see if something can be done on MacOS.

The terminal output doesn't seem to help much. In dark mode I get:

However the following code block seems to work to detect the system dark preference (tested on MacOS Catalina)

    bool dark = false;
    CFStringRef interfaceStyle = (CFStringRef)CFPreferencesCopyAppValue(CFSTR("AppleInterfaceStyle"), kCFPreferencesCurrentApplication);
    if (interfaceStyle) {
      char buffer[128];
      if (CFStringGetCString(interfaceStyle, buffer, sizeof(buffer), kCFStringEncodingUTF8)) {
        std::string style = buffer;
        g_message("Interface stlye: %s", style.c_str());
        if (auto pos = style.find("Dark"); pos != std::string::npos) {
            dark = true;
        }
      }
    }

It uses the CoreFoundation C API, so include the header file via

#include <CoreFoundation/CoreFoundation.h>

[rolandlo]

For Windows there are docs here how to determine the system dark mode preference: https://learn.microsoft.com/en-us/windows/apps/desktop/modernize/apply-windows-themes#know-when-dark-mode-is-enabled

[bhennion]

I don't quite feel confident I can cook up something without being able to test it directly. Maybe you could write a commit for MacOS and one fo Windows and either append them to this PR or to a follow-up?

[rolandlo]

I don't quite feel confident I can cook up something without being able to test it directly. Maybe you could write a commit for MacOS and one fo Windows and either append them to this PR or to a follow-up?

I have added a commit for MacOS. It works for me (on MacOS Catalina).

Turns out the Windows counterpart is more troublesome. The winrt header

#include <winrt/Windows.UI.ViewManagement.h>

must be included, which is not available by default. After installing mingw-w64-x86_64-cppwinrt the compiler complains that C++/WinRT requires coroutine support, which is currently missing and recommends to enable C++20 in the compiler. I don't think we want to do that at this moment. So Windows users will have to use "Force dark" or "Force light" in the settings and can't just follow the system theme.

[bhennion]

I don't quite feel confident I can cook up something without being able to test it directly. Maybe you could write a commit for MacOS and one fo Windows and either append them to this PR or to a follow-up?

I have added a commit for MacOS. It works for me (on MacOS Catalina).

Turns out the Windows counterpart is more troublesome. The winrt header

#include <winrt/Windows.UI.ViewManagement.h>

must be included, which is not available by default. After installing mingw-w64-x86_64-cppwinrt the compiler complains that C++/WinRT requires coroutine support, which is currently missing and recommends to enable C++20 in the compiler. I don't think we want to do that at this moment. So Windows users will have to use "Force dark" or "Force light" in the settings and can't just follow the system theme.

Thanks for the MacOS commit! As far as Windows is concerned, that fine by me.
Should we merge this then?

[rolandlo]

Should we merge this then?

I think so, yes.

[bhennion]

Thanks a lot @rolandlo!