Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DisplayList] cull clip operations that can be trivially and safely ignored #53367

Merged
merged 2 commits into from
Jun 13, 2024

Conversation

flar
Copy link
Contributor

@flar flar commented Jun 13, 2024

This mechanism pulls some clip-reduction logic from Impeller Entities up into DisplayListBuilder to simplify the work that Impeller has to do and also amortize the work if the DisplayList is reused from frame to frame. Since the DisplayList does not have access to information such as surface sizes, there will still be cases that Impeller can catch that must be conservatively included by the recording process in the Builder, so this mechanism does not replace the same code in Impeller, it merely catches some cases earlier, while recording widget output, to avoid the same work on every frame down in Impeller.

In this first pass only the most conservative and straightforward cases are handled - a clip on a layer which has a previous clip that the new clip fully contains (and which was not already restored).

This limited approach is already enough to eliminate a couple of clip operations in the layout of many of the pages in the new_gallery benchmarks.

/// Since both geometries are defining half-open spaces, their
/// defining geometry needs to consider their boundaries to
/// be equivalent with respect to interior and exterior.
[[nodiscard]] constexpr bool ContainsInclusive(const TPoint<Type>& p) const {
Copy link
Member

Choose a reason for hiding this comment

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

Can you a few test cases to https://github.com/flutter/engine/blob/main/impeller/geometry/geometry_unittests.cc for this? Just so its easier to find compared to knowing the specific DL tests its used in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean

TEST(RectTest, RectContainsPoint) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

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.

LGTM with nit

@flar flar added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 13, 2024
@auto-submit auto-submit bot merged commit 337f2fd into flutter:main Jun 13, 2024
27 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 15, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 15, 2024
…150290)

flutter/engine@8167dff...9779c27

2024-06-15 jason-simmons@users.noreply.github.com Manual roll of Dart SDK from e90b0a53e058 to dca20ab646c5 (flutter/engine#53410)
2024-06-15 chinmaygarde@google.com Update "Vulnerability scanning" to add the actions: read permission. (flutter/engine#53409)
2024-06-14 skia-flutter-autoroll@skia.org Roll Skia from 136c7cc18d1e to 6f6b45e1faea (3 revisions) (flutter/engine#53408)
2024-06-14 30870216+gaaclarke@users.noreply.github.com [Impeller] moved blur to unrotated local space, started respecting `respect_ctm` flag (flutter/engine#53377)
2024-06-14 skia-flutter-autoroll@skia.org Roll Skia from 5560041fcfc0 to 136c7cc18d1e (4 revisions) (flutter/engine#53406)
2024-06-14 jason-simmons@users.noreply.github.com Roll web_ui dependencies to enable the next roll of the Dart SDK (flutter/engine#53399)
2024-06-14 jacksongardner@google.com Hack to prevent Safari from being backgrounded during unit tests. (flutter/engine#53402)
2024-06-14 skia-flutter-autoroll@skia.org Manual roll Dart SDK from cabe65966815 to e90b0a53e058 (1 revision) (flutter/engine#53404)
2024-06-14 skia-flutter-autoroll@skia.org Roll Skia from de1a50046289 to 5560041fcfc0 (1 revision) (flutter/engine#53393)
2024-06-14 skia-flutter-autoroll@skia.org Manual roll Dart SDK from 115a9a2b26cd to cabe65966815 (1 revision) (flutter/engine#53390)
2024-06-14 skia-flutter-autoroll@skia.org Roll Skia from 3bebb0602780 to de1a50046289 (3 revisions) (flutter/engine#53388)
2024-06-14 skia-flutter-autoroll@skia.org Roll Skia from d248a9f99ff5 to 3bebb0602780 (1 revision) (flutter/engine#53387)
2024-06-14 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from pGxbL7JoNb3yAYFw4... to BkYg1UB1jdbAZv3gA... (flutter/engine#53386)
2024-06-14 flar@google.com [Impeller] restore accidentally deleted Cap/Join benchmarks (flutter/engine#53385)
2024-06-13 matanlurey@users.noreply.github.com Add a `FlutterEngineRule` (JUnit `TestRule`) and use it in `FlutterRendererTest` (flutter/engine#53361)
2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from b21429b0ea78 to d248a9f99ff5 (2 revisions) (flutter/engine#53383)
2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 40bdee9eedd6 to b21429b0ea78 (3 revisions) (flutter/engine#53382)
2024-06-13 flar@google.com [DisplayList] cull clip operations that can be trivially and safely ignored (flutter/engine#53367)
2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from 0f47a9333edb to 40bdee9eedd6 (2 revisions) (flutter/engine#53381)
2024-06-13 skia-flutter-autoroll@skia.org Roll Dart SDK from aa2265e5a192 to 115a9a2b26cd (5 revisions) (flutter/engine#53380)
2024-06-13 skia-flutter-autoroll@skia.org Roll Skia from bf5a0e0dbf41 to 0f47a9333edb (2 revisions) (flutter/engine#53378)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from pGxbL7JoNb3y to BkYg1UB1jdbA

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://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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
None yet
2 participants