GNU bug report logs - #51404
Support system dark mode on Windows 10

Previous Next

To reply to this bug, email your comments to 51404 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Vince Salvino <salvino <at> coderedcorp.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: Support system dark mode on Windows 10
Date: Tue, 26 Oct 2021 04:46:27 +0000

[Message part 1 (text/plain, inline)]

Attached is the patch. Additional info available here: https://github.com/vsalvino/emacs


Vince Salvino

[0001-Support-system-dark-mode-on-Windows-10-version-2004-.patch (application/octet-stream, attachment)]

Message #8 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vince Salvino <salvino <at> coderedcorp.com>
Cc: 51404 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Tue, 26 Oct 2021 17:01:51 +0300

> From: Vince Salvino <salvino <at> 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.

Message #11 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: salvino <at> coderedcorp.com
Cc: 51404 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Tue, 26 Oct 2021 19:18:11 +0300

> Date: Tue, 26 Oct 2021 17:01:51 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: 51404 <at> debbugs.gnu.org
> 
> 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.

Actually, I now see that you already started the legal paperwork
rolling, so we are okay in that department.

Thanks.

Message #14 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Vince Salvino <salvino <at> coderedcorp.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
Subject: RE: bug#51404: Support system dark mode on Windows 10
Date: Tue, 26 Oct 2021 16:49:34 +0000

> > +#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 <at> gnu.org> 
Sent: Tuesday, October 26, 2021 10:02 AM
To: Vince Salvino <salvino <at> coderedcorp.com>
Cc: 51404 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10

> From: Vince Salvino <salvino <at> 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.

Message #17 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vince Salvino <salvino <at> coderedcorp.com>
Cc: 51404 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Tue, 26 Oct 2021 20:05:04 +0300

> From: Vince Salvino <salvino <at> coderedcorp.com>
> CC: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
> Date: Tue, 26 Oct 2021 16:49:34 +0000
> 
> > > +#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.

These "undocumented" strings are all over the Internet, so...

Here are some examples that people may wish trying:

  https://stackoverflow.com/questions/19712368/c-winapi-old-styled-window
  https://developercommunity.visualstudio.com/t/tree-controls-not-displayed-correctly-in-windows-1/423037

And this is just from a couple of minutes of searching the Internet.

> > +/* 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.

The GTK behavior is a bad example, so I'd rather not follow it.
Doesn't the patch in its current form unconditionally change the
appearance of Emacs in some cases?  I think it does, and that means we
will have complaints about unexpected change in behavior.  You can
also bet on someone disliking the result.  So I think this has to be
customizable; let me know if you need help in doing that.

Message #20 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Vince Salvino <salvino <at> coderedcorp.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
Subject: RE: bug#51404: Support system dark mode on Windows 10
Date: Tue, 26 Oct 2021 18:20:35 +0000

[Message part 1 (text/plain, inline)]

Attached is the patch with some of your comments resolved.

Regarding exposing DARK_MODE_APP_NAME to lisp, I am staunchly against that. If users want to potentially segfault their emacs, or make the frame invisible/unusable, they are more than welcome to play with the C code.

Regarding toggling dark mode from within lisp, I think that is a decent idea, and left a TODO in the relevant place in the code. Help would be appreciated here. The current functionality is not "unconditional" per se, it follows the user-configurable OS setting (which is light by default, so no visual change from previous versions of Emacs). The manual has been updated with a relevant note.


Vince Salvino

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

> From: Vince Salvino <salvino <at> coderedcorp.com>
> CC: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
> Date: Tue, 26 Oct 2021 16:49:34 +0000
> 
> > > +#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.

These "undocumented" strings are all over the Internet, so...

Here are some examples that people may wish trying:

  https://stackoverflow.com/questions/19712368/c-winapi-old-styled-window
  https://developercommunity.visualstudio.com/t/tree-controls-not-displayed-correctly-in-windows-1/423037

And this is just from a couple of minutes of searching the Internet.

> > +/* 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.

The GTK behavior is a bad example, so I'd rather not follow it.
Doesn't the patch in its current form unconditionally change the appearance of Emacs in some cases?  I think it does, and that means we will have complaints about unexpected change in behavior.  You can also bet on someone disliking the result.  So I think this has to be customizable; let me know if you need help in doing that.

[0001-Support-system-dark-mode-on-Windows-10-version-2004-.patch (application/octet-stream, attachment)]

Message #27 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Vince Salvino <salvino <at> coderedcorp.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
Subject: RE: bug#51404: Support system dark mode on Windows 10
Date: Wed, 27 Oct 2021 21:41:05 +0000

[Message part 1 (text/plain, inline)]

Update: I managed to get my hands on an 1809 system and was able to get dark mode working there as well. As far as I can tell 1809 is the absolute minimum as that is when this setting and dark mode Explorer were introduced into Windows.

The advantage is that this will now work on Windows Server 2019 and Windows LTSC 2019, which some folks may be limited to as those are the latest Server and LTSC releases.

Attached patch includes the complete change, with relevant notes etc.

Vince Salvino

[0001-Support-system-dark-mode-on-Windows-10-version-1809-.patch (application/octet-stream, attachment)]

Message #30 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vince Salvino <salvino <at> coderedcorp.com>
Cc: 51404 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Thu, 28 Oct 2021 10:15:40 +0300

> From: Vince Salvino <salvino <at> coderedcorp.com>
> CC: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
> Date: Wed, 27 Oct 2021 21:41:05 +0000
> 
> Update: I managed to get my hands on an 1809 system and was able to get dark mode working there as well. As far as I can tell 1809 is the absolute minimum as that is when this setting and dark mode Explorer were introduced into Windows.
> 
> The advantage is that this will now work on Windows Server 2019 and Windows LTSC 2019, which some folks may be limited to as those are the latest Server and LTSC releases.
> 
> Attached patch includes the complete change, with relevant notes etc.

Thanks.  Your legal paperwork also came through, so I will be
installing this soon.

Message #33 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: salvino <at> coderedcorp.com
Cc: 51404 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Sat, 30 Oct 2021 13:34:12 +0300

> Date: Thu, 28 Oct 2021 10:15:40 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: 51404 <at> debbugs.gnu.org
> 
> > From: Vince Salvino <salvino <at> coderedcorp.com>
> > CC: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
> > Date: Wed, 27 Oct 2021 21:41:05 +0000
> > 
> > Update: I managed to get my hands on an 1809 system and was able to get dark mode working there as well. As far as I can tell 1809 is the absolute minimum as that is when this setting and dark mode Explorer were introduced into Windows.
> > 
> > The advantage is that this will now work on Windows Server 2019 and Windows LTSC 2019, which some folks may be limited to as those are the latest Server and LTSC releases.
> > 
> > Attached patch includes the complete change, with relevant notes etc.
> 
> Thanks.  Your legal paperwork also came through, so I will be
> installing this soon.

Now done, with a few minor adaptations to our style conventions.

Please in the future accompany your changes with ChangeLog-style
commit log messages, as described in CONTRIBUTE.  (I added those for
you in this case.)

Can we now please implement the Emacs-specific user setting that will
allow users to opt in or out of this feature?  Here's what I suggest:

  . define a variable exposed to Lisp using DEFVAR_BOOL; let's call it
    w32-follow-system-theme
  . move the determination of w32_darkmode from globals_of_w32fns to
    w32_term_init, and make it depend on the value of
    w32-follow-system-theme: only set w32_darkmode if the variable is
    non-zero
  . document that users can customize w32-follow-system-theme in their
    early-init file (which is processed before window-system
    initialization that calls x-open-connection)

WDYT?

(Let me know if you need help in making any of the above happen.)

Thanks.

Message #36 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Vince Salvino <salvino <at> coderedcorp.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Sat, 30 Oct 2021 17:13:13 +0000

 > define a variable exposed to Lisp using DEFVAR_BOOL; let's call it w32-follow-system-theme

My thought would be to give the user a bit more control. Rather than saying to follow system theme or not, perhaps they could choose from 3 values: follow theme, light, or dark, e.g.:

w32-system-theme:
* nil: follow system theme (default)
* light: force light mode (the old behavior)
* dark: force dark mode

Second, could you provide an existing value from early-init that I could follow as an example? (I never knew early init was a thing, so I am going to research this - it will probably make my personal init customization a lot better too!)

Vince Salvino

Message #39 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vince Salvino <salvino <at> coderedcorp.com>
Cc: 51404 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Sat, 30 Oct 2021 20:39:17 +0300

> From: Vince Salvino <salvino <at> coderedcorp.com>
> CC: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
> Date: Sat, 30 Oct 2021 17:13:13 +0000
> 
>  > define a variable exposed to Lisp using DEFVAR_BOOL; let's call it w32-follow-system-theme
> 
> My thought would be to give the user a bit more control. Rather than saying to follow system theme or not, perhaps they could choose from 3 values: follow theme, light, or dark, e.g.:
> 
> w32-system-theme:
> * nil: follow system theme (default)
> * light: force light mode (the old behavior)
> * dark: force dark mode

I'm not sure I understand why 'light' necessarily means the old
behavior: we didn't set any theme before this change, we just used the
Windows default.  So maybe there should be 4 values:

  nil: never follow the system theme (use Windows default)
  t: always follow the system theme
  light: force light theme (currently the same as nil)
  dark: force dark theme.

> Second, could you provide an existing value from early-init that I
> could follow as an example?

early-init is a file, called literally "early-init.el".  If you have
such a file in your ~/.emacs.d/ directory, Emacs will load it early on
during the startup.

> (I never knew early init was a thing, so I am going to research this - it will probably make my personal init customization a lot better too!)

The recommendation is to move to early-init.el only stuff that cannot
work in the normal init file.  That's because early-init is processed
when some of the infrastructure is not yet set up, so things could
fail there that will work correctly in the init file.

Message #42 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Vince Salvino <salvino <at> coderedcorp.com>, 51404 <at> debbugs.gnu.org,
 47291 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Thu, 11 Nov 2021 06:36:09 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> I'm not sure I understand why 'light' necessarily means the old
> behavior: we didn't set any theme before this change, we just used the
> Windows default.  So maybe there should be 4 values:
>
>   nil: never follow the system theme (use Windows default)
>   t: always follow the system theme
>   light: force light theme (currently the same as nil)
>   dark: force dark theme.

For a similar bug report, see bug#47291.  And we really should support
this on GNU/Linux, too, so having three different methods to support
this seems sub-optimal.

dynamic-setting.el seems like the most likely place to centralise all
this, I think?  I've had a look at what happens when you change the
theme in Gnome, and dynamic-setting-handle-config-changed-event gets
called with an

 (config-changed-event theme-name ":1")

event.  (But not what the event name is -- anybody know how to get at
that?)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #45 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: salvino <at> coderedcorp.com, 51404 <at> debbugs.gnu.org, 47291 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Thu, 11 Nov 2021 09:51:16 +0200

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: Vince Salvino <salvino <at> coderedcorp.com>,  51404 <at> debbugs.gnu.org,
>  47291 <at> debbugs.gnu.org
> Date: Thu, 11 Nov 2021 06:36:09 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > I'm not sure I understand why 'light' necessarily means the old
> > behavior: we didn't set any theme before this change, we just used the
> > Windows default.  So maybe there should be 4 values:
> >
> >   nil: never follow the system theme (use Windows default)
> >   t: always follow the system theme
> >   light: force light theme (currently the same as nil)
> >   dark: force dark theme.
> 
> For a similar bug report, see bug#47291.  And we really should support
> this on GNU/Linux, too, so having three different methods to support
> this seems sub-optimal.

I'm not sure unification is possible here, because the functionality
is quite different, AFAICT.  At least for the functionality in this
bug report, we cannot apply the system theme to an existing frame, we
can only apply it at frame creation time.  So having a handler for
such changes will be able to affect only the frames created after the
change.  Or at least that is my understanding; the code definitely
applies the dark/light theme as part of creating a frame.

Also, having a dynamic thing that tracks changes in these settings
would on Windows mean listening and processing a special window-system
message, which seems to be WM_THEMECHANGED or maybe WM_SETTINGCHANGE.
But that's not what the code installed in this bug report does.

So the functionality seems similar, but the details differ.

Message #48 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: salvino <at> coderedcorp.com, 51404 <at> debbugs.gnu.org, 47291 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Thu, 11 Nov 2021 13:15:03 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> I'm not sure unification is possible here, because the functionality
> is quite different, AFAICT.  At least for the functionality in this
> bug report, we cannot apply the system theme to an existing frame, we
> can only apply it at frame creation time.  So having a handler for
> such changes will be able to affect only the frames created after the
> change.  Or at least that is my understanding; the code definitely
> applies the dark/light theme as part of creating a frame.

Gtk Emacs doesn't respond to dark mode either -- so we have the
opportunity to decide how to handle these things across the board.
Perhaps in Gtk Emacs, dynamic-setting-handle-config-changed-event should
also just set something that will make the next frame creation use
different colours?

> Also, having a dynamic thing that tracks changes in these settings
> would on Windows mean listening and processing a special window-system
> message, which seems to be WM_THEMECHANGED or maybe WM_SETTINGCHANGE.
> But that's not what the code installed in this bug report does.
>
> So the functionality seems similar, but the details differ.

But perhaps Windows should be listening to those events, too?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #51 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: salvino <at> coderedcorp.com, 51404 <at> debbugs.gnu.org, 47291 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Thu, 11 Nov 2021 17:08:04 +0200

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: salvino <at> coderedcorp.com,  51404 <at> debbugs.gnu.org,  47291 <at> debbugs.gnu.org
> Date: Thu, 11 Nov 2021 13:15:03 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > I'm not sure unification is possible here, because the functionality
> > is quite different, AFAICT.  At least for the functionality in this
> > bug report, we cannot apply the system theme to an existing frame, we
> > can only apply it at frame creation time.  So having a handler for
> > such changes will be able to affect only the frames created after the
> > change.  Or at least that is my understanding; the code definitely
> > applies the dark/light theme as part of creating a frame.
> 
> Gtk Emacs doesn't respond to dark mode either -- so we have the
> opportunity to decide how to handle these things across the board.
> Perhaps in Gtk Emacs, dynamic-setting-handle-config-changed-event should
> also just set something that will make the next frame creation use
> different colours?

If that's what people want, sure.  I'd expect them to want Emacs to go
to dark background on all frames.

> > Also, having a dynamic thing that tracks changes in these settings
> > would on Windows mean listening and processing a special window-system
> > message, which seems to be WM_THEMECHANGED or maybe WM_SETTINGCHANGE.
> > But that's not what the code installed in this bug report does.
> >
> > So the functionality seems similar, but the details differ.
> 
> But perhaps Windows should be listening to those events, too?

Maybe.  But some expert (which is not me) will have to explain what to
do when we receive these messages, or submit patches for that.

Message #54 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: salvino <at> coderedcorp.com, 51404 <at> debbugs.gnu.org, 47291 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Fri, 12 Nov 2021 04:00:18 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> If that's what people want, sure.  I'd expect them to want Emacs to go
> to dark background on all frames.

Possibly -- there should probably be a user option, I guess.

> Maybe.  But some expert (which is not me) will have to explain what to
> do when we receive these messages, or submit patches for that.

On the Linux side, we convert the messages to input events and then
react to that event from special-event-map, which seems like a
reasonable structure.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #57 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: salvino <at> coderedcorp.com, 51404 <at> debbugs.gnu.org, 47291 <at> debbugs.gnu.org
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Fri, 12 Nov 2021 08:19:22 +0200

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: salvino <at> coderedcorp.com,  51404 <at> debbugs.gnu.org,  47291 <at> debbugs.gnu.org
> Date: Fri, 12 Nov 2021 04:00:18 +0100
> 
> > Maybe.  But some expert (which is not me) will have to explain what to
> > do when we receive these messages, or submit patches for that.
> 
> On the Linux side, we convert the messages to input events and then
> react to that event from special-event-map, which seems like a
> reasonable structure.

I mean how to tell what the message wants us to do, i.e. which parts
of the UI to change and in what way.

Message #60 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Vince Salvino <salvino <at> coderedcorp.com>
To: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
Subject: Re: bug#51404: Support system dark mode on Windows 10
Date: Fri, 14 Jan 2022 06:00:07 +0000

So, playing around with the idea of dynamically toggling dark/light modes on-the-fly, rather than only at program initialization as this patch currently does. I got a proof of concept working, however there is one ugly caveat. In order to change the theme, we must keep track of every window handle in existence (frame, scroll bar, other UI element) in a global.

The code for this is relatively simple, we can watch for WM_SETTINGCHANGE (in w32_wnd_proc) and then call w32_applytheme (which also needs a few modifications) to all of the HWND objects. But it seems like it would get quite messy and bloated to be storing references to all of these in globals. We would probably need some kind of array of references, and then loop through them upon receiving the signal.

The pseudo-code is something like:

	// Add every hwnd in existence to this array.
	global_hwnds = []

	w32_wnd_proc() {
		...
		case WM_SETTINGCHANGE:
			for each hwnd in global_hwnds {
				w32_applytheme(hwnd);
			}
		...
	}

Thoughts? Is it worth it?

Vince Salvino

Message #63 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Vince Salvino <salvino <at> coderedcorp.com>
To: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>
Subject: RE: Support system dark mode on Windows 10
Date: Sun, 23 Jan 2022 00:00:32 +0000

[Message part 1 (text/plain, inline)]

Attached is a patch which listens to the OS settings change, to dynamically change light/dark GUI during runtime.

Disclaimer here, I am not actually a C nor Win32 developer. I am not currently happy with the g_hwnds[256] implementation - that is purely a sloppy hack as a proof of concept. There is probably a much better way to track the window handles (all outlined in the TODO comment). However, this works if anyone wants to play around with it.

Vince Salvino

[0002-Support-MS-Windows-light-dark-mode-theme-change-duri.patch (application/octet-stream, attachment)]

Message #66 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Vince Salvino <salvino <at> coderedcorp.com>
To: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>
Subject: RE: RE: Support system dark mode on Windows 10
Date: Sat, 29 Jan 2022 03:34:32 +0000

[Message part 1 (text/plain, inline)]

Update: I improved the previous patch to use a linked list to track the window handles during runtime, and am reasonably happy with it. If this looks good please go ahead and install the attached patch 0002 to master. Thanks!


Vince Salvino

[0002-Support-MS-Windows-light-dark-mode-theme-change-duri.patch (application/octet-stream, attachment)]

Message #69 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vince Salvino <salvino <at> coderedcorp.com>
Cc: 51404 <at> debbugs.gnu.org
Subject: Re: Support system dark mode on Windows 10
Date: Sat, 29 Jan 2022 10:40:01 +0200

> From: Vince Salvino <salvino <at> coderedcorp.com>
> CC: Eli Zaretskii <eliz <at> gnu.org>
> Date: Sat, 29 Jan 2022 03:34:32 +0000
> 
> Update: I improved the previous patch to use a linked list to track the window handles during runtime, and am reasonably happy with it. If this looks good please go ahead and install the attached patch 0002 to master. Thanks!

Thanks.  A few comments below, mostly about minor stylistic issues.

> From a8c2f353372d8f015538804e17682e72e40af222 Mon Sep 17 00:00:00 2001
> From: Vince Salvino <salvino <at> coderedcorp.com>
> Date: Fri, 28 Jan 2022 22:25:13 -0500
> Subject: [PATCH] Support MS-Windows light/dark mode theme change during
>  runtime. (Bug#51404)

Please provide a ChangeLog-style description of changes (see
CONTRIBUTE for the details of the format we prefer).

> -/* If the OS is set to use dark mode.  */
> +/* If the OS supports light/dark mode. */
                                        ^^
Our style is to leave 2 spaces after the final period of the comment
(here and elsewhere in your patch).

> +/* Simple linked list to track window handles during runtime so they
> +   can be updated if the Windows light/dark mode theme is changed. */
> +struct HWND_NODE
> +{
> +  HWND hwnd;
> +  struct HWND_NODE *next;
> +};
> +struct HWND_NODE *g_hwnd_root;

I see where you add windows to the  list, but I don't see where you
remove deleted windows from the list.  Does that mean the list will
always grow indefinitely through an Emacs session, even if windows are
deleted?

>  /* Applies the Windows system theme (light or dark) to the window
> -   handle HWND.  */
> +   handle HWND. `track` should generally be TRUE to keep a reference
                 ^^
Two spaces between sentences there.  Also, our style of quoting in
comments is 'like this', not MD-style `like this`.

> +      /* After applying the theme, add the HWND to our global list so
> +	 it can be changed later if the OS light/dark mode theme is
> +	 changed. */
> +      if(track)
> +	{
> +	  struct HWND_NODE *curr = malloc(sizeof(struct HWND_NODE));

Please use xmalloc, not malloc, to allocate memory.

Also, our style is to leave one space between a function name and the
opening parenthesis that follows it.

> +	      /* Loop through all known HWNDs and apply theme */
                                                            ^^
Each comment should end with a period (and 2 spaces).

> +	      struct HWND_NODE *curr = g_hwnd_root;
> +	      while ( curr != NULL )
                     ^^^^^^^^^^^^^^
Our style is NOT to leave a space after the opening parenthesis and
before the closing parenthesis.

Message #72 received at 51404 <at> debbugs.gnu.org (full text, mbox):

From: Vince Salvino <salvino <at> coderedcorp.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "51404 <at> debbugs.gnu.org" <51404 <at> debbugs.gnu.org>
Subject: RE: Support system dark mode on Windows 10
Date: Sat, 29 Jan 2022 20:27:48 +0000

[Message part 1 (text/plain, inline)]

Thanks for the review. Attached is the revised patch, minus one thing specifically:

> I see where you add windows to the list, but I don't see where you remove deleted windows from the list.  Does that mean the list will always grow indefinitely through an Emacs session, even if windows are deleted?

I also had this concern but found it a bit sticky. I can't quite find a way to know if a HWND is destroyed. Windows seems to keep the HWNDs in memory, and even reuses them if a window is destroyed and new one is created. The win32 API seems to be designed around this behavior as calling functions with a HWND that is destroyed or that is not owned by the program will not have any adverse effects.

I experimented with WM_EMACS_DESTROYWINDOW but that seems to only be triggered on the titlebar destroy, not the other "windows" such as scrollbars, menu, etc.

To answer your question, yes the current implementation will grow indefinitely. Practically speaking the memory overhead is quite small though - as a marathon emacs session creating and destroying thousands of frames repeatedly might add up to few kilobytes memory overhead on supported Win 10 systems (each entry is 16 bytes). It's definitely sloppy programming, but I will have to continue to learn more about win32 to figure out the solution, given enough free time in the future.

A few items of note:
* https://stackoverflow.com/questions/2344233/validate-hwnd-using-win32-api
* I'm still digging through the code to figure out if emacs has a parent/child relationship for HWNDs, in which case this might be relevant (especially EnumChildWindows to loop through children and purge them from the list): https://docs.microsoft.com/en-us/windows/win32/winmsg/using-windows


Vince Salvino

[0002-Support-MS-Windows-light-dark-mode-theme-change-duri.patch (application/octet-stream, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.