Skip to content

Impeller will stroke paths directly from the original path via iteration - #167422

Merged
auto-submit[bot] merged 16 commits into
flutter:masterfrom
flar:impeller-stroke-iterated-paths
Apr 25, 2025
Merged

Impeller will stroke paths directly from the original path via iteration#167422
auto-submit[bot] merged 16 commits into
flutter:masterfrom
flar:impeller-stroke-iterated-paths

Conversation

@flar

@flarflar commented Apr 18, 2025

Copy link
Copy Markdown
Contributor

Currently stroking paths in Impeller goes through a few steps:

  • [Once per path object, if reused] If necessary, convert a source Skia path to an Impeller path
  • Ask the Impeller path object to create a polyline, with only line segments each potentially marked as "coming from a curve"
  • Widen the polyline treating curves specially
  • render the widened vertex list

This PR attempts to streamline that process by iterating the original curve and widening the strokes from that. It will require no conversion of the path from SkPath to impeller::Path (if it was created as an SkPath as is done for all ui.Path objects). It also avoids the intermediate polyline stage. It will also produce fewer outline vertices since it attempts to recognize which vertices are produced by the prior and future line segments and avoid duplicating them.

@github-actionsgithub-actionsBot added platform-android Android applications specifically platform-ios iOS applications specifically engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests team-ios Owned by iOS platform team labels Apr 18, 2025
@flar

flar commented Apr 18, 2025

Copy link
Copy Markdown
ContributorAuthor

In the current form, the new code does take more time to do its work, but only slightly so if you take into account the conversion that used to happen on non-reused path objects. I suspect this can be improved by folding the path iterator and the "pruning segment iterator" into a single stage. It turns out that it is much faster if the path was originally generated as an SkPath (as is done for all ui.Path objects coming from Flutter) and only slower if the path was generated as an Impeller Path (only used from benchmarks and unit tests).

This PR is being submitted in its current form to see how it might impact the golden tests.

@flar

flar commented Apr 18, 2025

Copy link
Copy Markdown
ContributorAuthor

GiantStrokeAllocation test is making assumptions about internal vertex production that fail on the new code (mainly because it generates a lot fewer segments). I'll need to see if its assumption is even still valid and to adjust the values it is expecting as necessary.

@flar

flar commented Apr 20, 2025

Copy link
Copy Markdown
ContributorAuthor

It looks like the added time for the new code is entirely due to the DlPath impeller path iterator being slow. If I force the path to an SkPath in the benchmarks then the new code is faster than the old code.

Notes:

  • BM_StrokePathSkia - create stroked vertices directly from a skia-based DlPath iterator
  • BM_StrokePathImpeller - create stroked vertices directly from an impeller-based DlPath iterator
  • BM_StrokePolyline - create stroked vertices from an impeller Polyline (created outside the benchmark loop)
  • BM_StrokePath - same as Polyline, but the Polyline is created every time inside the loop
  • BM_StrokeConvertedPath - same as StrokePath but the DlPath is created every time inside the loop (similar to if the DlPath was not cached in the DisplayList)

Note that the last 2 variants (StrokeConvertedPath and StrokePath) are representative of the work that DrawPath has to do for the 2 cases of DrawPath if it encounters a DlPath for the first time, or for a subsequent time if the path is reused. The StrokePathSkia case which happens regardless of whether the DlPath has ever been seen before represents the full workflow as well and is faster than even the case where we are stroking an already converted Polyline. The StrokePathImpeller case is identical to the StrokePathSkia case except that we are internally using the impeller path iterator vs the Skia SkPath iterator and it is a mystery how it can be that much slower.

Note also that the StrokePathSkia and StrokePathImpeller cases represent a workflow that never has to convert the path from one format to the other (though as we see, iterating the impeller case might want to consider converting to a Skia path for faster iteration if we ever care about that case - we don't care because it is only ever used in unit tests and benchmarks, though). We might want to revisit the impeller iterator performance in a future PR, but since that case isn't used in production code (where ui.Path internally uses an SkPath), that work is of very low priority.

---------------------------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations UserCounters...
---------------------------------------------------------------------------------------------------------------
BM_StrokePathSkia/stroke_Cubic_Butt_Bevel 5561 ns 5560 ns 101687 SinglePointCount=570 TotalPointCount=57.9616M
BM_StrokePathSkia/stroke_Cubic_Butt_Miter 6230 ns 6229 ns 112700 SinglePointCount=659 TotalPointCount=74.2693M
BM_StrokePathSkia/stroke_Cubic_Butt_Round 7068 ns 7067 ns 99384 SinglePointCount=1.136k TotalPointCount=112.9M
BM_StrokePathSkia/stroke_Cubic_Square_Bevel 5589 ns 5589 ns 127180 SinglePointCount=594 TotalPointCount=75.5449M
BM_StrokePathSkia/stroke_Cubic_Round_Bevel 5770 ns 5770 ns 122616 SinglePointCount=654 TotalPointCount=80.1909M
BM_StrokePathImpeller/stroke_Cubic_Butt_Bevel 13230 ns 13230 ns 55481 SinglePointCount=570 TotalPointCount=31.6242M
BM_StrokePathImpeller/stroke_Cubic_Butt_Miter 13781 ns 13776 ns 51023 SinglePointCount=659 TotalPointCount=33.6242M
BM_StrokePathImpeller/stroke_Cubic_Butt_Round 15708 ns 15705 ns 45456 SinglePointCount=1.136k TotalPointCount=51.638M
BM_StrokePathImpeller/stroke_Cubic_Square_Bevel 13131 ns 13129 ns 55329 SinglePointCount=594 TotalPointCount=32.8654M
BM_StrokePathImpeller/stroke_Cubic_Round_Bevel 13450 ns 13450 ns 54614 SinglePointCount=654 TotalPointCount=35.7176M
BM_StrokePolyline/stroke_Cubic_Butt_Bevel 6702 ns 6701 ns 104660 SinglePointCount=1038 TotalPointCount=108.637M
BM_StrokePolyline/stroke_Cubic_Butt_Miter 7407 ns 7406 ns 98272 SinglePointCount=945 TotalPointCount=92.867M
BM_StrokePolyline/stroke_Cubic_Butt_Round 8893 ns 8893 ns 80426 SinglePointCount=1044 TotalPointCount=83.9647M
BM_StrokePolyline/stroke_Cubic_Square_Bevel 6743 ns 6736 ns 102597 SinglePointCount=1062 TotalPointCount=108.958M
BM_StrokePolyline/stroke_Cubic_Round_Bevel 6986 ns 6985 ns 100173 SinglePointCount=1.11k TotalPointCount=111.192M
BM_StrokePath/stroke_Cubic_Butt_Bevel 8944 ns 8944 ns 79645 SinglePointCount=1038 TotalPointCount=82.6715M
BM_StrokePath/stroke_Cubic_Butt_Miter 9630 ns 9630 ns 74774 SinglePointCount=945 TotalPointCount=70.6614M
BM_StrokePath/stroke_Cubic_Butt_Round 11193 ns 11193 ns 63399 SinglePointCount=1044 TotalPointCount=66.1886M
BM_StrokePath/stroke_Cubic_Square_Bevel 8916 ns 8916 ns 78098 SinglePointCount=1062 TotalPointCount=82.9401M
BM_StrokePath/stroke_Cubic_Round_Bevel 9279 ns 9276 ns 76157 SinglePointCount=1.11k TotalPointCount=84.5343M
BM_StrokeConvertedPath/stroke_Cubic_Butt_Bevel 10859 ns 10858 ns 64693 SinglePointCount=1038 TotalPointCount=67.1513M
BM_StrokeConvertedPath/stroke_Cubic_Butt_Miter 11661 ns 11660 ns 60607 SinglePointCount=945 TotalPointCount=57.2736M
BM_StrokeConvertedPath/stroke_Cubic_Butt_Round 13168 ns 13168 ns 53940 SinglePointCount=1044 TotalPointCount=56.3134M
BM_StrokeConvertedPath/stroke_Cubic_Square_Bevel 11050 ns 11050 ns 64374 SinglePointCount=1062 TotalPointCount=68.3652M
BM_StrokeConvertedPath/stroke_Cubic_Round_Bevel 11213 ns 11212 ns 61947 SinglePointCount=1.11k TotalPointCount=68.7612M

@flar
flar marked this pull request as ready for review April 22, 2025 23:45
@flar
flar requested a review from a team as a code ownerApril 22, 2025 23:45
@flutter-dashboard

Copy link
Copy Markdown

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.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #167422 at sha 4a99b3f

@flutter-dashboardflutter-dashboardBot added the will affect goldens Changes to golden files label Apr 22, 2025
@flar

flar commented Apr 22, 2025

Copy link
Copy Markdown
ContributorAuthor

Some results from the dynamic_path_stroke_tessellation_perf__timeline_summary macrobenchmark (A is the current engine, B is the engine from this PR):

 Score Average A (noise) Average B (noise) Speed-up
average_frame_build_time_millis 6.25 (1.63%) 6.47 (0.73%) 0.97x worst_frame_build_time_millis 8.12 (2.88%) 9.46 (4.86%) 0.86x 90th_percentile_frame_build_time_millis 6.97 (0.90%) 7.14 (0.01%) 0.98x 99th_percentile_frame_build_time_millis 7.70 (0.12%) 7.90 (0.22%) 0.97x average_frame_rasterizer_time_millis 8.84 (3.66%) 6.43 (0.33%) 1.37x worst_frame_rasterizer_time_millis 12.38 (14.55%) 9.03 (3.89%) 1.37x 90th_percentile_frame_rasterizer_time_millis 9.14 (3.32%) 6.80 (0.82%) 1.34x 99th_percentile_frame_rasterizer_time_millis 10.23 (1.73%) 7.77 (3.40%) 1.32x old_gen_gc_count 0.00 (0.00%) 0.00 (0.00%) NaNx average_vsync_transitions_missed 1.00 (0.00%) 0.50 (100.00%) 2.00x 90th_percentile_vsync_transitions_missed 1.00 (0.00%) 0.50 (100.00%) 2.00x 99th_percentile_vsync_transitions_missed 1.00 (0.00%) 0.50 (100.00%) 2.00x total_ui_gc_time 490.77 (1.71%) 536.20 (2.24%) 0.92x 30hz_frame_percentage 0.00 (0.00%) 0.00 (0.00%) NaNx 60hz_frame_percentage 100.00 (0.00%) 100.00 (0.00%) 1.00x 80hz_frame_percentage 0.00 (0.00%) 0.00 (0.00%) NaNx 90hz_frame_percentage 0.00 (0.00%) 0.00 (0.00%) NaNx 120hz_frame_percentage 0.00 (0.00%) 0.00 (0.00%) NaNx illegal_refresh_rate_frame_count 0.00 (0.00%) 0.00 (0.00%) NaNx average_gpu_frame_time 0.00 (0.00%) 0.00 (0.00%) NaNx 90th_percentile_gpu_frame_time 0.00 (0.00%) 0.00 (0.00%) NaNx 99th_percentile_gpu_frame_time 0.00 (0.00%) 0.00 (0.00%) NaNx worst_gpu_frame_time 0.00 (0.00%) 0.00 (0.00%) NaNx average_gpu_memory_mb 92.94 (0.00%) 88.94 (0.00%) 1.04x 90th_percentile_gpu_memory_mb 92.94 (0.00%) 88.94 (0.00%) 1.04x 99th_percentile_gpu_memory_mb 92.94 (0.00%) 88.94 (0.00%) 1.04x worst_gpu_memory_mb 92.94 (0.00%) 88.94 (0.00%) 1.04x 

@flutter-dashboard

Copy link
Copy Markdown

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #167422 at sha cdf647b

@flar

flar commented Apr 23, 2025

Copy link
Copy Markdown
ContributorAuthor

The material.m3_material.circular_progress_indicator.indeterminate goldens look like they truncate some of the shorter arcs. I'll want to fix that before this PR is done.

@flarflar changed the title initial prototype of Stroking paths directly from path iterationImpeller will stroke paths directly from the original path via iterationApr 23, 2025
@flutter-dashboard

Copy link
Copy Markdown

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #167422 at sha 3aa2c93

@flar

flar commented Apr 23, 2025

Copy link
Copy Markdown
ContributorAuthor

Looking through the last batch of golden diffs they all look pretty much within the realm of "slightly different math". There was one notable win for the new code: CanRenderQuadraticStrokeWithInstantTurn - on the upper left rounded corner

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 7, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e: impellerImpeller rendering backend issues and features requestsengineflutter/engine related. See also e: labels.platform-androidAndroid applications specificallyplatform-iosiOS applications specificallyteam-iosOwned by iOS platform teamwill affect goldensChanges to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@flar@jonahwilliams@gaaclarke