Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable dark menus on Windows 11 builds greater than 22631 #788

Merged
merged 1 commit into from
Aug 12, 2023

Conversation

razielanarki
Copy link
Contributor

@razielanarki razielanarki commented Aug 11, 2023

  • Allow Windows builds greater than 22631 to access "private" APIs. (Tested on Windows 11 Insiders Canary build 25926)
  • Update PreferredAppMode enum
  • Call AllowDarkModeForWindow (@133) from dark::set_titlebar_mode. This fixes the menubar / context menus to use dark mode.

Tested both the x86 build (with foobar2000 v1.6.16) and the x64 build (with foobar2000 v2.1 preview 2023-08-01).

Copy link
Owner

@reupen reupen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Allow Windows builds greater than 22631 to access "private" APIs. (Tested on Windows 11 Insiders Canary build 25926)

Okay, the max version check was just in there as a precaution against future breakage, but removing it is okay as the risk seems low (plus the build numbers are all over the place anyway).

Update PreferredAppMode enum

Do you have some source that those are the official names? Otherwise I don't see much reason to rename those (it'd just be some names that someone else has made up).

Call AllowDarkModeForWindow (@133) from dark::set_titlebar_mode. This fixes the menubar / context menus to use dark mode.

I don't see any difference from this on build 25926; menus are already dark with the max version check removed. Could you elaborate on what this is fixing?

@razielanarki
Copy link
Contributor Author

razielanarki commented Aug 11, 2023

Thanks for the PR!

Allow Windows builds greater than 22631 to access "private" APIs. (Tested on Windows 11 Insiders Canary build 25926)

Okay, the max version check was just in there as a precaution against future breakage, but removing it is okay as the risk seems low (plus the build numbers are all over the place anyway).

so, yeah, this change alone may be just the one needed (see laterfor some of my thoughts).

Update PreferredAppMode enum

Do you have some source that those are the official names? Otherwise I don't see much reason to rename those (it'd just be some names that someone else has made up).

i think the names come from the disassembly of uxtheme.dll@122 (screenshot from IDA free, with pdb symbols loaded acquired via windbg):
image

with a little digging around this reads as such:

PreferredAppMode g_preferredAppMode;       // set by uxtheme@135 (SetPreferredAppMode)
BOOL             g_isSystemAppModeLight;   // contains the user setting

BOOL ShouldAppUserDarkMode()
{
  BOOL result;

  result = FALSE;
  // substituting the names makes sense here:
  if( g_preferredAppMode != ForceLight && ( !g_isSystemAppModeLight || g_preferredAppMode == ForceDark ) )
    return TRUE;
  return FALSE;
}

Call AllowDarkModeForWindow (@133) from dark::set_titlebar_mode. This fixes the menubar / context menus to use dark mode.

I don't see any difference from this on build 25926; menus are already dark with the max version check removed. Could you elaborate on what this is fixing?

hm, yeah.. i first added the latter, then realized there's the build number check ... 👼

however, looking at the disassembly of uxtheme.dll@133 (AllowDarkModeForWindow) just relays the call to uxtheme.dll@140 (AllowDarkModeForWindowWithTelemetryId, same sig), which after checking a HWND prop via GetPropW, calls an unexported and kinda convoluted function (ApplyStringProp), which SetPropW-s some props on the HWND...

...which may or may not be just triggering telemetry of dark mode usage. hard to tell.
could also flag stg for the DWM, etc (COM interop/winrt xaml islands come to mind), or could be just a remnant of the legacy way of setting dark mode, kept for compatibility.

so, yeah, at this point the latter (ie: calling AllowDarkModeForWindow) doesn't seem absolutely necessary, in this case at least. (and would also incur a name change/clarification of dark::set_titlebar_mode)

@reupen
Copy link
Owner

reupen commented Aug 12, 2023

Thanks for the PR!

Allow Windows builds greater than 22631 to access "private" APIs. (Tested on Windows 11 Insiders Canary build 25926)

Okay, the max version check was just in there as a precaution against future breakage, but removing it is okay as the risk seems low (plus the build numbers are all over the place anyway).

so, yeah, this change alone may be just the one needed (see laterfor some of my thoughts).

Okay, removing the max version check and changing are_private_apis_allowed() to just call does_os_support_dark_mode() is fine. (I may block Wine too as I've seen crashes there, but not in any rush to do that...)

Other changes I don't think are necessary so would remove those.

however, looking at the disassembly of uxtheme.dll@133 (AllowDarkModeForWindow) just relays the call to uxtheme.dll@140 (AllowDarkModeForWindowWithTelemetryId, same sig), which after checking a HWND prop via GetPropW, calls an unexported and kinda convoluted function (ApplyStringProp), which SetPropW-s some props on the HWND...

...which may or may not be just triggering telemetry of dark mode usage. hard to tell. could also flag stg for the DWM, etc (COM interop/winrt xaml islands come to mind), or could be just a remnant of the legacy way of setting dark mode, kept for compatibility.

so, yeah, at this point the latter (ie: calling AllowDarkModeForWindow) doesn't seem absolutely necessary, in this case at least. (and would also incur a name change/clarification of dark::set_titlebar_mode)

Not sure on the mechanism, but IIRC AllowDarkModeForWindow causes DarkMode:: to be automatically prepended to theme class names when dark mode is active. I just use SetWindowTheme or prepend DarkMode:: etc when calling OpenThemeData instead (which is more flexible in any case).

Allow Windows builds greater than 22631 to access "private" APIs.
(Tested on Windows 11 Insiders Canary build 25926)
Copy link
Owner

@reupen reupen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build was failing – have updated this to get it over the line. Will merge shortly.

@reupen reupen changed the title dark mode fixes Enable dark menus on Windows 11 builds greater than 22631 Aug 12, 2023
@reupen reupen merged commit 99e0c74 into reupen:main Aug 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants