-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
[Impeller] Impeller RRect blur is slower than Skia's cached blur shadows. #148496
Comments
Debugging this a bit, the cost is evenly split between the rrect blur and the linear gradient shader. The ALU is limiting operations, which I'd expect for rrect blur but gradient is a bit surprising (well, not that surprising). So if we need an example of where some elbow grease is needed for rrect blur or gradient this is probably it. |
Ive been noticing increasing jank in list views, which was only very minor and so subtle as not noticeable in 3.10.x until now its more noticeable and mildly annoying in 3.22. I do have alot of gradients and blurs and images inside my listview |
@mark8044 Without more information (i.e. a full bug report) we won't be able to say if that is the same issue as here. |
@gaaclarke if you look at the limiters though, its ALU ops and not memory writes. So while reducing overdraw will help its not the cause of the slowness w.r.t. skia |
Yea I saw that. Simplifying this to one oval and one gradient |
Yea well my actual code uses different colors for each of the circles so I was rendering them each separate. I guess I could optimize this a bit so that it renders one with transformations and a color blend instead? However, this wont still fix the issue that this is still incredibly slow. This doesn't seem to be the case when rendering the exact same UI with Skia. |
Right, there may be ways to optimize the UI but we should still make a reasonable effort to be at least as fast as Skia. |
…cpu (#53007) issue: flutter/flutter#148496 The linked issue showed us as ALU bound with the linear gradient, this should relieve some of that pressure. Tests: There is already existing golden tests for this, this just changed the performance, no new functionality. ## summary of math improvements: - (num_colors - 1) * division -> multiplication - (num_colors - 1) * subtractions removed - removes 1 `dot()` - 1 division -> multiplication - 1 subtraction removed [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
I landed one patch that reduces the arithmetic cycles for the linear gradient. It does also bump the required number of registers slightly though. Once it has merged into the framework it would be nice if the OP could give it a look. I've spent some time looking into rrect_blur. The majority of the time comes from special functions (exp and sqrt). For the regular blur I was able to pre-calculate the gaussian blur coefficients on the CPU. It's difficult to do here since we are manipulating the blur based on the x and y values of the position, but I think it should be possible. I also briefly looked at our exp and sqrt usage.
Jonah reminded me we should see where we can take advantage of lower precision. So I'm keeping an eye out for that. I might end up having to move the code to the cpu so I can get a better idea of what sort of numbers we're working with. TLDR linear gradient should be faster, rrect_blur is a lot trickier to optimize. |
I transitioned rrect_blur to c++ so it can be examined for optimizations on the cpu: https://gist.github.com/gaaclarke/36ed9fad08f2a34a9230a61b328ea3cc |
I don't know if it's a fluke of my test, but the majority of the time |
I just tested out the change it looks a bit better now then it was before the changes but it is still very far off from the performance with Skia. In Skia there isn't any frame spikes when scrolling and it runs really smooth. However, in Impeller there is still really visible frame spikes and jitter when scrolling. Also just posting [✓] Flutter (Channel master, 3.23.0-9.0.pre.3, on macOS 14.5 23F79 darwin-arm64, locale en-US)
• Flutter version 3.23.0-9.0.pre.3 on channel master at /opt/homebrew/Caskroom/flutter/3.19.2/flutter
• Upstream repository https://github.com/flutter/flutter.git
• Framework revision a1a33e63b9 (6 hours ago), 2024-05-28 03:56:25 -0400
• Engine revision b1751088c7
• Dart version 3.5.0 (build 3.5.0-191.0.dev)
• DevTools version 2.36.0-dev.10 |
Thanks for checking, that version of flutter would have had the optimization at flutter/engine#53007. Possible further optimizations:
Beyond that I'm not sure. The linear gradient could be optimized for the number of colors, but more variants creates performance problems elsewhere. |
We can do much, much better with linear gradients: #148651 |
issue flutter/flutter#148496 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
issue: #148496 ![rrectblurbenchmark (2)](https://github.com/flutter/flutter/assets/30870216/ec849519-4619-4c2f-8bb3-8e0584c4566f) (I slightly tweaked the animation to make sure the large radius blurs aren't always at the bottom of the screen).
issue: flutter#148496 ![rrectblurbenchmark (2)](https://github.com/flutter/flutter/assets/30870216/ec849519-4619-4c2f-8bb3-8e0584c4566f) (I slightly tweaked the animation to make sure the large radius blurs aren't always at the bottom of the screen).
Numbers with flutter/engine#53166: Skia: 3.5ms So fixing gradients shaves off about half the time. To get the rest, we need to improve the performance of the rrect blur. |
#53166) Create a fast gradient variant that assumes vertex interpolation is sufficient to compute the gradient color. This follows the approximate design from flutter/flutter#148651 This is only implemented for draws that are exactly horizontal or vertical gradients on a rectangle with an identity effect transform. This could be easily extended to all horizontal or vertical gradients by extending the drawGeometry call to make the cover rect customizable. Handling diagonal gradients and effect transforms is more work but feasible. this is sufficient to make flutter/flutter#148496 about 2x as fast, at least for me. Applications with different gradients will not yet benefit.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Ahh so for the rrect shader, the difference with Skia is that Skia has a specialized circle case which is much simpler. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Fear not, I plan to ;) ... searching to see if I'm the only one with the problem is usually how I start debugging. When I ran without impeller, performance was back to normal.
I'll try to cut out a section of the app for a bug report. |
I added some more adjustments - recognizing that all of the mask filters are computing the same exact blur. This allows me to dedup all of the blurs into a single compuation, speeding this up to about ~3ms per frame. This is about 0.5 ms better than Skia, so assuming we can land something here we could call it good :) |
I haven't gotten back to this in a while. While we landed the changes to improve gradient performance, mask blur performance is still backed up behind other work |
Steps to reproduce
Code sample
Code sample
Performance profiling on master channel
Timeline Traces
Timeline Traces JSON
dart_devtools_2024-05-16_17 42 26.249.json.zip
Video demonstration
Video demonstration
[Upload media here]
What target platforms are you seeing this bug on?
iOS
OS/Browser name and version | Device information
iOS 17.4.1
IPhone 13 Pro Max
Does the problem occur on emulator/simulator as well as on physical devices?
Unknown
Is the problem only reproducible with Impeller?
Yes
Logs
Logs
[Paste your logs here]
Flutter Doctor output
Doctor output
The text was updated successfully, but these errors were encountered: