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

Layout cycle crash on Windows with Button with image inside FlexLayout #21970

Closed
jeremy-visionaid opened this issue Apr 22, 2024 · 13 comments
Closed
Assignees
Labels
area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter i/regression This issue described a confirmed regression on a currently supported version platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@jeremy-visionaid
Copy link
Contributor

Description

Reopen #21548

Steps to Reproduce

Reopen #21548

Link to public reproduction project repository

No response

Version with bug

8.0.20 SR4

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

8.0.7 SR2

Affected platforms

Windows

Affected platform versions

No response

Did you find any workaround?

No response

Relevant log output

No response

@jeremy-visionaid jeremy-visionaid added the t/bug Something isn't working label Apr 22, 2024
@jeremy-visionaid
Copy link
Contributor Author

@Foda Please reopen #21548

@schlaman
Copy link

I have the same exception using FlexLayout and it works just fine with 8.0.14 but has the Layout Cycle crash with 8.0.20 (.NET Maui SR4).

This is a pretty severe bug. Will this be addressed quickly?

Thanks,
Craig

@Zhanglirong-Winnie Zhanglirong-Winnie added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Apr 22, 2024
@jeremy-visionaid
Copy link
Contributor Author

@schlaman Oh, that's interesting. The original bug pointed out that there were two manifestations of the problem. But it was closed permaturely since only one case (resizing) got fixed.

@Zhanglirong-Winnie
Copy link

Verified this issue with Visual Studio 17.10.0 Preview 4(8.0.20&8.0.14&8.0.10). Can repro on windows platform with sample project.
https://github.com/molesmoke/MauiTestApp/tree/flexlayout-cycle
8.0.10&8.0.14&8.0.20
image
8.0.7
image

@Zhanglirong-Winnie Zhanglirong-Winnie added the i/regression This issue described a confirmed regression on a currently supported version label Apr 22, 2024
@schlaman
Copy link

Awesome that you have it repro'd!

@jeremy-visionaid
Copy link
Contributor Author

I also wrote a unit test to prevent regressions (currently with a Skip directive for Windows since it fails):
#21651

@schlaman
Copy link

Totally love the regression prevention!! I appreciate you being on this so quick. I'm about ready to release a major product I've worked on for the last year and really rely on .NET Maui being solid. I totally love what it has become :)

@PureWeen
Copy link
Member

Can you test with the latest nightly?

https://github.com/dotnet/maui/wiki/Nightly-Builds

@mattleibow
Copy link
Member

I wonder if this is fixed by #21737

@jeremy-visionaid
Copy link
Contributor Author

jeremy-visionaid commented Apr 22, 2024

@PureWeen @mattleibow I can do better than that, the referenced device test (#21651) is still failing on main (Although it's currently manifesting as a hang, you can still see the layout cycle messages in the debug output)

@PureWeen PureWeen added this to the .NET 8 SR5 milestone Apr 23, 2024
@Eilon Eilon added the area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter label Apr 26, 2024
@BurkusCat
Copy link
Contributor

BurkusCat commented Apr 30, 2024

This was something I was going to post myself a few weeks ago but it appeared to go away / not be in the release builds. Now I am getting it consistently and in the release builds in 8.0.20 SR4 (I think I got it in previous versions too).

I'm specifically getting the issue when the Buttons have a Margin applied to them. The crash goes away when there are no margins.

Here is a workaround I'm going with for the time being where I had some ContentViews to space things out. It will look bad in some scenarios but to be honest my FlexLayouts kind of looked bad already so it isn't too much worse for me 😂

<FlexLayout>
    <Button />
    <ContentView WidthRequest="5" />
    <Button />
    <ContentView WidthRequest="5" />
    <Button />
</FlexLayout>

@jeremy-visionaid in your original sample, I can workaround the issue by removing MinimumHeightRequest and MinimumWidthRequest from the global Button style. I'm guessing there will be quite a number of different triggers for the same exception.

@jeremy-visionaid
Copy link
Contributor Author

@BurkusCat Looks like @Foda reworked the device test I wrote for it into #21733, so I'm hopeful this will all go away when it gets merged. But setting both MaximumWidthRequest and MaximumHeightRequest on the Buttons does prevent the layout cycle crash for me as a workaround.

@PureWeen PureWeen moved this from Todo to In Progress in MAUI SDK Ongoing May 8, 2024
@PureWeen PureWeen modified the milestones: .NET 8 SR5, .NET 8 SR6 May 10, 2024
Foda added a commit that referenced this issue May 15, 2024
### Description of Change

This PR forces images inside buttons to always fit inside the available
space, rather than expand be clipped

### Issues Fixed

Fixes #21683
#21970
@Foda Foda closed this as completed May 28, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in MAUI SDK Ongoing May 28, 2024
@BurkusCat
Copy link
Contributor

In MAUI 8.0.60 this appears to be fixed. I was able to remove the workaround and things look a lot nicer now 👍 Thank you

@github-actions github-actions bot locked and limited conversation to collaborators Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter i/regression This issue described a confirmed regression on a currently supported version platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

9 participants