bug#51404: Support system dark mode on Windows 10

[Vince Salvino <salvino@coderedcorp.com>]

> > +#define DARK_MODE_APP_NAME L"DarkMode_Explorer"

> Can we make this exposed to Lisp, rather than hard-coded?  Hard-coding a specific application for a theme sounds un-Emacsy.  People could want to experiment with other apps.

Given that this is not so much a preference, as an undocumented magic string in Win32, I think anyone who wants to play with this is going to require knowledge of C and gdb to experiment, to risk causing erratic and unknown behavior. So I would be inclined to keep it in C.


> > +#ifndef DWMWA_USE_IMMERSIVE_DARK_MODE #define 
> > +DWMWA_USE_IMMERSIVE_DARK_MODE 20

> Why not use 19 and 20, depending on the Windows build number, and thus expand the applicability of the feature?

I can add support for 19, but do not have the ability to test it on those specific Win10 builds to confirm if it actually works as expected. If someone is able to test on a version of Windows 10 older than 2004, then I will include. Erring on the side of stability for now.


> +/* Applies the Windows system theme (light or dark) to a window 
> +handle. */ static void w32_applytheme(HWND hwnd) {
> +  if (w32_darkmode) {
> +    /* Set window theme to that of a built-in Windows app (Explorer)
> +       because it has dark scroll bars and other UI elements. */

> Likewise here: it should be able to control this behavior by a user option.  We cannot assume that every Emacs user will automatically want to follow the system theme.

I agree this would be a "nice to have", but the current functionality is in-line with behavior on other systems (GTK, macOS, etc. i.e. the application has no say in window decorations which are controlled by the window manager). If we did add an elisp setting it should default to the registry value at runtime. I also have no idea how to create an elisp setting and read it in C. Examples or contributions to this patch would be helpful.


> > +    /* Set the titlebar to system dark mode. */
> > +    if (DwmSetWindowAttribute_fn) {
> > +      DwmSetWindowAttribute_fn
> > +	(hwnd,
> > +	 DWMWA_USE_IMMERSIVE_DARK_MODE,
> > +	 &w32_darkmode,
> > +	 sizeof(w32_darkmode));
> > +    }

> Does it make sense to call DwmSetWindowAttribute if we couldn't call SetWindowTheme?  I know that such a situation shouldn't normally happen, but what if it does?  If we need both calls, the second call should be conditioned by SetWindowTheme_fn as well.

There is no harm in calling one without the other. SetWindowTheme sets things like scrollbars. DwmSetWindowAttribute specifically sets the titlebar. My original proof-of-concept only had DwmSetWindowAttribute and worked fine.

I will make the other requested changes, i.e. registry helper, style guide, and NEWS; and submit an updated patch.


Vince Salvino

-----Original Message-----
From: Eli Zaretskii <eliz@gnu.org> 
Sent: Tuesday, October 26, 2021 10:02 AM
To: Vince Salvino <salvino@coderedcorp.com>
Cc: 51404@debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10

> From: Vince Salvino <salvino@coderedcorp.com>
> Date: Tue, 26 Oct 2021 04:46:27 +0000
> 
> Attached is the patch. Additional info available here: 
> https://github.com/vsalvino/emacs

Thanks.  I have some comments and questions below, but in any case these changes are large enough to require copyright assignment from you.  If you'd be willing to start the legal paperwork at this time, I will send you the form to fill with the appropriate instructions.

>  LPBYTE
>  w32_get_resource (const char *key, LPDWORD lpdwtype)
> +{
> +  return w32_query_registry(REG_ROOT, key, lpdwtype); }
> +
> +/* Enables reading any key/name from the Windows Registry */ LPBYTE 
> +w32_query_registry (const char *root, const char *key, LPDWORD 
> +lpdwtype)

I'd prefer that you simply add an extra argument to the existing w32_get_resource, and adjust its single caller to pass REG_ROOT there.

> +/*
> +  Internal/undocumented constants for Windows Dark mode.
> +  See: https://github.com/microsoft/WindowsAppSDK/issues/41
> +*/

Please follow our style for comments, both single-line and multi-line.

> +#define DARK_MODE_APP_NAME L"DarkMode_Explorer"

Can we make this exposed to Lisp, rather than hard-coded?  Hard-coding a specific application for a theme sounds un-Emacsy.  People could want to experiment with other apps.

> +#ifndef DWMWA_USE_IMMERSIVE_DARK_MODE #define 
> +DWMWA_USE_IMMERSIVE_DARK_MODE 20

Why not use 19 and 20, depending on the Windows build number, and thus expand the applicability of the feature?

> +/* Applies the Windows system theme (light or dark) to a window 
> +handle. */ static void w32_applytheme(HWND hwnd) {
> +  if (w32_darkmode) {
> +    /* Set window theme to that of a built-in Windows app (Explorer)
> +       because it has dark scroll bars and other UI elements. */

Likewise here: it should be able to control this behavior by a user option.  We cannot assume that every Emacs user will automatically want to follow the system theme.

> +    if(SetWindowTheme_fn) {
> +      SetWindowTheme_fn(hwnd, DARK_MODE_APP_NAME, NULL);
> +    }

Please follow our style of using braces in C code.

> +    /* Set the titlebar to system dark mode. */
> +    if (DwmSetWindowAttribute_fn) {
> +      DwmSetWindowAttribute_fn
> +	(hwnd,
> +	 DWMWA_USE_IMMERSIVE_DARK_MODE,
> +	 &w32_darkmode,
> +	 sizeof(w32_darkmode));
> +    }

Does it make sense to call DwmSetWindowAttribute if we couldn't call SetWindowTheme?  I know that such a situation shouldn't normally happen, but what if it does?  If we need both calls, the second call should be conditioned by SetWindowTheme_fn as well.

Last, but not least: this feature should be called out in NEWS and preferably also described in the "MS-Windows" Appendix in the Emacs manual.

Thanks again for working on this.