[Impeller] copy data out of DlVertices. #45355
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
I manually verified the existing golden tests are still valid, and I believe these run on CI too. |
There was a problem hiding this comment.
I wonder if it makes sense to mark this branched from dl_dispatcher?
There was a problem hiding this comment.
I'm going to pass on doing this.
|
Ah, my fix was just going to be to have a shared_ptr to DlVertices in the object and to use DlVertices::Make() to make a new copy. I didn't look too closely, but this looks like it is saving an extra step as well. |
jonahwilliams
added
the
autosubmit
…133825) flutter/engine@494fd7f...f28eb11 2023-08-31 [email protected] [Impeller] copy data out of DlVertices. (flutter/engine#45355) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
The implementation of DLVerticesGeometry is holding onto a raw ptr that it doesn't own nor does it know that the lifecycle is deterministic. My bad!
This mostly worked (ehhh) until the stopwatch visualizer change. I've confirmed that going back to copying the data fixes the issue with the stopwatch visualizer.
Fixes flutter/flutter#133813