Skip to content

Optimize DP setter codegen for Color, CornerRadius, and Thickness on WinAppSDK - #809

Merged
Sergio0694 merged 4 commits into
mainfrom
user/sergiopedri/xamlsetters-updates
Sep 14, 2026
Merged

Sergio0694 merged 4 commits into
mainfrom
user/sergiopedri/xamlsetters-updates

Conversation

@Sergio0694

Copy link
Copy Markdown
Member

Follow-up to the previous XamlBindingHelper-based DP setter optimizations.

The latest version of WinAppSDK exposes three new static methods on Microsoft.UI.Xaml.Markup.XamlBindingHelper:

  • SetPropertyFromColor(object, DependencyProperty, Windows.UI.Color)
  • SetPropertyFromCornerRadius(object, DependencyProperty, Microsoft.UI.Xaml.CornerRadius)
  • SetPropertyFromThickness(object, DependencyProperty, Microsoft.UI.Xaml.Thickness)

These extend the optimized setter path the generator already uses for other well-known WinRT-projected types, but they (1) only exist on WinAppSDK (the UWP Windows.UI.Xaml.Markup.XamlBindingHelper has no equivalent) and (2) were only added in a recent WinAppSDK release. Consumers on older WinAppSDK versions must therefore not get codegen that calls them.

What changed

  • Execute.GetXamlBindingHelperSetMethodName now takes the current Compilation and the useWindowsUIXaml flag so it can probe the resolved XamlBindingHelper type for the new methods.
  • A new private Execute.HasXamlBindingHelperMethod helper resolves Microsoft.UI.Xaml.Markup.XamlBindingHelper (via WellKnownTypeNames.XamlBindingHelper(useWindowsUIXaml: false)) and verifies the full static void Method(object, DependencyProperty, T) shape, where T matches a caller-supplied metadata name. The strict signature check guards against any future overload with the same name but a different shape.
  • The well-known type-to-method tables are consolidated into two ReadOnlySpan<(string, string)> literals — one for the methods available on both UWP and WinAppSDK, and one for the new WinAppSDK-only methods. UWP short-circuits before the second loop, so no GetTypeByMetadataName work happens on the UWP path.
  • The single call site in DependencyPropertyGenerator.cs flows useWindowsUIXaml and context.SemanticModel.Compilation to the helper using named arguments.

Correctness / incremental notes

  • UWP (useWindowsUIXaml == true) is left completely untouched — the new code is gated behind !useWindowsUIXaml, and existing snapshot tests (which target UWP) are unaffected.
  • The probe runs inside the existing ForAttributeWithMetadataNameAndOptions transform, which already takes a non-equatable GeneratorAttributeSyntaxContext and produces equatable output. The resulting method name flows through the existing equatable DependencyPropertyInfo.XamlBindingHelperSetMethodName string, so downstream output correctly invalidates when a project upgrades its WinAppSDK reference.
  • Compilation.GetTypeByMetadataName is internally cached, and the probe only runs after the property type already matches one of the three new metadata names, so the per-property overhead is negligible.

Tests

No new tests are added: the new behavior depends on whether the host compilation references a WinAppSDK version that exposes these methods, and that cannot easily be toggled inside the existing snapshot test infrastructure. The change is purely additive on the WinAppSDK code path and leaves UWP/useWindowsUIXaml == true codegen unchanged, so existing test assertions continue to hold.

@Arlodotexe
Arlodotexe self-requested a review June 16, 2026 19:26
@MitchRazga

Copy link
Copy Markdown

On the "no new tests" note, WinUI 3 doesn't really seem to have good test coverage for XamlBindingHelper (or it just isn't public yet).
It shouldn't have to be the responsibility of the CommunityToolkit to cover these but maybe a few very simple
round-trip tests would be beneficial since some of these optimizations may have a different behavior than expected.

For example I don't think SetPropertyFromByte has actually ever worked as the name implies, or if the docs are just unclear.
It doesn't actually set a byte value, it looks like SetPropertyFromByteImpl calls CValueBoxer::BoxValue with a BYTE, but since CValueBoxer has no BYTE overload, and BOOLEAN is typedef BYTE, it binds the BOOLEAN one: box->SetBool(!!value). Meaning every non-zero byte is stored as true.

I'm happy to write some tests for this PR or in a follow up if you think they are worthwhile 😄

Sergio0694 and others added 4 commits September 14, 2026 09:32
Introduce a 'HasXamlBindingHelperMethod' helper that resolves the WinAppSDK 'Microsoft.UI.Xaml.Markup.XamlBindingHelper' type from the current 'Compilation' and verifies that a given static 'SetPropertyFrom*' method exists with the expected '(object, DependencyProperty, T)' signature.

This unblocks gating optimized codegen on the availability of newer 'XamlBindingHelper' overloads that were added in recent WinAppSDK versions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extend the optimized 'XamlBindingHelper.SetPropertyFrom*' codegen path to also cover `Color`, `CornerRadius`, and `Thickness` property types when targeting WinAppSDK.

The corresponding 'SetPropertyFromColor', 'SetPropertyFromCornerRadius', and 'SetPropertyFromThickness' static methods on 'Microsoft.UI.Xaml.Markup.XamlBindingHelper' were added in WinAppSDK 1.6, and have no equivalent on the UWP 'Windows.UI.Xaml.Markup.XamlBindingHelper'.

To avoid breaking codegen when consumers target older WinAppSDK versions, 'GetXamlBindingHelperSetMethodName' now takes the current 'Compilation' and 'useWindowsUIXaml' flag and uses 'HasXamlBindingHelperMethod' to opt in to the new methods only when they actually exist with the expected signature.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Consolidate repetitive type checks into ReadOnlySpan<(string,string)> loops to map well-known types to their corresponding XamlBindingHelper SetPropertyFrom* methods. Add an early return when useWindowsUIXaml is false to avoid probing WinAppSDK-only APIs on UWP. Combine type name checks with HasXamlBindingHelperMethod calls for WinAppSDK-only types. Also update the call site to use named arguments (typeSymbol, useWindowsUIXaml, compilation) for clarity.
…hable

The early-return guard introduced in the previous commit used '!useWindowsUIXaml', which short-circuited on WinAppSDK and skipped the new 'SetPropertyFromColor', 'SetPropertyFromCornerRadius', and 'SetPropertyFromThickness' probes entirely (the opposite of the intent stated in the inline comment, and of the actual feature this code is meant to enable).

Flip the condition to 'useWindowsUIXaml' so we bail out on UWP (which never gets these methods) and continue probing on WinAppSDK.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Sergio0694
Sergio0694 force-pushed the user/sergiopedri/xamlsetters-updates branch from 51dbd2e to 7d98da6 Compare September 14, 2026 16:32

@Arlodotexe Arlodotexe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the code line-by-line, no questions or critiques. @Sergio0694 If you've tested it yourself already via a local ProjectReference or via the PR feed, we're good to merge and ship to the Labs NuGet feed and prerelease packages on nuget.org.

@Arlodotexe
Arlodotexe disabled auto-merge September 14, 2026 17:37
@Arlodotexe

Arlodotexe commented Sep 14, 2026

Copy link
Copy Markdown
Member

Disabling auto-close to confirm it's been tested with OP, versions are derived from the date so we can only ship a package update once per day per component.

@Sergio0694

Copy link
Copy Markdown
Member Author

Yeah I think I did test this back then. I'll merge this so I can rebase the other one and continue doing changes.

@Sergio0694
Sergio0694 merged commit adce7d9 into main Sep 14, 2026
24 checks passed
@Sergio0694
Sergio0694 deleted the user/sergiopedri/xamlsetters-updates branch September 14, 2026 18:30
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.

4 participants