microsoft / WindowsAppSDK

The Windows App SDK empowers all Windows desktop apps with modern Windows UI, APIs, and platform features, including back-compat support, shipped via NuGet.
https://docs.microsoft.com/windows/apps/windows-app-sdk/
MIT License
3.85k stars 326 forks source link

RegNotifyChangeKeyValue is not working in WinUI3 app #4075

Open HO-COOH opened 12 months ago

HO-COOH commented 12 months ago

Describe the bug

Following microsoft/WindowsAppSDK#4006, I tried to monitor HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Themes\Personalize registry for light/dark theme listener, but RegNotifyChangeKeyValue does not seem to work in a packaged winui3 app.

The same code works in a console application project

Steps to reproduce the bug

  1. Create a C++ winui3 app (packaged)
  2. Use the following code, in MainWindow constructor

    std::thread{
                [] {
                    auto handle = CreateEvent(NULL, true, false, nullptr);
                    HKEY key{};
                    RegOpenKeyEx(HKEY_CURRENT_USER, LR"(Software\Microsoft\Windows\CurrentVersion\Themes\Personalize)", 0, KEY_READ, &key);
                    while (true)
                    {
                        auto hr = RegNotifyChangeKeyValue(
                            key,
                            true,
                            REG_NOTIFY_CHANGE_NAME |
                            REG_NOTIFY_CHANGE_ATTRIBUTES |
                            REG_NOTIFY_CHANGE_LAST_SET |
                            REG_NOTIFY_CHANGE_SECURITY,
                            handle,
                            true
                        );
                        if (WaitForSingleObject(handle, INFINITE) != WAIT_FAILED)
                        {
                            MessageBox(NULL, L"changed", L"", 0);
                            OutputDebugString(L"Triggered\n");
                        }
                    }
    
                    //while (true)
                    //{
                    //    DWORD value{};
                    //    DWORD bytes = sizeof(value);
                    //    auto hr = RegGetValue(key, nullptr, LR"(AppsUseLightTheme)", RRF_RT_DWORD, nullptr, &value, &bytes);
                    //    OutputDebugString(std::format(L"hr: {}, value: {}, cb: {}\n", hr, value, bytes).data());
                    //    std::this_thread::sleep_for(std::chrono::seconds{ 1 });
                    //}
                }
            }.detach();
  3. Build and run the app, put a breakpoint on the OutputDebugString line. Toggle light/dark mode in system settings, the breakpoint is not hit.
  4. Use the same code in a console application project, it will break.

Bonus: If you use the commented code to monitor the value, it will reads the correct changes of the value, suggesting there is something wrong internally.

Expected behavior

No response

Screenshots

No response

NuGet package version

Windows App SDK 1.4.3: 1.4.231115000

Packaging type

Packaged (MSIX)

Windows version

Windows 10 version 1809 (17763, October 2018 Update)

IDE

Visual Studio 2022

Additional context

No response

DarranRowe commented 12 months ago

There are two questions, assuming that your project is created exactly how you described. In other words, assuming that the application is still packaged.

First, if you remove the packaging, does the application work correctly? (Use <WindowsPackageType>None</WindowsPackageType> in the .vcxproj file.) Secondly, if you use the Unvirtualized Resources restricted capability, does the application work correctly?

The only way that I know of to detect the change between light and dark themes, outside of ColorValuesChanged, is to handle the WM_SETTINGSCHANGE message and look for the "ImmersiveColorSet" string.

HO-COOH commented 12 months ago

@DarranRowe Can't afford to make it unpackged. And that unvirtualized registry write capability requires Windows 10 18362, which is higher than what I want to target.

DarranRowe commented 12 months ago

Even if they are not what you want, it is just a testing step. Those checks are only there to see if it is the package getting in the way of the registry read.

JaiganeshKumaran commented 12 months ago

This seems weird as I do remember listening for changes on the exact particular registry key in a UWP app.

DarranRowe commented 12 months ago

https://github.com/microsoft/WindowsAppSDK/assets/52577874/452b7086-7362-4b17-b9e7-c4cfb51fbc39

It isn't a problem with Windows 11. That was also why I asked to test things without the package getting in the way. There is no reason for the WinUI 3 library to affect anything, it is just a UI library running in a desktop application process. Since this is packaged, then the Windows App SDK dynamic dependencies wouldn't have an effect. I don't think the dynamic dependencies detouring touch the registry functions anyway. However, the registry virtualization from the package means that it could be a Windows bug or deficiency on the older versions of Windows 10. If this was tested unpackaged and it worked as expected, then it would indicate that it is something to do with the package itself.

ChrisGuzak commented 11 months ago

packaged apps get registry virtualization by default. you can disable that with manifest settings. learn more here.

HO-COOH commented 11 months ago

@ChrisGuzak Any work around for Windows 10 17763?

DrusTheAxe commented 10 months ago

@ChrisGuzak Any work around for Windows 10 17763?

@brialmsft @jsidewhite @kanismohammed ?

softworkz commented 2 weeks ago

I can confirm that RegNotifyChangeKeyValue works fine after disabling registry virtualization via:

<Package>
  <Properties>
    <desktop6:RegistryWriteVirtualization>disabled</desktop6:RegistryWriteVirtualization>
  </Properties>
  <Capabilities>
    <rescap:Capability Name="unvirtualizedResources" />
  </Capabilities>
</Package>

in the appxmanifest.

Any work around for Windows 10 17763?

If you need it badly, you could try to look into the VirtualDesktop API which has some change events. While researching I came across this:

https://github.com/Grabacr07/VirtualDesktop

It doesn't support 17763 because the COM GUIDs are regularly changing (kind of private API), but another project has the COM GUIDs for older Windows versions:

https://github.com/MScholtes/VirtualDesktop/blob/c601d38796e947d7647c9124a5087fb4b595cbd9/VirtualDesktopServer2016.cs#L2-L3

So maybe(!) you'll be able to get the change event you need from this (I was looking for detecting a wallpaper change).

softworkz commented 2 weeks ago

@HO-COOH - if all you need is to monitor a theme change, why not use this?

            ((FrameworkElement)this.window.Content).ActualThemeChanged += this.Window_ThemeChanged;
HO-COOH commented 2 weeks ago

@HO-COOH - if all you need is to monitor a theme change, why not use this?

        ((FrameworkElement)this.window.Content).ActualThemeChanged += this.Window_ThemeChanged;

Ideally we want to provide a theme setting for user to able to override the system theme in app, so this is not an option. An independent theme listener is also necessary for fixing WinUI theme-related bugs (i.e fixing the incorrect title bar button color).

HO-COOH commented 2 weeks ago

It doesn't support 17763 because the COM GUIDs are regularly changing (kind of private API), but another project has the COM GUIDs for older Windows versions:

https://github.com/MScholtes/VirtualDesktop/blob/c601d38796e947d7647c9124a5087fb4b595cbd9/VirtualDesktopServer2016.cs#L2-L3

So maybe(!) you'll be able to get the change event you need from this (I was looking for detecting a wallpaper change).

Undocumented / private COM GUID is never an option for me. I need Microsoft to provide clearly documented, predictable behavior for achieving this, if they claim WASDK supports down to 17763.

Oh wait, I was talking about RegNotifyChangeKeyValue wasn't I? I want a way to listen for any reg key changes, and your solution is only solving a specific case.

softworkz commented 2 weeks ago

Undocumented / private COM GUID is never an option for me. I need Microsoft to provide clearly documented, predictable behavior for achieving this, if they claim WASDK supports down to 17763.

Also WASDK doesn't claim to support all of its features down to 17763. Also 17763 is out of support, so there's nothing to expect from MS.

Anyway, you asked for a workaround!

Oh wait, I was talking about RegNotifyChangeKeyValue wasn't I? I want a way to listen for any reg key changes, and your solution is only solving a specific case.

You wrote in your OP:

I tried to monitor HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Themes\Personalize registry for light/dark theme listener,

softworkz commented 2 weeks ago

Ideally we want to provide a theme setting for user to able to override the system theme in app, so this is not an option.

Why not? When overridden, you know your app's theme, why do you want to still know about desktop theme changes in this case?

An independent theme listener is also necessary for fixing WinUI theme-related bugs (i.e fixing the incorrect title bar button color).

Yes, I'm doing that based on ColorValuesChanged (not targeting 17763..)

Monitoring WM_SETTINGSCHANGE is the only other way as @DarranRowe mentioned already. You can use WinUIEx or custom impl to monitor. It fires on theme change but not on wallpaper change.

HO-COOH commented 2 weeks ago

You wrote in your OP:

I tried to monitor HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Themes\Personalize registry for light/dark theme listener,

That's just a context for explaining the title I am asking. What if someone needs to monitor some other reg values and needs it to be packaged and also want to support 17763? There is no possible way until now, isn't it? So we either:

Also, I already implemented one using WM_SETTINGCHANGED that supports 17763, and submited a PR to community toolkit, but somehow not getting merged. So everyone have to re-implement this, if they need to fix the titlebar button issue

softworkz commented 2 weeks ago
  • use polling And this is quite epic I am talking about.

It's not that bad of an option. A theme change itself takes a while until it's fully completed, and when a user changes the theme they don't specifically whatch your title bar colors to be changed instantly, so polling the reg value like every 5 seconds should be perfectly sufficient.

Have you ever watched ProcessMon from SysInternals for registry invocations? Quite often, you see hundreds of registry api calls within a single second, not just by apps but the OS itself. Registry reads are cheap and in no way of concern when doing like every few seconds (neither at higher frequencies). But as developers, we hate doing so of course. It's just a last resort if you need 17763 so badly.

But as you can see, since they don't even manage to fix something blatantly trivial and widely visible on any WinUI3 app like the window titlebar control buttons - you cannot seriously expect that they'd do anything to make this work on an unsupported OS version.. 😜