Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] started std::moving Commands instead of copying #44508

Merged
merged 2 commits into from
Aug 8, 2023

Conversation

gaaclarke
Copy link
Member

Each Command is 440 B so copying them when adding them to the RenderPass adds up. This change represents a 36% speedup in TextContents::Render when scrolling around on the main screen of the of the Flutter Gallery.

issue: flutter/flutter#131787

before

Screenshot 2023-08-08 at 1 11 30 PM

after

Screenshot 2023-08-08 at 1 10 05 PM

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Copy link
Member

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM!

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 8, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 8, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Aug 8, 2023

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

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 8, 2023
@auto-submit auto-submit bot merged commit 00b6b63 into flutter:main Aug 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 9, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 9, 2023
…132179)

flutter/engine@82292b8...29b117a

2023-08-08 matanlurey@users.noreply.github.com [Impeller] Add gradient dithering for Radial/Sweep/Conical gradients (flutter/engine#44331)
2023-08-08 skia-flutter-autoroll@skia.org Roll Skia from 68b80f663be6 to 1fbe521b2c56 (2 revisions) (flutter/engine#44518)
2023-08-08 chris@bracken.jp [macOS] Improve engine retain cycle testing (flutter/engine#44509)
2023-08-08 34871572+gmackall@users.noreply.github.com Fix name in description of 'create_cipd_packages.sh' (flutter/engine#44513)
2023-08-08 30870216+gaaclarke@users.noreply.github.com [Impeller] started std::moving `Command`s instead of copying (flutter/engine#44508)

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 chinmaygarde@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://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
@reidbaker reidbaker mentioned this pull request Aug 15, 2023
14 tasks
gaaclarke added a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…#44508)

Each Command is 440 B so copying them when adding them to the RenderPass adds up.  This change represents a 36% speedup in `TextContents::Render` when scrolling around on the main screen of the of the Flutter Gallery.

issue: flutter/flutter#131787

## before
<img width="1217" alt="Screenshot 2023-08-08 at 1 11 30 PM" src="https://github.com/flutter/engine/assets/30870216/3545f9cd-b596-4a7c-bcf0-a20c7f7c40e8">

## after

<img width="1048" alt="Screenshot 2023-08-08 at 1 10 05 PM" src="https://github.com/flutter/engine/assets/30870216/3443ec70-5d4f-4022-b3e7-41b13c25cb00">

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants