Speed-up Border rendering by avoiding useless pass during size allocation

[albyrock87]

Description of Change

While updating the frame from Handler we set Width and Height.
Each of them triggers border mapping:

The first one is useless as the other dimension will change right after.
We can avoid this by batching the change on SizeAllocated:

maui/src/Controls/src/Core/VisualElement/VisualElement.cs

Lines 1718 to 1733 in 4b8e604

	void UpdateBoundsComponents(Rect bounds)
	{
	_frame = bounds;
	BatchBegin();
	X = bounds.X;
	Y = bounds.Y;
	Width = bounds.Width;
	Height = bounds.Height;
	SizeAllocated(Width, Height);
	SizeChanged?.Invoke(this, EventArgs.Empty);
	BatchCommit();
	}

This brings us to a better result:

Issues Fixed

Fixes #24843
Contributes to #24123

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[rmarinho]

Should this set to false on disconnect ?

[rmarinho]

Humm no, I was reading this wrong , but I m afraid that on reuse this "cache" could fail

[albyrock87]

@rmarinho what do you mean with reuse?

There is a very small window where this is true.

maui/src/Controls/src/Core/VisualElement/VisualElement.cs

Lines 1718 to 1733 in e0b3526

	void UpdateBoundsComponents(Rect bounds)
	{
	_frame = bounds;
	BatchBegin();
	X = bounds.X;
	Y = bounds.Y;
	Width = bounds.Width;
	Height = bounds.Height;
	SizeAllocated(Width, Height);
	SizeChanged?.Invoke(this, EventArgs.Empty);
	BatchCommit();
	}

Width = bounds.Width; // this eventually sets it to `true`
Height = bounds.Height; // this eventually sets it to `true`
// right after
SizeAllocated(Width, Height); // this sets it back to false

[rmarinho]

Reuse where on a NavigationStack on a page, if you for example push a modal, the border handler could get called a Disconnect and then Connect when the modal is popped and you get to the root page.

[albyrock87]

I must be missing something here.

I don't understand why that would make any difference.

Before this changes, everything depended on Width and/or Height property changed and that behavior has not changed with my PR.

[PureWeen]

Yea, I don't think it matters

If the handler disconnects and reconnects

Handler?.UpdateValue(nameof(IBorderStroke.Shape));

is going to run as part of the reconnect anyway.

[PureWeen]

is there a way this could move down into the handler?

OnSizeAllocated is basically called during the arrange pass when Handler?.PlatformArrange(Frame); is called.

[albyrock87]

Regarding moving this into the handler:

• MapFrame aka MappingFrame is basically static platform-specific code, so it's not a good idea to wire it up there: it's not always going through a specific handler method
  

  maui/src/Core/src/Handlers/View/ViewHandler.cs

  Lines 445 to 456 in 68524c8

  	static partial void MappingFrame(IViewHandler handler, IView view);
  	/// <summary>
  	/// Maps the abstract <see cref="IView.Frame"/> property to the platform-specific implementations.
  	/// </summary>
  	/// <param name="handler">The associated handler.</param>
  	/// <param name="view">The associated <see cref="IView"/> instance.</param>
  	/// <param name="args">The arguments passed associated to this event.</param>
  	public static void MapFrame(IViewHandler handler, IView view, object? args)
  	{
  	MappingFrame(handler, view);
  	}

• PlatformArrange on the border handler is used to arrange border's content, so it happens after its container has arranged the Border, so also in this case it's not good put that code here (it doesn't work - look how the border radius is messed up here)
  

I see nothing wrong in invoking the handler on property changed, because it's exactly what happens on everything else.

maui/src/Controls/src/Core/Element/Element.cs

Lines 628 to 644 in 68524c8

	/// <summary>Method that is called when a bound property is changed.</summary>
	/// <param name="propertyName">The name of the bound property that changed.</param>
	protected override void OnPropertyChanged([CallerMemberName] string propertyName = null)
	{
	base.OnPropertyChanged(propertyName);
	UpdateHandlerValue(propertyName);
	if (_effects?.Count > 0)
	{
	var args = new PropertyChangedEventArgs(propertyName);
	foreach (Effect effect in _effects)
	{
	effect?.SendOnElementPropertyChanged(args);
	}
	}
	}

[albyrock87]

I've just rebased onto main to solve conflicts on PublicAPI.

[rmarinho]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[bcaceiro]

This looks quite promising. MAjority of CollectionViews have as a base element a border, will this be available in net8 or net9?

[PureWeen]

Let me know your thoughts
https://github.com/dotnet/maui/tree/PureWeen/reduce-border-mappping-calls

[albyrock87]

@PureWeen it works, even if I don't know why it didn't when I tried weeks ago.
I've updated the PR accordingly. Thanks.

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).