[Impeller] Pack impeller:Path into 2 vecs instead of 3. #55028

jonahwilliams · 2024-09-07T17:04:32Z

This better aligns impeller::Path with how Skia::Path works, and probably gives us a route to a faster Skia path to Impeller::Path conversion (since dart:ui Path will be a Skia path for a long time).

We used 3 vectors to separately store:

Path points
Path Verbs
Path contours (index lookups for contour starts).

This gave us almost O(1) access for path points based on the verb. However, we don't ever actually use the lookup except for in tests. In all real scenarios we iterate through all path verbs, which means this iterators can simply increment the offset into the path points array. Finally, we can store the contour information in the path points array as well by representing the bool is_closed with another Point value :) (for easier alignment).

jonahwilliams · 2024-09-07T20:48:13Z

impeller/scene/node.h

@@ -7,6 +7,7 @@

 #include <memory>
 #include <optional>
+#include <variant>


This include was being pulled in transitively via impeller::Path

jonahwilliams · 2024-09-07T20:48:35Z

impeller/geometry/path.h

@@ -138,14 +153,6 @@ class Path {

  bool IsEmpty() const;

-  template <class T>


This was only used in a single test so I removed it.

flutter-dashboard · 2024-09-09T16:29:27Z

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.

Changes reported for pull request #55028 at sha 8630224

jonahwilliams · 2024-09-09T16:32:23Z

Still some kinks to work out of this one according to the tests.

flutter-dashboard · 2024-09-09T23:54:13Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

flutter-dashboard · 2024-09-10T04:46:26Z

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

Changes reported for pull request #55028 at sha 85dbffc

flutter-dashboard · 2024-09-13T00:49:18Z

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

Changes reported for pull request #55028 at sha 8accf3e

auto-submit · 2024-09-23T19:39:21Z

auto label is removed for flutter/engine/55028, due to - The status or check suite Linux linux_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

…utter/engine#55028)

…155648) flutter/engine@559f2ff...746ce61 2024-09-25 skia-flutter-autoroll@skia.org Roll Dart SDK from 7af8c4882e07 to eda9ae15367e (2 revisions) (flutter/engine#55417) 2024-09-25 bdero@google.com [Flutter GPU] Add CullMode. (flutter/engine#55409) 2024-09-25 skia-flutter-autoroll@skia.org Roll Skia from 2e92f0b443f4 to 3541cdf2fae6 (1 revision) (flutter/engine#55412) 2024-09-24 jonahwilliams@google.com [Impeller] fix Impeller on windows. (flutter/engine#55323) 2024-09-24 jonahwilliams@google.com [Impeller] add basic culling checks during text frame dispatcher. (flutter/engine#55168) 2024-09-24 matanlurey@users.noreply.github.com Move each dart GN rule to a standalone file. (flutter/engine#55404) 2024-09-24 skia-flutter-autoroll@skia.org Roll Skia from 118914b760cb to 2e92f0b443f4 (1 revision) (flutter/engine#55406) 2024-09-24 jonahwilliams@google.com [Impeller] Pack impeller:Path into 2 vecs instead of 3. (flutter/engine#55028) 2024-09-24 codefu@google.com Disallow time traveling frame times (flutter/engine#55310) 2024-09-24 jason-simmons@users.noreply.github.com [Impeller] Delete command pools held by the CommandPoolRecyclerVK after decoding an image on the IO thread (flutter/engine#55398) 2024-09-24 skia-flutter-autoroll@skia.org Roll Skia from cf28f9dd411d to 118914b760cb (1 revision) (flutter/engine#55405) 2024-09-24 bdero@google.com [Flutter GPU] Add pipeline stencil config. (flutter/engine#55272) 2024-09-24 skia-flutter-autoroll@skia.org Roll Skia from 6e5ff9253147 to cf28f9dd411d (1 revision) (flutter/engine#55399) 2024-09-24 jonahwilliams@google.com [Impeller] delete expensive trace event. (flutter/engine#55400) 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 bdero@google.com,rmistry@google.com,zra@google.com 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

…lutter#155648) flutter/engine@559f2ff...746ce61 2024-09-25 skia-flutter-autoroll@skia.org Roll Dart SDK from 7af8c4882e07 to eda9ae15367e (2 revisions) (flutter/engine#55417) 2024-09-25 bdero@google.com [Flutter GPU] Add CullMode. (flutter/engine#55409) 2024-09-25 skia-flutter-autoroll@skia.org Roll Skia from 2e92f0b443f4 to 3541cdf2fae6 (1 revision) (flutter/engine#55412) 2024-09-24 jonahwilliams@google.com [Impeller] fix Impeller on windows. (flutter/engine#55323) 2024-09-24 jonahwilliams@google.com [Impeller] add basic culling checks during text frame dispatcher. (flutter/engine#55168) 2024-09-24 matanlurey@users.noreply.github.com Move each dart GN rule to a standalone file. (flutter/engine#55404) 2024-09-24 skia-flutter-autoroll@skia.org Roll Skia from 118914b760cb to 2e92f0b443f4 (1 revision) (flutter/engine#55406) 2024-09-24 jonahwilliams@google.com [Impeller] Pack impeller:Path into 2 vecs instead of 3. (flutter/engine#55028) 2024-09-24 codefu@google.com Disallow time traveling frame times (flutter/engine#55310) 2024-09-24 jason-simmons@users.noreply.github.com [Impeller] Delete command pools held by the CommandPoolRecyclerVK after decoding an image on the IO thread (flutter/engine#55398) 2024-09-24 skia-flutter-autoroll@skia.org Roll Skia from cf28f9dd411d to 118914b760cb (1 revision) (flutter/engine#55405) 2024-09-24 bdero@google.com [Flutter GPU] Add pipeline stencil config. (flutter/engine#55272) 2024-09-24 skia-flutter-autoroll@skia.org Roll Skia from 6e5ff9253147 to cf28f9dd411d (1 revision) (flutter/engine#55399) 2024-09-24 jonahwilliams@google.com [Impeller] delete expensive trace event. (flutter/engine#55400) 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 bdero@google.com,rmistry@google.com,zra@google.com 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

jonahwilliams added 2 commits September 7, 2024 09:24

[Impeller] use 2 vecs instead of 3.

462fa72

test cleanups.

846c2aa

jonahwilliams changed the title ~~[Impeller] use 2 vecs instead of 3.~~ [Impeller] Pack impeller:Path into 2 vecs instead of 3. Sep 7, 2024

github-actions bot added the e: impeller label Sep 7, 2024

jonahwilliams added 5 commits September 7, 2024 10:32

add missing include to scene.

9102785

Fix off by N lookup at end of Path::CreatePolyline

ca55dac

++

184f719

move to header

827523c

++

8630224

jonahwilliams commented Sep 7, 2024

View reviewed changes

jonahwilliams marked this pull request as ready for review September 9, 2024 16:24

flutter-dashboard bot added the will affect goldens label Sep 9, 2024

jonahwilliams marked this pull request as draft September 9, 2024 23:51

fix incorrect origin computation.

85dbffc

jonahwilliams marked this pull request as ready for review September 10, 2024 04:08

jonahwilliams marked this pull request as draft September 10, 2024 06:51

correct empty contour computation.

dba9b06

jonahwilliams marked this pull request as ready for review September 12, 2024 03:42

jonahwilliams added 2 commits September 11, 2024 21:43

remove print.

027d4d6

++

833edc5

jonahwilliams marked this pull request as draft September 12, 2024 06:18

++

6899f86

jonahwilliams marked this pull request as ready for review September 12, 2024 17:13

Merge branch 'main' of github.com:flutter/engine into align_path

8accf3e

++

7f4a2c7

chinmaygarde approved these changes Sep 23, 2024

View reviewed changes

Merge branch 'main' into align_path

50dd210

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2024

++

095fe34

jonahwilliams added autosubmit Merge PR when tree becomes green via auto submit App and removed will affect goldens labels Sep 24, 2024

auto-submit bot merged commit 0ce0d22 into flutter:main Sep 24, 2024
30 checks passed

jonahwilliams deleted the align_path branch September 24, 2024 23:05

engine-flutter-autoroll mentioned this pull request Sep 25, 2024

Roll Flutter Engine from 559f2ff31c74 to 746ce6124844 (14 revisions) flutter/flutter#155648

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 25, 2024

0ce0d2279 [Impeller] Pack impeller:Path into 2 vecs instead of 3. (fl…

7d70855

…utter/engine#55028)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Impeller] Pack impeller:Path into 2 vecs instead of 3. #55028

[Impeller] Pack impeller:Path into 2 vecs instead of 3. #55028

jonahwilliams commented Sep 7, 2024 •

edited

Loading

jonahwilliams Sep 7, 2024

jonahwilliams Sep 7, 2024

flutter-dashboard bot commented Sep 9, 2024

jonahwilliams commented Sep 9, 2024

flutter-dashboard bot commented Sep 9, 2024

flutter-dashboard bot commented Sep 10, 2024

flutter-dashboard bot commented Sep 13, 2024

auto-submit bot commented Sep 23, 2024

		@@ -138,14 +153,6 @@ class Path {

		bool IsEmpty() const;

		template <class T>

[Impeller] Pack impeller:Path into 2 vecs instead of 3. #55028

[Impeller] Pack impeller:Path into 2 vecs instead of 3. #55028

Conversation

jonahwilliams commented Sep 7, 2024 • edited Loading

jonahwilliams Sep 7, 2024

Choose a reason for hiding this comment

jonahwilliams Sep 7, 2024

Choose a reason for hiding this comment

flutter-dashboard bot commented Sep 9, 2024

jonahwilliams commented Sep 9, 2024

flutter-dashboard bot commented Sep 9, 2024

flutter-dashboard bot commented Sep 10, 2024

flutter-dashboard bot commented Sep 13, 2024

auto-submit bot commented Sep 23, 2024

jonahwilliams commented Sep 7, 2024 •

edited

Loading