[Windows] Fix for setting IsClippedToBounds="True" inside a Border control will crash on Windows

[Tamilarasan-Paranthaman]

Root Cause

1. The Clip property is applied to the LayoutPanel, and then Composition.Visual.Clip is applied to the content (LayoutPanel) within the ContentPanel class. As a result, setting both the Clip property and Composition.Visual.Clip on a panel causes an exception.

Description of Change

• Fixed the issue in the LayoutPanel by skipping the clipping when the parent is a Border. In such cases, ContentPanel will now handle clipping.

Issues Fixed

Fixes #18937

Screenshot

Before Issue Fix	After Issue Fix
ClippedToBoundsIssue.mp4	ClippedToBoundsFixed.mp4

[dotnet-policy-service]

Hey there @Tamilarasan-Paranthaman! 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]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[jsuarezruiz]

Could avoid async void? Because error propagation and also the call stack may not accurately represent the execution flow at the time the exception was thrown, complicating the process of identifying and fixing bugs.

Return a Task and rename the method to UpdateClipAsync.

[Tamilarasan-Paranthaman]

@jsuarezruiz, I have renamed the method to UpdateClipAsync and changed its return type to Task. Could you please check once and let me know if you have any concerns?

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[Foda]

I don't think this is the right way to approach this problem. Let me try and ping some WinAppSDK folks and see if they have any suggestions.

In the meantime: the Loaded event should be fired after layout is complete on a Visual -- maybe you can check if that has fired first before assigning the clip?

[Foda]

WinAppSDK folks also suggested to try switching SizeChanged to LayoutUpdated

[Tamilarasan-Paranthaman]

I don't think this is the right way to approach this problem. Let me try and ping some WinAppSDK folks and see if they have any suggestions.

In the meantime: the Loaded event should be fired after layout is complete on a Visual -- maybe you can check if that has fired first before assigning the clip?

With our PR fix, the Loaded event is fired before assigning the clip, ensuring that the layout is completed on the visual before applying the clip.

[Tamilarasan-Paranthaman]

WinAppSDK folks also suggested to try switching SizeChanged to LayoutUpdated

We ensured the functionality by calling the UpdateClip method on the LayoutUpdated event, as LayoutUpdated triggers frequently whenever the layout is updated. However, we only need to update the clip during the initial load and when the size changes. To address this, we added flags to limit updates to these specific scenarios. Despite this, after switching from the SizeChanged event to the LayoutUpdated event, the issue persists.

[Foda]

I've narrowed down the root cause of the issue, and reported it here: https://github.com/microsoft/WindowsAppSDK/issues/4914

We can remove all use of async. To fix this:

In LayoutPanel.cs change ArrangeOverride to use Composition.Visual.Clip:

protected override WSize ArrangeOverride(WSize finalSize)
{
	var actual = base.ArrangeOverride(finalSize);
	var visual = ElementCompositionPreview.GetElementVisual(this);
	visual.Clip = ClipsToBounds ? visual.Compositor.CreateInsetClip() : null;

	return actual;
}

Then, add a check in ContentPanel.cs to ensure the clip isn't overridden:

void UpdateClip(IShape? borderShape, double width, double height)
{
	if (Content is null)
	{
		return;
	}

	if (height <= 0 && width <= 0)
	{
		return;
	}

        // Ensure clip isn't overridden on LayoutPanel
	if (Content is LayoutPanel panel && panel.ClipsToBounds)
	{
		return;
	}

[Tamilarasan-Paranthaman]

I've narrowed down the root cause of the issue, and reported it here: https://github.com/microsoft/WindowsAppSDK/issues/4914

We can remove all use of async. To fix this:

In LayoutPanel.cs change ArrangeOverride to use Composition.Visual.Clip:

protected override WSize ArrangeOverride(WSize finalSize)
{
	var actual = base.ArrangeOverride(finalSize);
	var visual = ElementCompositionPreview.GetElementVisual(this);
	visual.Clip = ClipsToBounds ? visual.Compositor.CreateInsetClip() : null;

	return actual;
}

Then, add a check in ContentPanel.cs to ensure the clip isn't overridden:

void UpdateClip(IShape? borderShape, double width, double height)
{
	if (Content is null)
	{
		return;
	}

	if (height <= 0 && width <= 0)
	{
		return;
	}

        // Ensure clip isn't overridden on LayoutPanel
	if (Content is LayoutPanel panel && panel.ClipsToBounds)
	{
		return;
	}

As per the suggested fix, I tested it using other shapes in the border, such as Ellipse and RoundRectangle. I observed that clipping was not properly applied due to the clipping logic already being handled in the LayoutPanel and returned in the UpdateClip method of the ContentPanel class.

As a result, the additional clipping code in the UpdateClip based on the border shape was not executed, leading to improper clipping in the UI. To address this, I fixed the issue in the LayoutPanel by skipping the clipping when the parent is a Border. In such cases, clipping will now be handled by the ContentPanel.

Could you please review the changes and let me know if you have any concerns?

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).