unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Vince Salvino <salvino@coderedcorp.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: "51404@debbugs.gnu.org" <51404@debbugs.gnu.org>
Subject: bug#51404: Support system dark mode on Windows 10
Date: Sat, 29 Jan 2022 20:27:48 +0000	[thread overview]
Message-ID: <CH2PR12MB4231CFB23C02AB61B8FD8C14A5239@CH2PR12MB4231.namprd12.prod.outlook.com> (raw)
In-Reply-To: <83r18raqam.fsf@gnu.org>

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

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

[-- Attachment #2: 0002-Support-MS-Windows-light-dark-mode-theme-change-duri.patch --]
[-- Type: application/octet-stream, Size: 7706 bytes --]

From ab9bbf47353fcc7b55bd93eaf5d40a27e5c1eb1d Mon Sep 17 00:00:00 2001
From: Vince Salvino <salvino@coderedcorp.com>
Date: Sat, 29 Jan 2022 15:15:01 -0500
Subject: [PATCH] Support MS-Windows light/dark mode theme change during
 runtime

Track HWNDs and update them during WM_SETTINGCHANGE events (Bug#51404).
* src/w32fns.c (w32_applytheme, w32_querydarkmode, w32_wnd_proc,
  globals_of_w32fns): Track and manipulate HWND structs.
---
 src/w32fns.c | 119 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 95 insertions(+), 24 deletions(-)

diff --git a/src/w32fns.c b/src/w32fns.c
index 1ea685d194..8f475cebe4 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -78,6 +78,7 @@ #define _WIN32_WINNT 0x0600
   See: https://github.com/microsoft/WindowsAppSDK/issues/41
 */
 #define DARK_MODE_APP_NAME L"DarkMode_Explorer"
+#define LIGHT_MODE_APP_NAME L"Explorer"
 /* For Windows 10 version 1809, 1903, 1909. */
 #ifndef DWMWA_USE_IMMERSIVE_DARK_MODE_OLD
 #define DWMWA_USE_IMMERSIVE_DARK_MODE_OLD 19
@@ -273,9 +274,20 @@ #define MENU_FREE_DELAY 1000
 int w32_minor_version;
 int w32_build_number;
 
-/* If the OS is set to use dark mode.  */
+/* If the OS supports light/dark mode.  */
+BOOL w32_supports_darkmode = FALSE;
+/* If Emacs should use the OS's dark mode.  */
 BOOL w32_darkmode = FALSE;
 
+/* 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;
+
 /* Distinguish between Windows NT and Windows 95.  */
 int os_subtype;
 
@@ -2303,19 +2315,58 @@ w32_init_class (HINSTANCE hinst)
     }
 }
 
-/* Applies the Windows system theme (light or dark) to the window
-   handle HWND.  */
+
+/**
+ * w32_query_darkmode:
+ *
+ * Gets the preferred Windows app mode:
+ * * FALSE = Light mode (this is equivalent to the user specifying
+ *           Light, or the absence of any setting).
+ * * TRUE = Dark mode (added in Windows 10 1809).
+ */
+static BOOL
+w32_querydarkmode (void)
+{
+  if (w32_supports_darkmode)
+    {
+      /* Check Windows Registry for system theme.
+	 TODO: "Nice to have" would be to create a lisp setting (which
+	 defaults to this Windows Registry value), then read that lisp
+	 value here instead.  This would allow the user to forcibly
+	 override the system theme (which is also user-configurable in
+	 Windows settings; see MS-Windows section in Emacs manual).  */
+      LPBYTE val =
+	w32_get_resource ("Software\\Microsoft\\Windows\\CurrentVersion\\Themes\\Personalize",
+			  "AppsUseLightTheme",
+			  NULL);
+      return val && *val == 0;
+    }
+  return FALSE;
+}
+
+/**
+ * w32_applytheme:
+ *
+ * Applies the Windows system theme (light or dark) to the window
+ * handle HWND.  `track' should generally be TRUE to keep a reference
+ * to this HWND for future use.
+ */
 static void
-w32_applytheme (HWND hwnd)
+w32_applytheme (HWND hwnd, BOOL track)
 {
-  if (w32_darkmode)
+  if (w32_supports_darkmode)
     {
       /* Set window theme to that of a built-in Windows app (Explorer),
 	 because it has dark scroll bars and other UI elements.  */
       if (SetWindowTheme_fn)
-	SetWindowTheme_fn (hwnd, DARK_MODE_APP_NAME, NULL);
+	{
+	  if (w32_darkmode)
+	    SetWindowTheme_fn (hwnd, DARK_MODE_APP_NAME, NULL);
+	  else
+	    SetWindowTheme_fn (hwnd, LIGHT_MODE_APP_NAME, NULL);
+	}
 
-      /* Set the titlebar to system dark mode.  */
+      /* Toggle darkmode titlebar on or off.  */
       if (DwmSetWindowAttribute_fn)
 	{
 	  /* Windows 10 version 2004 and up, Windows 11.  */
@@ -2323,9 +2374,20 @@ w32_applytheme (HWND hwnd)
 	  /* Windows 10 older than 2004.  */
 	  if (w32_build_number < 19041)
 	    attr = DWMWA_USE_IMMERSIVE_DARK_MODE_OLD;
+	  /* Toggle dark mode flag based on value of 'w32_darkmode'.  */
 	  DwmSetWindowAttribute_fn (hwnd, attr,
 				    &w32_darkmode, sizeof (w32_darkmode));
 	}
+
+      /* Add the HWND to our global list so it can be updated later if
+	 the OS light/dark mode theme is changed.  */
+      if(track)
+	{
+	  struct HWND_NODE *curr = xmalloc (sizeof (struct HWND_NODE));
+	  curr->hwnd = hwnd;
+	  curr->next = g_hwnd_root;
+	  g_hwnd_root = curr;
+	}
     }
 }
 
@@ -2342,7 +2404,7 @@ w32_createvscrollbar (struct frame *f, struct scroll_bar * bar)
 		       bar->left, bar->top, bar->width, bar->height,
 		       FRAME_W32_WINDOW (f), NULL, hinst, NULL);
   if (hwnd)
-    w32_applytheme (hwnd);
+    w32_applytheme (hwnd, TRUE);
   return hwnd;
 }
 
@@ -2359,7 +2421,7 @@ w32_createhscrollbar (struct frame *f, struct scroll_bar * bar)
 		       bar->left, bar->top, bar->width, bar->height,
 		       FRAME_W32_WINDOW (f), NULL, hinst, NULL);
   if (hwnd)
-    w32_applytheme (hwnd);
+    w32_applytheme (hwnd, TRUE);
   return hwnd;
 }
 
@@ -2447,7 +2509,7 @@ w32_createwindow (struct frame *f, int *coords)
       DragAcceptFiles (hwnd, TRUE);
 
       /* Enable system light/dark theme.  */
-      w32_applytheme (hwnd);
+      w32_applytheme (hwnd, TRUE);
 
       /* Do this to discard the default setting specified by our parent. */
       ShowWindow (hwnd, SW_HIDE);
@@ -5178,6 +5240,25 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
 	 changed, so if Emacs is interested in some of them, it could
 	 update its internal values.  */
       my_post_msg (&wmsg, hwnd, msg, wParam, lParam);
+
+      /* Check if settings changed Light/Dark mode.
+	 Re-lookup the setting and update the HWNDs accordingly.  */
+      if(w32_supports_darkmode)
+	{
+	  BOOL new_darkmode = w32_querydarkmode();
+	  if (w32_darkmode != new_darkmode)
+	    {
+	      w32_darkmode = new_darkmode;
+	      /* Loop through all known HWNDs and apply theme.  */
+	      struct HWND_NODE *curr = g_hwnd_root;
+	      while (curr != NULL)
+		{
+		  w32_applytheme (curr->hwnd, FALSE);
+		  curr = curr->next;
+		}
+	    }
+	}
+
       goto dflt;
 
     case WM_SETFOCUS:
@@ -11150,13 +11231,14 @@ globals_of_w32fns (void)
     get_proc_addr (hm_kernel32, "SetThreadDescription");
 
   /* Support OS dark mode on Windows 10 version 1809 and higher.
-     See `w32_applytheme` which uses appropriate APIs per version of Windows.
+     See `w32_applytheme' which uses appropriate APIs per version of Windows.
      For future wretches who may need to understand Windows build numbers:
      https://docs.microsoft.com/en-us/windows/release-health/release-information
   */
   if (os_subtype == OS_SUBTYPE_NT
       && w32_major_version >= 10 && w32_build_number >= 17763)
     {
+      w32_supports_darkmode = TRUE;
       /* Load dwmapi.dll and uxtheme.dll, which will be needed to set
 	 window themes.  */
       HMODULE dwmapi_lib = LoadLibrary("dwmapi.dll");
@@ -11165,19 +11247,8 @@ globals_of_w32fns (void)
       HMODULE uxtheme_lib = LoadLibrary("uxtheme.dll");
       SetWindowTheme_fn = (SetWindowTheme_Proc)
 	get_proc_addr (uxtheme_lib, "SetWindowTheme");
-
-      /* Check Windows Registry for system theme and set w32_darkmode.
-	 TODO: "Nice to have" would be to create a lisp setting (which
-	 defaults to this Windows Registry value), then read that lisp
-	 value here instead. This would allow the user to forcibly
-	 override the system theme (which is also user-configurable in
-	 Windows settings; see MS-Windows section in Emacs manual). */
-      LPBYTE val =
-	w32_get_resource ("Software\\Microsoft\\Windows\\CurrentVersion\\Themes\\Personalize",
-			  "AppsUseLightTheme",
-			  NULL);
-      if (val && *val == 0)
-	w32_darkmode = TRUE;
+      /* Set the preferred mode from OS settings.  */
+      w32_darkmode = w32_querydarkmode();
     }
 
   except_code = 0;
-- 
2.35.0.windows.1


      reply	other threads:[~2022-01-29 20:27 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-26  4:46 bug#51404: Support system dark mode on Windows 10 Vince Salvino
2021-10-26 14:01 ` Eli Zaretskii
2021-10-26 16:18   ` Eli Zaretskii
2021-10-26 16:49   ` Vince Salvino
2021-10-26 17:05     ` Eli Zaretskii
2021-10-26 18:20       ` Vince Salvino
2021-10-27 21:41         ` Vince Salvino
2021-10-28  7:15           ` Eli Zaretskii
2021-10-30 10:34             ` Eli Zaretskii
2021-10-30 17:13               ` Vince Salvino
2021-10-30 17:39                 ` Eli Zaretskii
2021-11-11  5:36                   ` bug#47291: " Lars Ingebrigtsen
2021-11-11  7:51                     ` Eli Zaretskii
2021-11-11 12:15                       ` Lars Ingebrigtsen
2021-11-11 15:08                         ` Eli Zaretskii
2021-11-12  3:00                           ` Lars Ingebrigtsen
2021-11-12  6:19                             ` Eli Zaretskii
2022-01-14  6:00 ` Vince Salvino
2022-01-23  0:00 ` Vince Salvino
2022-01-29  3:34   ` bug#51404: " Vince Salvino
2022-01-29  8:40     ` bug#51404: " Eli Zaretskii
2022-01-29 20:27       ` Vince Salvino [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CH2PR12MB4231CFB23C02AB61B8FD8C14A5239@CH2PR12MB4231.namprd12.prod.outlook.com \
    --to=salvino@coderedcorp.com \
    --cc=51404@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).