[Impeller] Reland: remove most temporary allocation during polyline generation. #52180
Conversation
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. |
|
One of these goldens looks wrong and the other didn't show up, investigating. |
|
Should be fixed. THe stroke path issue was different than I thought so I pulled that change out of this PR. |
|
|
||
| // Note: the origin point is repeated but not referenced in the indices | ||
| // below | ||
| std::vector<Point> expected = {{0, 0}, {10, 0}, {10, 10}, {0, 10}, {0, 0}}; |
There was a problem hiding this comment.
The change here is that the first point is repeated if a contour is closed, where as it isn't repeated if it isn't closed. But we don't reference the first point multiple times (in the index buffer) so the triangles are the same.
This is because we need to make sure that a filled shape without an explicit close includes the first point.
|
Golden file changes are available for triage from new commit, Click here to view. |
|
PTAL @chinmaygarde |
jonahwilliams
added
the
autosubmit
…polyline generation. (flutter/engine#52180)
…147005) flutter/engine@6abfa56...46ff024 2024-04-18 [email protected] [Impeller] Reland: remove most temporary allocation during polyline generation. (flutter/engine#52180) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…lyline generation. (flutter#52180)" This reverts commit 46ff024.
…lutter#147005) flutter/engine@6abfa56...46ff024 2024-04-18 [email protected] [Impeller] Reland: remove most temporary allocation during polyline generation. (flutter/engine#52180) 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
|
Noticed an issue in the benchmark results related to this PR before it was reverted. The If this is relanded, then |
Part of flutter/flutter#143077
Only allocate into reused arenas instead of allocating a new vector of data. Fixes flutter/flutter#133348
Also moves tessellation logic into the c/tessellator and out of the impeller tessellator. This was necessary to fix a compilation error. introduced by including host_buffer -> allocator -> fml mapping -> window.h include which has a function definition that conflicts with the c tessellator definition.
Fixes missing points in case a filled path is not explicitly closed.