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

CollectionView with ItemSpacing => Infinite layout loop Android #21379

Closed
rudyspano opened this issue Mar 22, 2024 · 8 comments
Closed

CollectionView with ItemSpacing => Infinite layout loop Android #21379

rudyspano opened this issue Mar 22, 2024 · 8 comments
Assignees
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 t/bug Something isn't working
Milestone

Comments

@rudyspano
Copy link

rudyspano commented Mar 22, 2024

Description

A CollectionView with an ItemSpacing of 1 i causes infinite changes of Cell content controls Height on Android.
As you can see in video and simple repro project

RefreshViewlayoutloop.mov

=> Huge performance Impact.

Reproducible since 8.0.10 then 8.0.14.
Not reproducible in 8.0.6

Simple repro project in attachment.
reprov2.zip

Height values imprecisions, probably due to Android px to MAUI dp conversion..?

Steps to Reproduce

See repro project.

Link to public reproduction project repository

No response

Version with bug

8.0.14 and 8.0.10

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI, Yes, this used to work in Xamarin.Forms

Last version that worked well

8.0.6 SR1

Affected platforms

Android

Affected platform versions

No response

Did you find any workaround?

Not using ItemSpacing

Relevant log output

No response

@rudyspano rudyspano added the t/bug Something isn't working label Mar 22, 2024
@PureWeen
Copy link
Member

PureWeen commented Mar 22, 2024

@rudyspano I ran your sample locally and I'm not seeing an infinite loop

The break point inside GridProblem_PropertyChanged never fires for me if I don't scroll the CV.

Also the Repro you've provided doesn't compile out of the box. It's missing a style, so perhaps the repro isn't correct?

@PureWeen PureWeen added platform/android 🤖 s/needs-info Issue needs more info from the author area-controls-collectionview CollectionView, CarouselView, IndicatorView potential-regression This issue described a possible regression on a currently supported version., verification pending labels Mar 22, 2024
@RoiChen001 RoiChen001 added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Mar 22, 2024
@rudyspano
Copy link
Author

rudyspano commented Mar 22, 2024

@PureWeen , I have updated the sample with a Console output.
I've tested my zip, if I just do a Retore Nuget package on solution, the project runs on Android.

Don't you see a lot of call of propertychanged with values near 158 (cell HeightRequest) ?

@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 Mar 22, 2024
@RoiChen001 RoiChen001 removed s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Mar 22, 2024
@PureWeen
Copy link
Member

PureWeen commented Mar 22, 2024

@rudyspano it looks like your repro didn't finish uploading

@PureWeen , I have updated the sample with a Console output. I've tested my zip, if I just do a Retore Nuget package on solution, the project runs on Android.

Don't you see a lot of call of propertychanged with values near 158 (cell HeightRequest) ?

No, the property changed event doesn't fire for me after everything has settled

@PureWeen PureWeen added s/needs-info Issue needs more info from the author and removed s/needs-attention Issue has more information and needs another look labels Mar 22, 2024
@PureWeen
Copy link
Member

Alright, I was able to reproduce in 8.0.7

@PureWeen PureWeen added i/regression This issue described a confirmed regression on a currently supported version and removed s/needs-info Issue needs more info from the author potential-regression This issue described a possible regression on a currently supported version., verification pending labels Mar 22, 2024
@PureWeen PureWeen added this to the .NET 8 SR4 milestone Mar 22, 2024
@PureWeen PureWeen self-assigned this Mar 22, 2024
@rudyspano
Copy link
Author

rudyspano commented Mar 22, 2024

@PureWeen , ok good news 🙂.
I will fix the repro monday. <= Updated
Latest version is 8.0.14? Right?
Why on Android, we do not have real values ? I know that Android uses pixels but when call stack switches to cross plateform unit, we should see 158 and not 157,8 or 158,1. I've seen other other properties that results in same behavior and that's very confusing for Debug sessions...

Thanks for your investigation.

@PureWeen
Copy link
Member

PureWeen commented Mar 27, 2024

@rudyspano can you test the latest nightly and see if you are still seeing this issue?

Can you test with the latest nightly build?
https://github.com/dotnet/maui/wiki/Nightly-Builds

@rudyspano
Copy link
Author

@PureWeen , I confirm that fixes the issue ! (in my repro and more complex project)

Is it also fixed by this PR https://github.com/dotnet/maui/pull/21409/files ?
Still, if I inspect Height value (console output), I see weird Height: 158.1818181818182 for a HeightRequest of 158

Thanks

@PureWeen
Copy link
Member

awesome thank you for testing @rudyspano!!

I think it was this PR that fixed it
#18856

I couldn't reproduce the issue when I was working on 21409 so that's my guess.

Does this issue match what you're curious about with measuring?
#21474

My guess is that what's happening with measure is that it's just a result of converting forward and backwards and after taking into account platform paddings/margins.

Though I don't know for sure.

I'm going to close this issue for now.
If the measuring part of this is causing you issues and isn't the same as 21474 can you log a new issue elaborating on your scenario a bit?

@github-actions github-actions bot locked and limited conversation to collaborators May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 t/bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

3 participants