CommunityToolkit / Windows

Collection of controls for WinUI 2, WinUI 3, and Uno Platform developers. Simplifies and demonstrates common developer tasks building experiences for Windows with .NET.
https://aka.ms/windowstoolkitdocs
Other
430 stars 54 forks source link

SurfaceLoader fails in WinUI because of unsupported API usage #316

Open Marv51 opened 5 months ago

Marv51 commented 5 months ago

Describe the bug

In the latest Toolkit from Jan 24 (8.0.240109), SurfaceLoader fails to create a brush, and it will always return null on WinUI 3.

This is because the private SurfaceLoader.LoadSurfaceBrushAsync method unconditionally calls DisplayInformation.GetForCurrentView(). https://github.com/CommunityToolkit/Windows/blob/2735c8718984daa2ddee7a5aa9235f57bead0df7/components/Media/src/Helpers/SurfaceLoader.cs#L103 This is not supported on WinUI 3 and throws a COMException. This exception gets handled, but an empty/null brush will always be returned.

Steps to reproduce

// The Uri does not matter, it throws before loading anything.
_ = await SurfaceLoader.LoadImageAsync(new Uri("https://example.com"), CommunityToolkit.WinUI.Media.DpiMode.DisplayDpiWith96AsLowerBound);

// The exception is handled so it will return `null`.

Expected behavior

No exception is expected.

The old-7.1 winUI branch worked for me. On that branch this dpi issue was commented out:

https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/080bdb99e6810eb2157ad635f00e64a64acabb50/CommunityToolkit.WinUI.UI.Media/Helpers/SurfaceLoader.cs#L99

Screenshots

No response

Code Platform

Windows Build Number

Other Windows Build number

No response

App minimum and target SDK version

Other SDK version

No response

Visual Studio Version

2022

Visual Studio Build Number

No response

Device form factor

No response

Additional context

It might be tempting to think that this is the same issue as #233, however it is not. This is about 10 lines further into SurfaceLoader and this wasn't fixed by #299 in the latest update.

Help us help you

No, I'm unable to contribute a solution.

Marv51 commented 1 week ago

Should I submit a PR with the old 7.x "fix" of commenting out the DPI references?