Feat: Implement ThemeListener for winui3 #445
Conversation
@dotnet-policy-service agree |
|
The build log is so long that it crashes my Edge. |
|
@Arlodotexe we should look at reducing the verbosity of the build and seeing if we can hook into the checkbox somehow or something... @HO-COOH thanks for the PR! The errors are about the setup around the PInvoke: Are you familiar with the C#/Win32 project? That's the best way these days to setup PInvoke a bit more painlessly. It'd be great if we could just swap over to that and avoid a bunch of the code here from the PR for the native methods. @niels9001 forgot that we didn't do this for TitleBar yet either: https://github.com/CommunityToolkit/Labs-Windows/blob/main/components/TitleBar/src/NativeMethods.cs - though once 1.6 is out, we may not worry about it. |
I do have some knowledge about it. I am concerned about whether it will be compatible with other builds. Will after installing this nuget still builds on like WASM builds? |
|
@HO-COOH Have you ever tried ‘ThemeSettings’ WinRT API for the high contrast aware? Looks like you’re trying to create a window and hook an event change of window style. Also CsWin32 is a generator of P/Invoke so it shouldn't make a problem. var windowId = MainWindow.Instance.AppWindow.Id;
ThemeSettings = ThemeSettings.CreateForWindowId(windowId);
ThemeSettings.Changed += (s, e) => { IsHighContrastChanged?.Invoke(this, s.HighContrast); };you can check out there and try it out. To be honest, this isn’t handy too but more easy to handle imo. |
|
@michael-hawker Done |
|
Could it be that something is blocking the basic functionality? (things like) Theme adaptation should be as native as possible and I am convinced there's no need for a complex solution. |
|
My solution is way too easy. |
The |
|
I thought it did (I made this service)? |
Is this self-contained or packaged? I believe it's self-contained, because I can directly run |
|
Not self contain, we have alias 'files.exe'. |
|
Then it should not be working on Windows 10. My video (and also Microsoft's own doc) is clear enough |
| unsafe | ||
| { | ||
| var wcx = new Windows.Win32.UI.WindowsAndMessaging.WNDCLASSEXW(); | ||
| wcx.cbSize = (uint)Marshal.SizeOf(wcx); |
There was a problem hiding this comment.
This is not AOT safe. You need to configure CsWin32 in blittable mode (copy this)
| wcx.cbSize = (uint)Marshal.SizeOf(wcx); | ||
| fixed (char* pClassName = s_className) | ||
| { | ||
| wcx.lpszClassName = new Windows.Win32.Foundation.PCWSTR(pClassName); |
There was a problem hiding this comment.
This is a GC hole. The string will be unpinned while still in use.
| { | ||
| wcx.lpszClassName = new Windows.Win32.Foundation.PCWSTR(pClassName); | ||
| } | ||
| wcx.lpfnWndProc = wndProc; |
There was a problem hiding this comment.
This is not AOT safe. You need to use the blittable mode and pass a pointer to an [UnsafeCallersOnly] method.
I am not familiar with this. Is there any doc I can look into? |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
I just open a PR on your repo, can you take a look? |
HO-COOH
force-pushed
the
feat/winui3-themelistener
branch
from
70c489e to
1cd0f4b
Compare
|
@Lightczx Thanks for your work. And @Sergio0694 please review it |
|
I think Win32MessageLoop is too heavy. We can use RegNotifyChangeKeyValue to impl this. Here is a sample public partial class ThemeListener
{
private const UIntPtr HKEY_CURRENT_USER = 0x80000001;
private const uint KEY_NOTIFY = 0x0010;
private const uint KEY_READ = ((0x00020000) | (0x0001) | (0x0008) | (0x0010)) & (~0x00100000);
public Action<bool>? ThemeChanged;
public async Task Loop(CancellationToken cancellationToken)
{
IntPtr subKeyName = Marshal.StringToCoTaskMemAnsi("SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Themes\\Personalize");
IntPtr hKeyRef = Marshal.AllocCoTaskMem(8);
int hr = RegOpenKeyExA(HKEY_CURRENT_USER, subKeyName, 0, KEY_NOTIFY | KEY_READ, hKeyRef);
Marshal.FreeCoTaskMem(subKeyName);
if (hr != 0)
{
Marshal.FreeCoTaskMem(hKeyRef);
return;
}
IntPtr hKey = Marshal.ReadIntPtr(hKeyRef);
Marshal.FreeCoTaskMem(hKeyRef);
subKeyName = Marshal.StringToCoTaskMemAnsi("AppsUseLightTheme");
IntPtr dwSizeRef = Marshal.AllocCoTaskMem(4);
IntPtr valueRef = Marshal.AllocCoTaskMem(4);
Marshal.WriteInt32(dwSizeRef, 4);
while (true)
{
RegNotifyChangeKeyValue(hKey, 1, 0x00000004, IntPtr.Zero, 0);
RegQueryValueExA(hKey, subKeyName, IntPtr.Zero, IntPtr.Zero, valueRef, dwSizeRef);
int value = Marshal.ReadInt32(valueRef);
bool result = value > 0;
if (cancellationToken.IsCancellationRequested)
{
break;
}
ThemeChanged?.Invoke(result);
}
Marshal.FreeCoTaskMem(subKeyName);
Marshal.FreeCoTaskMem(dwSizeRef);
Marshal.FreeCoTaskMem(valueRef);
RegCloseKey(hKey);
}
[LibraryImport("Advapi32.dll")]
private static partial int RegOpenKeyExA(UIntPtr hKey, IntPtr lpSubKey, uint ulOptions, uint samDesired, IntPtr phkResult);
[LibraryImport("Advapi32.dll")]
private static partial int RegQueryValueExA(IntPtr hKey, IntPtr lpValueName, IntPtr lpReserved, IntPtr lpType, IntPtr lpData, IntPtr lpcbData);
[LibraryImport("Advapi32.dll")]
private static partial int RegNotifyChangeKeyValue(IntPtr hKey, uint bWatchSubtree, uint dwNotifyFilter, IntPtr hEvent, uint fAsynchronous);
[LibraryImport("Advapi32.dll")]
private static partial int RegCloseKey(IntPtr hKey);
} |
|
Packaged or not doesn't matter, if you disable virtualization of registry it should work and my app also work in that case. But you got the point, it doesn't work for most cases. |
This means the package can be harder to pass(or never pass) windows store verification. |
|
Why not use Microsoft.win32.SystemEvents？ /// <summary>
/// A standard Win32 window proc for our broadcast window.
/// </summary>
private IntPtr WindowProc(IntPtr hWnd, int msg, nint wParam, nint lParam)
{
switch (msg)
{
case Interop.User32.WM_SETTINGCHANGE:
string? newString;
IntPtr newStringPtr = lParam;
if (lParam != 0)
{
newString = Marshal.PtrToStringUni(lParam);
if (newString != null)
{
newStringPtr = Marshal.StringToHGlobalUni(newString);
}
}
Interop.User32.PostMessageW(_windowHandle, Interop.User32.WM_REFLECT + msg, wParam, newStringPtr);
break; case Interop.User32.WM_REFLECT + Interop.User32.WM_SETTINGCHANGE:
try
{
OnUserPreferenceChanging(msg - Interop.User32.WM_REFLECT, wParam, lParam);
OnUserPreferenceChanged(msg - Interop.User32.WM_REFLECT, wParam, lParam);
}
finally
{
try
{
if (lParam != 0)
{
Marshal.FreeHGlobal(lParam);
}
}
catch (Exception e)
{
Debug.Fail("Exception occurred while freeing memory: " + e.ToString());
}
}
break; |
@cnbluefire You are adding a whole new package as dependency. Plus I tried it, the |
|
QQ for the folks on this thread, where do folks use the ThemeListener over just listening to the |
Because it is supposed to listen to system theme, not app theme which can be override by user (also more and more app supports this feature) |
|
Using ShouldAppsUseDarkMode in the window message loop would be alternative to reg key. [DllImport("uxtheme.dll", SetLastError = true, EntryPoint = "#132")]
public static extern bool ShouldAppsUseDarkMode(); |
Take a look at microsoft/WindowsAppSDK#41 (comment) |
|
Well, I know, all of them are undocumented, the key also may not be present likewise. I'm saying combining (respecting the reg key if any and fall backing to SystemUsesLightTheme too) might be a thorough way of doing this. |
Fixes
Closes #135
PR Type
What kind of change does this PR introduce?
Bugfix
What is the current behavior?
It does not work under WinUI3.
What is the new behavior?
This PR implements the broken
ThemeListener, closes #135. I really need a workingThemeListenerfor handling Dark/Light theme changes. I am not concerned withHighContrastat the time being.PR Checklist
Please check if your PR fulfills the following requirements:
Other information