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

[regression/8.0.99-ci.net8.24472.1] Invalidate Shadow Issue: VisualElement.Shadow property change no longer invalidates platform shadow #24882

Closed
Stedy59 opened this issue Sep 23, 2024 · 10 comments · Fixed by #24990
Labels
area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing fixed-in-8.0.92 fixed-in-9.0.10 i/regression This issue described a confirmed regression on a currently supported version p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint platform/android 🤖 s/needs-attention Issue has more information and needs another look t/bug Something isn't working
Milestone

Comments

@Stedy59
Copy link

Stedy59 commented Sep 23, 2024

Description

Changes to a control's "shadow property" or changes to the shadow's properties directly no longer invalidate the platforms shadow.

Steps to Reproduce

No response

Link to public reproduction project repository

No response

Version with bug

8.0.90 SR9

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

8.0.70 SR7

Affected platforms

Android

Affected platform versions

Android - All

Did you find any workaround?

No response

Relevant log output

No response

@Stedy59 Stedy59 added the t/bug Something isn't working label Sep 23, 2024
Copy link
Contributor

Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you!

Open similar issues:

Closed similar issues:

Note: You can give me feedback by thumbs upping or thumbs downing this comment.

@Stedy59
Copy link
Author

Stedy59 commented Sep 23, 2024

Working in build 8.0.70-ci.net8.24321.1

screencapture.8.0.70-ci.net8.24321.1.mp4

No shadow invalidate in build 8.0.99-ci.net8.24472.1

NOTE - my custom handler for the dropdown receives the bound property change and modifies the AppCompatTextViews Shadow.

The only way to force the invalidate was to change the font on the views controls, or change the Shadow property to null, then provide a new shadow. ;(

screencapture.8.0.99-ci.net8.24472.1.mp4

@jfversluis jfversluis added the s/needs-repro Attach a solution or code which reproduces the issue label Sep 24, 2024
@PureWeen PureWeen added potential-regression This issue described a possible regression on a currently supported version., verification pending area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing labels Sep 24, 2024
@albyrock87
Copy link
Contributor

albyrock87 commented Sep 24, 2024

I've tested this (on iOS) with b2c1d25 and also e0b3526 => it works fine:

	private void ChangeShadowClicked(object sender, EventArgs e)
	{
		var button = (View)TheButton.Parent;
		button.Shadow.Radius += 8;
	}
<Border
        HorizontalOptions="Center"
        BackgroundColor="LightBlue"
        Shadow="{Shadow Brush=Black, Offset='0,2', Radius=24, Opacity=0.2}"
        StrokeShape="{RoundRectangle CornerRadius=20}">
        <Button x:Name="TheButton" BackgroundColor="LightGreen" WidthRequest="200" HeightRequest="400" />
</Border>

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-repro Attach a solution or code which reproduces the issue labels Sep 24, 2024
@PureWeen PureWeen added platform/android 🤖 s/needs-info Issue needs more info from the author and removed s/needs-attention Issue has more information and needs another look labels Sep 24, 2024
@Stedy59
Copy link
Author

Stedy59 commented Sep 25, 2024

I've tested this (on iOS) with b2c1d25 and also e0b3526 => it works fine:

	private void ChangeShadowClicked(object sender, EventArgs e)
	{
		var button = (View)TheButton.Parent;
		button.Shadow.Radius += 8;
	}
<Border
        HorizontalOptions="Center"
        BackgroundColor="LightBlue"
        Shadow="{Shadow Brush=Black, Offset='0,2', Radius=24, Opacity=0.2}"
        StrokeShape="{RoundRectangle CornerRadius=20}">
        <Button x:Name="TheButton" BackgroundColor="LightGreen" WidthRequest="200" HeightRequest="400" />
</Border>

Issue is with Android... Not IOS. Thanks.

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Sep 25, 2024
@albyrock87
Copy link
Contributor

@Stedy59 reading Affected platform versions / Android - All confused me, sorry :)

I've opened a PR to fix the issue :)

@rmarinho rmarinho added the p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint label Sep 27, 2024
@rmarinho rmarinho added this to the .NET 8 SR9 milestone Sep 27, 2024
@albyrock87
Copy link
Contributor

For reference this is very likely what broke it:
#24222

@albyrock87
Copy link
Contributor

@rmarinho I suggest you to revert #24222 which broke:

  • shadow changes
  • clip changes

@jonathanpeppers setHasShadow and setHasClip are misleading names.. an invalidation is always expected there in order to update drawing upon changes.
I think we we could call invalidate instead of postInvalidate there considering MAUI will call those methods only from the main thread.

@jonathanpeppers
Copy link
Member

@albyrock87 can you send a PR that just calls invalidate() instead? The slowdown was how postInvalidate() was queuing up so many callbacks. The sample was queuing up 100s of those.

@albyrock87
Copy link
Contributor

@jonathanpeppers yes, I can create a simple PR just with that code.
Someone from the team will have to regenerate the .aar after it's merged though (for security reasons).

@albyrock87
Copy link
Contributor

albyrock87 commented Sep 29, 2024

@github-project-automation github-project-automation bot moved this from Ready To Review to Done in MAUI SDK Ongoing Oct 10, 2024
@PureWeen PureWeen added i/regression This issue described a confirmed regression on a currently supported version and removed potential-regression This issue described a possible regression on a currently supported version., verification pending labels Oct 15, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing fixed-in-8.0.92 fixed-in-9.0.10 i/regression This issue described a confirmed regression on a currently supported version p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint platform/android 🤖 s/needs-attention Issue has more information and needs another look t/bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants