[WinUI] Perf when creating non-trivial grids (and other visual controls) (take 2)

[MartyIX]

Description of Change

Depends on #21959

Alternative to #21818 which attempts to implement @jonathanpeppers's idea:

The part that makes me want to rethink this:

1. Every control has TranslateX/Y default to 0
2. Do we actually need to pass 0 to the platform? for every control?

I think there needs to be a concept of a default value
Where if you set TranslateX/Y it would pass to the platform and not otherwise
But I don't know if that breaks some fundamental design
this is just my observation
It would also be nice to get rid of one layer of Dictionary<string, lookup if that were possible

The idea of this PR is to introduce IView.IsPlatformViewNew which is true when there is a new platform view (which is known to have default values by default). So the first assignment of all properties can take into account this property and skip costly interop operations when an element is being added to the UI tree for the first time.

Relevant lines of code:

• https://github.com/dotnet/maui/pull/22108/files#diff-fe5090f560d95c536e43dfc5f39442add6ad2d8eb555d675bdb4a0f82a30793dR59 - set IView.IsPlatformViewNew to true.
• https://github.com/dotnet/maui/pull/22108/files#diff-fe5090f560d95c536e43dfc5f39442add6ad2d8eb555d675bdb4a0f82a30793dR92 - set IView.IsPlatformViewNew to false.
• https://github.com/dotnet/maui/pull/22108/files#diff-6e0bbf3ab78552fc393db84d62ff02aa472a933af77ca0c93362bd0dd3e703a3 - optimizations

Notes:

• Clicking "Batch Generate Grid" multiple times lead to ever increasing time to finish the operation. I'm not sure why.
• Relevant issue [performance] all PropertyMapper's set the same values over and over #17303

Performance

Main: 851 ms

PR: 559 ms

-> 34% faster.

Issues Fixed

Contributes to #21787

[MartyIX]

The name is terrible. It should be internal if possible.

[jonathanpeppers]

I don't think there is a way to make this internal if it is a property on IElement.

Is there a way this could detect if Handler was null and became non-null for the first time? Then maybe we wouldn't need a new bool property?

[MartyIX]

Is there a way this could detect if Handler was null and became non-null for the first time? Then maybe we wouldn't need a new bool property?

I don't think I can simulate it that way because I need to set IsPlatformViewNew to true and then back to false:

• https://github.com/dotnet/maui/pull/22108/files#diff-fe5090f560d95c536e43dfc5f39442add6ad2d8eb555d675bdb4a0f82a30793dR59 - set IView.IsPlatformViewNew to true.
• https://github.com/dotnet/maui/pull/22108/files#diff-fe5090f560d95c536e43dfc5f39442add6ad2d8eb555d675bdb4a0f82a30793dR92 - set IView.IsPlatformViewNew to false.

an alternative which I would consider the best (in an ideal world) would be to pass a flag to:

maui/src/Core/src/Handlers/Element/ElementHandler.cs

Line 79 in e33d176

_mapper.UpdateProperties(this, VirtualView);

but unfortunately then I would need to propagate that flag here:

maui/src/Core/src/PropertyMapper.cs

Line 47 in e33d176

action?.Invoke(viewHandler, virtualView);

and that would mean changing the way mappers are defined:

maui/src/Core/src/PropertyMapper.cs

Line 18 in e33d176

protected readonly Dictionary<string, Action<IElementHandler, IElement>> _mapper = new(StringComparer.Ordinal);

which seems like a very invasive thing to do (with many and many changes required). That's why I went with the IView.IsPlatformViewNew flag to avoid making changes to mappers.

[dotnet-policy-service]

Hey there @MartyIX! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[jsuarezruiz]

Based on the changes, needs to update also the Stubs used in the testing projects. For example:

D:\a\_work\1\s\src\Core\tests\DeviceTests.Shared\Stubs\WindowStub.cs(6,28): error CS0535: 'WindowStub' does not implement interface member 'IElement.IsPlatformViewNew' [D:\a\_work\1\s\src\Core\tests\DeviceTests.Shared\Core.DeviceTests.Shared.csproj::TargetFramework=net8.0-android]
    16 Error(s)

[MartyIX]

Based on the changes, needs to update also the Stubs used in the testing projects. For example:

I tried to fix it in b5d7287.

I believe that this PR requires a conceptual review because it's not clear (to me) whether the approach is OK or not.

[jonathanpeppers]

I don't think there is a way to make this internal if it is a property on IElement.

Is there a way this could detect if Handler was null and became non-null for the first time? Then maybe we wouldn't need a new bool property?

[jonathanpeppers]

Is the default for AutomationId null or string.Empty? I don't know the answer, so just checking.

[MartyIX]

I saw null when testing it so I used null but I will re-check.

[jonathanpeppers]

Is there a way this could do slightly better if someone set ScaleX/ScaleY, but not rotation or translation?

[MartyIX]

Personally, I think it would be a good start to extract changes from this file to a standalone PR because setting

maui/src/Core/src/Platform/Windows/TransformationExtensions.cs

Lines 21 to 22 in e33d176

	frameworkElement.RenderTransformOrigin = new global::Windows.Foundation.Point(anchorX, anchorY);
	frameworkElement.RenderTransform = new ScaleTransform { ScaleX = scaleX, ScaleY = scaleY };

and then setting

maui/src/Core/src/Platform/Windows/TransformationExtensions.cs

Line 28 in e33d176

frameworkElement.RenderTransform = null;

feels like RenderTransformOrigin does not be set at all in that situation and it also seems to be quite costly. I would need to re-measure it but I think this small change would have noticeable effect.

Is there a way this could do slightly better if someone set ScaleX/ScaleY, but not rotation or translation?

I'll check it.