Set title bar light or dark on Windows based on theme

[andriiryzhkov]

Fixes #17392

This implementation is based on this approach from GIMP https://gitlab.gnome.org/GNOME/gimp/-/issues/9646.
It uses DwmSetWindowAttribute() to set dialogue title bars to either dark or light mode on Windows. This is done by checking the dialogue's background color and setting to dark mode if it's below a threshold (currently 0.5, 0.5, 0.5).
The function dtwin_set_titlebar_color(GtkWidget *widget) needs to be called just before each dialog window in darktable is shown.

[ralfbrown]

The code would be much cleaner if you implemented a dtwin_set_titlebar_color for every architecture (as a no-op on everthing but windows). We've worked hard over the past few years to remove conditional compilation from mainline code and hide it inside macros (e.g. DT_OMP_FOR) or utility functions, because that hiding makes the code more maintainable - you only need to change one definition rather than every location where it is used.

I suggest putting the conttents of win/dtwin.h inside an #ifdef _WIN32 and adding an #else with

#define dtwin_set_titlebar_color(widget)

Then you can drop the #ifdef guards around all calls to the function and all #includes of dtwin.h.

[andriiryzhkov]

@ralfbrown, Thank you for the good advice.
I've made the changes you asked for. I also moved the function to dedicated win/titlebar.c so it does not interfere with existing code and I can safely drop the #ifdef guards around all #includes of win/titlebar.h. I also reviewed the function code and made some small improvements there.

[dterrahe]

Rather than adding a function call for each and every time a gtk window gets shown (and making sure this is done for all new ones in future), you could override/extend the default handler for the show or realize signal (and then chain to the original handler at the end of your callback).

This g_signal_override_class_handler approach is used once already in dt to override the query-tooltip behavior to add shortcuts, where it applies to all widgets.

Here you probably want to do something like

g_signal_override_class_handler("realize", gtk_window_get_type(), G_CALLBACK(_window_set_titlebar_color));

in dt_gui_gtk_init and at the end of _window_set_titlebar_color call g_signal_chain_from_overridden_handler

Of course you only override the handler under windows and a separate source file for just the handler _window_set_titlebar_color is somewhat overkill if it only needs to be referenced from gui\gtk.c

[dterrahe]

In case that wasn't clear, its implemented in b9c7846

I've overridden configure-event so the dark mode will get updated after a theme change for any open window as soon as it is resized/moved.

[TurboGit]

@andriiryzhkov : Given the simplicity of @dterrahe proposal in the mentioned commit I suppose we should go this way. Have you double checked that this works on your side?

[andriiryzhkov]

@dterrahe: I love your approach; thank you for the advice.
@TurboGit: I double-checked the @dterrahe proposal and can confirm that it works. Updates are committed.

The only edge case left is that after a theme change, the title bar will be updated only when any open window is resized or moved. I don't see it as a critical problem, but please advise if you know how to force updating the title bar right after the theme change.

[andriiryzhkov]

Improved functionality a little bit to attempt to set the title bar color to darktable background color. If failed, it tries to set the title bar light/dark (as previously implemented).

I believe it looks even nicer.

[dterrahe]

if you know how to force updating the title bar right after the theme change

For the main window, you could explicitly update it in the theme loading code. If you want "all" open windows to update (realistically this would be main+prefs+second preview) then you could override the style-updated signal handler. Unfortunately that one doesn't seem to get called when the window is first opened so you'd need to override both handlers (or switch "configure-event" to something like "realize", "map" or "show", I haven't tested which work reliably. These all have the same function signature as "styled-updated" so you wouldn't need two different callbacks...).

[andriiryzhkov]

I found that a combination of configure-event and style-updated overrides fit the need.

[dterrahe]

d6a50a8 works and updates main and second preview window titlebars immediately when changing the theme in preferences (upon pressing "save CSS and apply").
But titlebar of the prefs dialog itself doesn't seem to respond to the style-updated signal. Don't know why. Closing prefs and reopening does work of course.

[dterrahe]

I found that a combination of configure-event and style-updated overrides fit the need

be careful; those signals have different arguments and return types so using the same call to g_signal_chain_from_overridden_handler might (?) cause problems. So you should really have separate callback functions in that case. Besides, if you are monitoring style updates, you don't also need to receive configure changes for every move of a window. The combination of map and style-update works too and is simpler.

[andriiryzhkov]

Indeed, the preferences dialog title bar responds to style-update when selecting theme, but ignores CSS tweaks.

[dterrahe]

Indeed, the preferences dialog title bar responds to style-update when selecting theme, but ignores CSS tweaks.

Speculating: a theme change may cause a resize (because of font sizes) and that would trigger a configure-event. Maybe not every theme change would trigger it.

[andriiryzhkov]

With map and style-updated:

• main window initially - title bar color NOT updated
• all dialog windows - title bar color updated
• all windows after theme selection - title bar color updated
• all windows (except preferences) after CSS tweaks - title bar color updated
• preferences window after CSS tweaks - title bar color NOT updated

[dterrahe]

• main window initially - title bar color NOT updated

Sloppy testing on my side!

Yes, and this makes sense because the overrides must be in place before the main window is created in _init_widgets. If you move the two calls to g_signal_override_class_handler earlier in dt_gui_gtk_init (directly after the other platform specific code bracketed by #ifdef MAC_INTEGRATION for example) then my very limited testing shows it working...

[andriiryzhkov]

Done. The initial main window has the correct title bar color.

The only thing left is that after CSS tweaks are saved, the preferences dialog does not update the title bar color until it is opened next time.

[dterrahe]

after CSS tweaks are saved, the preferences dialog does not update the title bar color

It looks like gtk actually checks whether anything in the css has changed that would impact the dialog, before sending it a style-updated message. Since there is no explicit link between dialogs and bg_color (dialogs use plugin_bg_color) gtk thinks changing it wouldn't impact it. If you change plugin_bg_color the titlebar for the preferences dialog gets updated too. Fun!

[dterrahe]

Done.

The callback should be

static void _window_set_titlebar_color(GtkWidget *widget)

and likewise the chaining call should be

g_signal_chain_from_overridden_handler(widget);

[dterrahe]

Since there is no explicit link between dialogs and bg_color (dialogs use plugin_bg_color)

So for consistency you could consider changing the code to retrieve the background color to get the actual color of the window/dialog:

      GdkRGBA *bg_color = NULL;
      gtk_style_context_get(style, GTK_STATE_FLAG_NORMAL, GTK_STYLE_PROPERTY_BACKGROUND_COLOR, &bg_color, NULL);
      if(bg_color)
      {
        HWND hwnd = (HWND)gdk_win32_window_get_handle(window);
        if(hwnd)
        {
          // attempt to set the title bar color to theme bg_color.
          // this is supported starting with Windows 11 Build 22000
          COLORREF titlebar_color = RGB((BYTE)(bg_color->red * 255),
                                        (BYTE)(bg_color->green * 255),
                                        (BYTE)(bg_color->blue * 255));

          HRESULT hr = DwmSetWindowAttribute(hwnd, DWMWA_CAPTION_COLOR,
                                             &titlebar_color, sizeof(titlebar_color));
          if (FAILED(hr)) {
            // if setting title bar color failed,
            // attempt to set it light/dark depending to theme bg_color.
            // this is supported starting with Windows 10 version 1809.
            //
            // if the background color is below the threshold (currently 0.5),
            // then we're likely in dark mode
            gboolean use_dark_mode = (bg_color->red   * bg_color->alpha < 0.5 &&
                                      bg_color->green * bg_color->alpha < 0.5 &&
                                      bg_color->blue  * bg_color->alpha < 0.5);

            DwmSetWindowAttribute(hwnd, DWMWA_USE_IMMERSIVE_DARK_MODE,
                                  &use_dark_mode, sizeof(use_dark_mode));
          }
        }
        gdk_rgba_free (bg_color);
      }

[andriiryzhkov]

static void _window_set_titlebar_color(GtkWidget *widget)

Done

[andriiryzhkov]

So for consistency you could consider changing the code to retrieve the background color to get the actual color of the window/dialog

Yeah, that was the initial implementation from GIMP. At some point, I decided getting the bg_color CSS property would be safer. But it looks like they work identically. And the problem with the preferences dialog is still there.

[dterrahe]

And the problem with the preferences dialog is still there.

I'm not sure what you mean. If you are saying changing the line

@define-color bg_color @grey_25;

in the CSS tweaks field and then pressing "save CSS and apply" doesn't change the preference dialog box title color, then that's correct, because it does not depend on that color (not in the themes either).

When I change

@define-color plugin_bg_color @grey_25;

and press "save CSS and apply" the preferences titlebar gets updated immediately (but in this case, obviously, not the application window titlebar, because it is not linked to that).

[andriiryzhkov]

Just one comment: if we stick to retrieving the bg_color from CSS, there will be no situation in which the application title bar is not updated after pressing "save CSS and apply." It looks safer from a UI perspective.

[dterrahe]

if we stick to retrieving the bg_color from CSS, there will be no situation in which the application title bar is not update

Depends on what you are trying to achieve. If you want to force all titlebars to the value of bg_color, then sure. But a user can override the window background color with anything they want (either in the theme or in css override). Do you then want to mismatch the titlebar? In that case, why not introduce a new color just for the title (defaulting to bg_color) so the user has complete freedom to set it separately.

Mind you, personally I like the window/dialog being consistent with their titlebars. And that means dialog titles have a different color than the main window, which is exactly what the current PR does.