fluentui
fluentui copied to clipboard
[Bug]: fluent v9 Spinner from react-components package uses main thread for animation, causes jank under CPU load
Library
React Components / v9 (@fluentui/react-components)
System Info
N/A
Are you reporting Accessibility issue?
no
Reproduction
https://codesandbox.io/s/empty-sky-8dpd3l?file=/src/App.tsx
Bug Description
Actual Behavior
Under CPU load, the spinning animation produces jank and frames are dropped because the animation relies on the main thread to update.
Expected Behavior
The animation can be offloaded to the compositor and is free from jank regardless of the CPU load. (Note: this is how the spinner imported from fluentui/react@^8.0
behaves).
Let me know if it helps to provide profiles for both the good and the perf-impacted spinner. Happy to help where possible -- my alias is magostis. Thanks!
Logs
No response
Requested priority
Normal
Products/sites affected
No response
Are you willing to submit a PR to fix?
yes
Validations
- [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
- [X] The provided reproduction is a minimal reproducible example of the bug.
hey @mgostisha, thanks for report ! yeah please provide those profiles.
Here's the profiles for the v8 and v9 spinners.
Fluent v9 Spinner: https://trace.cafe/t/3FOODKrKWC Fluent v8 Spinner: https://trace.cafe/t/hNFRUbN6S6
For v9, you can see that it needs to go through the browser's calc/paint/layout steps to update the animation, whereas the v8 spinner does not.
This is exacerbated when you click on the "Do Work" button, which will emulate heavy CPU work. In the repro codesandbox, the v9 spinner is above the v8 spinner. Clicking that button will show animation jank in the v9 spinner, but not in the v8 one. Thanks for looking into this!
This is due to the v9 Spinner
using svg for the animation, which means that the animation can't be offloaded to the compositor. I will update the animation to not use svg so that it can leverage the compositor.
After further investigation of this issue, it is not as straightforward to change the animation from svg to something else while maintaining the same animation frames. @mgostisha You are welcome to submit a fix and contribute to the library if you would like.
After further investigation of this issue, it is not as straightforward to change the animation from svg to something else while maintaining the same animation frames. @mgostisha You are welcome to submit a fix and contribute to the library if you would like.
The issue itself isn't specifically that an SVG element is animating, it's that an SVG attribute is animating. SVG element animations can be compositor-only, as long the attributes being animated are compositor-only. Unfortunately, it's not very transparent or obvious which attributes are compositor-only, but this switch
in Chromium's code spells them out: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/animation/compositor_animations.cc;drc=d2b5c17eff35da6ebff8ba20c99688b87e1bc752;l=308
Since this spinner animation is essentially a rotate, it should be possible to create a compositor-only by layering SVG's and using rotate
animations.
Reopening as this is under active investigation again
@JustSlone is there anything else you need as far information to progress forward? happy to help look at a fix, it would just be a little bit.
@mgostisha We are working on a fix, with this PR: #30567