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

Implicit Animation causes dead lock. #4038

Closed
HppZ opened this issue May 20, 2021 · 14 comments
Closed

Implicit Animation causes dead lock. #4038

HppZ opened this issue May 20, 2021 · 14 comments
Labels
animations 🏮 bug 🐛 An unexpected issue that highlights incorrect behavior external ⤴️ Requires an update to an external dependency or due to code outside the Toolkit.
Milestone

Comments

@HppZ
Copy link

HppZ commented May 20, 2021

Describe the bug

Implicit Animation causes dead lock.
image

Steps to Reproduce

https://github.com/HppZ/ImplicitAnimationBug

Steps to reproduce the behavior:
click Button, click 'show toast', repeat until the bug out

Expected behavior

no dead lock.

Environment

NuGet Package(s):

Package Version(s):
Microsoft.Toolkit.Uwp.UI.Animations 6.1.1

Windows 10 Build Number:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [ ] May 2019 Update (18362)
- [x] May 2020 Update (19041)
- [ ] Insider Build (build number: )

App min and target version:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [x] October 2018 Update (17763)
- [ ] May 2019 Update (18362)
- [ ] May 2020 Update (19041)
- [ ] Insider Build (xxxxx)

Device form factor:
- [x] Desktop
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

Visual Studio
- [ ] 2017 (version: )
- [x] 2019 (version: 16.9.4)
- [ ] 2019 Preview (version: )

Additional context

Add any other context about the problem here.

@HppZ HppZ added the bug 🐛 An unexpected issue that highlights incorrect behavior label May 20, 2021
@ghost ghost added the needs triage 🔍 label May 20, 2021
@ghost
Copy link

ghost commented May 20, 2021

Hello HppZ, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@Kyaa-dost
Copy link
Contributor

@HppZ If you can provide more detail on the repro step by step that would be great. Thanks!

@HppZ
Copy link
Author

HppZ commented May 21, 2021

Steps to reproduce the behavior:
click Button, click 'show toast', repeat until the bug out

you can't reproduce the bug using the repo?

@HppZ
Copy link
Author

HppZ commented May 21, 2021

动画

@Kyaa-dost
Copy link
Contributor

Kyaa-dost commented May 21, 2021

@HppZ Thanks for showcasing it via gif and I am able to repro. Do you want to work on this fix and create a PR?

@michael-hawker
Copy link
Member

FYI @Sergio0694

@michael-hawker michael-hawker added this to the 7.1 milestone May 21, 2021
@HppZ
Copy link
Author

HppZ commented May 24, 2021

how is this going?

@Sergio0694
Copy link
Member

It's only been 4 days, and there was also the weekend among those 4 days. I'll take a look at this later today 🙂

@HppZ
Copy link
Author

HppZ commented May 24, 2021

Thanks~

@Sergio0694
Copy link
Member

Alright I have a small update, I don't think the bug is caused by the Toolkit per se.
I've removed the usage of those implicit animations entirely and just manually set them up via the Composition Animations:

public static void SetupImplicitShowAnimations(UIElement target)
{
    Compositor compositor = ElementCompositionPreview.GetElementVisual(target).Compositor;
    CompositionAnimationGroup animations = compositor.CreateAnimationGroup();

    ScalarKeyFrameAnimation scalarAnimation = compositor.CreateScalarKeyFrameAnimation();
    scalarAnimation.InsertKeyFrame(0.0f, 0f);
    scalarAnimation.InsertKeyFrame(1.0f, 1f);
    scalarAnimation.Duration = TimeSpan.FromSeconds(1);
    scalarAnimation.Target = nameof(Visual.Opacity);

    animations.Add(scalarAnimation);

    ElementCompositionPreview.SetImplicitShowAnimation(target, animations);
}

public static void SetupImplicitHideAnimations(UIElement target)
{
    Compositor compositor = ElementCompositionPreview.GetElementVisual(target).Compositor;
    CompositionAnimationGroup animations = compositor.CreateAnimationGroup();

    ScalarKeyFrameAnimation scalarAnimation = compositor.CreateScalarKeyFrameAnimation();
    scalarAnimation.InsertKeyFrame(1.0f, 0f);
    scalarAnimation.Duration = TimeSpan.FromSeconds(1);
    scalarAnimation.Target = nameof(Visual.Opacity);

    animations.Add(scalarAnimation);

    ElementCompositionPreview.SetImplicitHideAnimation(target, animations);
}

And I got the same result. I also tried to invoke these two methods both when applying the template for the custom control, both just right from the constructor (so not on the root Border element in the template, but just on the control itself), and on the Popup as well. The issue always manifested in all of these cases, with the whole UI thread just blocking at some seemingly random point, and the entire app completely going unresponsive.

Marking this issue as external, as it seems to be related to some UWP XAML quirk and not the Toolkit itself.

To add another couple notes:

  • I've noticed that the implicit hide animation doesn't actually trigger at all when the popup is closed, if you apply them on the popup object directly. The show animation seems fine though. I find this a bit odd due to inconsistency, and I'm wondering if this is just showing some particular implementation detail of how popups are collapsed, which would result in this observed behavior. Might also be related to the issue in general, since the hang only seems to occur after at least one hide animation is triggered after the delay.
  • Triggering animations sequentially seems to work fine, as in, showing the popup, waiting for it to go away, then showing another, and repeating. This to me sounds like a concurrency issue somewhere in the XAML framework, and it reminds me of Loaded/Unloaded/IsLoaded APIs break down on quick swapping microsoft/microsoft-ui-xaml#1900. The issue only seems to repro if you're concurrently displaying some popups while others are still pending: after the oldest one is collapsed after the delay and its hide animations are triggered, it seems that showing other popups just causes the whole UI thread to hang.

cc. @michael-hawker I reckon either Mike or Miguel will be able to share some info on this, in case we wanted to ping them 🙂

@Sergio0694 Sergio0694 added the external ⤴️ Requires an update to an external dependency or due to code outside the Toolkit. label May 24, 2021
@michael-hawker
Copy link
Member

Thanks @Sergio0694 - think we should open a new WinUI issue with your example code or do you think it's related to the other issue you linked to?

FYI @marb2000 @MikeHillberg

@Sergio0694
Copy link
Member

@michael-hawker I think opening an issue in the WinUI repo will be useful to better track this. If anything, it can't hurt 😄
My guess is (though I'm not wary of the eexact internal details of how the XAML framework works) that the two issues might be related or at least have a similar root cause due to the similarities in how they present, but regardless I'd saying opening a separate issue makes sense given that the two scenarios are still relaatively different though. In this case the whole UI thread is hanging, and I can repro this even without changing the visibility of the popup, but just by showing/hiding it. In microsoft/microsoft-ui-xaml#1900 instead I was strictly toggling the visibility, and the result was an inconsistent ordering of the raised Loaded and Unloaded events, but with no actual UI hang. Also no animations were involved. This is all just my impression as an outsider, of course 🙂

@HppZ
Copy link
Author

HppZ commented May 25, 2021

microsoft/microsoft-ui-xaml#5051

@Sergio0694
Copy link
Member

Closing this as it was confirmed to be caused by a UWP XAML bug and not related to the Toolkit 🙂

@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
animations 🏮 bug 🐛 An unexpected issue that highlights incorrect behavior external ⤴️ Requires an update to an external dependency or due to code outside the Toolkit.
Projects
None yet
Development

No branches or pull requests

4 participants