Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reflect default value of IsClippedToBounds property #15894

Closed
wants to merge 1 commit into from

Conversation

cat0363
Copy link
Contributor

@cat0363 cat0363 commented Jun 28, 2023

Description of Change

In PR #11352, the default value is true when the IsClippedToBounds property value on the iOS side is not set, but the default value when the IsClippedToBounds property value is not set on the Android side is Element.CornerRadius > 0f.

This PR unifies the different default values ​​for both iOS and Android.

If the IsClippedToBounds property is not specified on XAML, the default value is false as shown below.

[src\Controls\src\Core\Layout.cs]

public static readonly BindableProperty IsClippedToBoundsProperty =
    BindableProperty.Create(nameof(IsClippedToBounds), typeof(bool), typeof(Layout), false,
        propertyChanged: IsClippedToBoundsPropertyChanged);

Even in Xamarin.Forms, the default value of the Frame.IsClippedBounds property was false.

https://learn.microsoft.com/en-us/dotnet/api/xamarin.forms.layout.isclippedtobounds?view=xamarin-forms#xamarin-forms-layout-isclippedtobounds

At least the current behavior does not correctly reflect the default value of IsClippedToBoundsProperty.

Below is the source code before the change.

[src\Controls\src\Core\Compatibility\Handlers\iOS\FrameRenderer.cs]

public virtual void SetupLayer()
{
    omit ...

    _actualView.Layer.RasterizationScale = UIScreen.MainScreen.Scale;
    _actualView.Layer.ShouldRasterize = true;
    _actualView.Layer.MasksToBounds = Element.IsClippedToBoundsSet(true);
}

[src\Controls\src\Core\Compatibility\Handlers\Android\FrameRenderer.cs]

void UpdateClippedToBounds()
{
    if (Element == null)
        return;

    var shouldClip = Element.IsClippedToBoundsSet(Element.CornerRadius > 0f);

    this.SetClipToOutline(shouldClip);
}

Below is the modified source code.

[src\Controls\src\Core\Compatibility\Handlers\iOS\FrameRenderer.cs]

public virtual void SetupLayer()
{
    omit ...

    _actualView.Layer.RasterizationScale = UIScreen.MainScreen.Scale;
    _actualView.Layer.ShouldRasterize = true;
    _actualView.Layer.MasksToBounds = Element.IsClippedToBoundsSet(Element.IsClippedToBounds);
}

[src\Controls\src\Core\Compatibility\Handlers\Android\FrameRenderer.cs]

void UpdateClippedToBounds()
{
    if (Element == null)
        return;

    var shouldClip = Element.IsClippedToBoundsSet(Element.IsClippedToBounds);

    this.SetClipToOutline(shouldClip);
}

Applying this PR has implications if the IsClippedToBounds property is not specified on the XAML.

Since iOS expects true as the default value, applying this PR results in false, which is different from the expected result.

Android expects to be true when CornerRadius is greater than 0, so applying this PR will result in a different result than expected.

Either way, on both iOS and Android, the results are different from what you might have expected so far.

Before applying this PR, you should consider what the default value of IsClippedToBounds should be.
After consideration, this PR should be applied if the current default value of false for the IsClippedToBounds property is followed.

I'll leave it up to you how you decide, because it has a big impact.

From the above, the execution results for Android and iOS are as follows.

[Android IsClippedToBounds unspecified]

[Android IsClippedToBounds = "True"]

[Android IsClippedToBounds = "False"]

[iOS IsClippedToBounds unspecified]

[iOS IsClippedToBounds = "True"]

[iOS IsClippedToBounds = "False"]

Issues Fixed

Fixes #15029

@ghost ghost added the community ✨ Community Contribution label Jun 28, 2023
@ghost
Copy link

ghost commented Jun 28, 2023

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

@PureWeen PureWeen requested a review from tj-devel709 June 28, 2023 21:35
@tj-devel709
Copy link
Member

Hi @cat0363, thanks so much for your time and this PR! I am seeing slightly different results or may not be fully following your notes.

When testing on the Maui repo in main, I get these results with the following code snippet: https://gist.github.com/tj-devel709/710a9ed4f4c70d1a185e59721a1d2510
Screenshot 2023-07-05 at 6 08 55 PM
Screenshot 2023-07-05 at 6 10 17 PM

It looks like the behavior is the same for both platforms, but please let me know if I am missing something.
It also seems to me that Element.CornerRadius > 0f and true are not the same, but will give similar behavior to crop when we have a corner radius > 0 and that no cropping is necessary if there CornerRadius == 0.

Could you also provide a repro of the behavior you are seeing and let me know what version you are using.
Thank you!

@cat0363
Copy link
Contributor Author

cat0363 commented Jul 6, 2023

Hi, @tj-devel709
Thank you for your reply.
The version of .NET MAUI that I used when I reported the issue that this PR was based on is 7.0.86.
This PR was created by forking the source code of the last commit with revision number 329046ff on
June 21, 2023 7:12:19 from main, but the issue was confirmed with version 7.0.86 of .NET MAUI.

This is the result of verification based on the source code forked from main when this PR was created.

[iOS]
2023-07-06_10-03-47-午前

[Android]
Screenshot_1688603452

I got the same result as your verification result. The reason I didn't get the expected results was because
I was running version 7.0.86 of .NET MAUI. I was lacking verification with the latest source code.
If what you describe is the specification, no problem.

I understand from your explanation that whether or not to trim is the specification below if the value of the IsClipToBounds property is not specified.

[CornerRadius > 0f] => Clip
[CornerRadius == 0] => Not Clip

I didn't know this implicit specification.

If IsClippedToBounds is not specified, these documents do not describe whether or not the value of CornerRadius is clipped, so I think it is necessary to describe it.

https://learn.microsoft.com/en-us/dotnet/maui/user-interface/controls/frame
https://learn.microsoft.com/en-us/dotnet/api/microsoft.maui.controls.frame?view=net-maui-7.0

Thank you for sparing your valuable time.

@tj-devel709
Copy link
Member

@cat0363, thank you for verifying on your end.
What you are saying totally makes sense and I will pass on the word to see if we can be more explicit on the docs about the default behavior!

I will close this PR for now, but we can always re-open it later if needed or decided we want to go down this route! :)

@tj-devel709 tj-devel709 closed this Jul 6, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frame's IsClippedToBounds default value behavior is incorrect on Android.
2 participants