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.

Sign up for GitHub

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

Jump to bottom

Revert "Reland "add non-rendering operation culling to DisplayListBuilder" (#41463)" #43721

Merged
merged 1 commit into from
Jul 15, 2023
Merged

Revert "Reland "add non-rendering operation culling to DisplayListBuilder" (#41463)" #43721

merged 1 commit into from
Jul 15, 2023

Conversation

zanderso
Copy link
Member

Reverts #43698

A framework tree test started failing on the engine roll with this PR: flutter/flutter#130643

The test failure is in https://ci.chromium.org/ui/p/flutter/builders/prod/Linux_android%20hybrid_android_views_integration_test/8517/overview

[2023-07-14 19:33:21.980926] [STDOUT] stdout: [   +6 ms] I/PlatformViewsController( 9988): Using hybrid composition for platform view: 5
[2023-07-14 19:33:22.767236] [STDOUT] stdout: [ +786 ms] 00:19 �[32m+4�[0m�[31m -1�[0m: Flutter surface with hybrid composition Uses FlutterImageView when Android view is on the screen �[1m�[31m[E]�[0m�[0m
[2023-07-14 19:33:22.767765] [STDOUT] stdout: [        ]   Expected: '|-FlutterView\n'
[2023-07-14 19:33:22.767815] [STDOUT] stdout: [        ]               '  |-FlutterSurfaceView\n'
[2023-07-14 19:33:22.767924] [STDOUT] stdout: [        ]               '  |-FlutterImageView\n'
[2023-07-14 19:33:22.768084] [STDOUT] stdout: [        ]               '  |-ViewGroup\n'
[2023-07-14 19:33:22.768162] [STDOUT] stdout: [        ]               '    |-ViewGroup\n'
[2023-07-14 19:33:22.768800] [STDOUT] stdout: [        ]               '  |-FlutterImageView\n'
[2023-07-14 19:33:22.768835] [STDOUT] stdout: [        ]               ''
[2023-07-14 19:33:22.768853] [STDOUT] stdout: [        ]     Actual: '|-FlutterView\n'
[2023-07-14 19:33:22.768882] [STDOUT] stdout: [        ]               '  |-FlutterSurfaceView\n'
[2023-07-14 19:33:22.768900] [STDOUT] stdout: [        ]               '  |-FlutterImageView\n'
[2023-07-14 19:33:22.768917] [STDOUT] stdout: [        ]               '  |-ViewGroup\n'
[2023-07-14 19:33:22.768956] [STDOUT] stdout: [        ]               '    |-ViewGroup\n'
[2023-07-14 19:33:22.769119] [STDOUT] stdout: [        ]               ''
[2023-07-14 19:33:22.769156] [STDOUT] stdout: [        ]      Which: is different. Both strings start the same, but the actual value is missing the following trailing characters:   |-Flutte ...
[2023-07-14 19:33:22.779280] [STDOUT] stdout: [  +10 ms]   package:matcher/src/expect/expect.dart 149:31     fail
[2023-07-14 19:33:22.779326] [STDOUT] stdout: [        ]   package:matcher/src/expect/expect.dart 144:3      _expect
[2023-07-14 19:33:22.780315] [STDOUT] stdout: [        ]   package:matcher/src/expect/expect.dart 56:3       expect
[2023-07-14 19:33:22.780345] [STDOUT] stdout: [        ]   test_driver/main_test.dart 124:7                  main.<fn>.<fn>
[2023-07-14 19:33:22.780356] [STDOUT] stdout: [        ]   ===== asynchronous gap ===========================
[2023-07-14 19:33:22.780365] [STDOUT] stdout: [        ]   package:test_api/src/backend/declarer.dart 215:9  Declarer.test.<fn>.<fn>
[2023-07-14 19:33:22.780376] [STDOUT] stdout: [        ]   ===== asynchronous gap ===========================
[2023-07-14 19:33:22.780385] [STDOUT] stdout: [        ]   package:test_api/src/backend/declarer.dart 213:7  Declarer.test.<fn>
[2023-07-14 19:33:22.780395] [STDOUT] stdout: [        ]   ===== asynchronous gap ===========================
[2023-07-14 19:33:22.780405] [STDOUT] stdout: [        ]   package:test_api/src/backend/invoker.dart 258:9   Invoker._waitForOutstandingCallbacks.<fn>
[2023-07-14 19:33:22.780415] [STDOUT] stdout: [        ] 00:19 �[32m+4�[0m�[31m -1�[0m: Flutter surface with hybrid composition (tearDownAll)�[0m
[2023-07-14 19:33:22.907295] [STDOUT] stdout: [ +126 ms] 00:19 �[32m+4�[0m�[31m -1�[0m: (tearDownAll)�[0m
[2023-07-14 19:33:22.947855] [STDOUT] stdout: [  +41 ms] 00:19 �[32m+4�[0m�[31m -1�[0m: �[31mSome tests failed.�[0m

This change in that roll looks like it may be related.

@zanderso zanderso requested review from bdero and flar July 15, 2023 13:54
@zanderso zanderso added the revert Label used to revert changes in a closed and merged pull request. label Jul 15, 2023
@auto-submit auto-submit bot merged commit 886a14e into main Jul 15, 2023
19 of 28 checks passed
@auto-submit auto-submit bot deleted the revert-43698-reland-40083-5 branch July 15, 2023 14:14
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 15, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 15, 2023
@engine-flutter-autoroll
886a14e36 Revert "Reland "add non-rendering operation culling to Disp…
5b69e2a 
…layListBuilder" (flutter#41463)" (flutter/engine#43721)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 15, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 15, 2023
@engine-flutter-autoroll
886a14e36 Revert "Reland "add non-rendering operation culling to Disp…
f535799 
…layListBuilder" (flutter#41463)" (flutter/engine#43721)
zanderso pushed a commit to flutter/flutter that referenced this pull request Jul 15, 2023
@engine-flutter-autoroll
Manual roll Flutter Engine from 403866d16137 to 683087731feb (16 revi…
4f437d4 
…sions) (#130666)

Manual roll requested by zra@google.com

flutter/engine@403866d...6830877

2023-07-15 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from
DEENqWMCYI1SMYsYH... to rmzZN2ZAgpbjAFi5_... (flutter/engine#43722)
2023-07-15 zanderso@users.noreply.github.com Revert "Reland "add
non-rendering operation culling to DisplayListBuilder" (#41463)"
(flutter/engine#43721)
2023-07-15 skia-flutter-autoroll@skia.org Roll Clang from 6d667d4b261e
to ebd0b8a0472b (flutter/engine#43673)
2023-07-15 skia-flutter-autoroll@skia.org Roll Dart SDK from
671bbdf6c542 to 0bd185c282d2 (1 revision) (flutter/engine#43719)
2023-07-15 26625149+0xZOne@users.noreply.github.com Optimizing
performance by avoiding multiple GC operations caused by multiple
surface destruction notifications (flutter/engine#43587)
2023-07-15 skia-flutter-autoroll@skia.org Roll Skia from 975eb1250431 to
6fb535aede4e (1 revision) (flutter/engine#43716)
2023-07-15 skia-flutter-autoroll@skia.org Roll Dart SDK from
d1fcadf22aad to 671bbdf6c542 (2 revisions) (flutter/engine#43715)
2023-07-15 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from
Z-1lzZAOYHvVrdjQ8... to oeYLDNShuD-FTgGwU... (flutter/engine#43714)
2023-07-15 skia-flutter-autoroll@skia.org Roll Skia from 271b2b6d5aaa to
975eb1250431 (2 revisions) (flutter/engine#43712)
2023-07-15 goderbauer@google.com Move ViewConfiguration ownership to
FlutterView (flutter/engine#43701)
2023-07-14 yjbanov@google.com [web] always add secondary role managers
(flutter/engine#43663)
2023-07-14 dnfield@google.com [Impeller] Fix text scaling issues again,
this time with perspective when a save layer is involved
(flutter/engine#43695)
2023-07-14 flar@google.com Reland "add non-rendering operation culling
to DisplayListBuilder" (#41463) (flutter/engine#43698)
2023-07-14 skia-flutter-autoroll@skia.org Roll Skia from 5f6578398870 to
271b2b6d5aaa (2 revisions) (flutter/engine#43706)
2023-07-14 jonahwilliams@google.com [Impeller] Ensure that missing color
attachment 0u does not cause crash in embedder API
(flutter/engine#43705)
2023-07-14 skia-flutter-autoroll@skia.org Roll Skia from 315c7f08c731 to
5f6578398870 (4 revisions) (flutter/engine#43704)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from DEENqWMCYI1S to rmzZN2ZAgpbj
  fuchsia/sdk/core/mac-amd64 from Z-1lzZAOYHvV to oeYLDNShuD-F

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 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
@flar flar mentioned this pull request Jul 17, 2023
Closed
@chinmaygarde chinmaygarde mentioned this pull request Jul 19, 2023
Closed
harryterkelsen pushed a commit to harryterkelsen/engine that referenced this pull request Jul 20, 2023
@zanderso @harryterkelsen
Revert "Reland "add non-rendering operation culling to DisplayListBui…
066dd6b 
…lder" (flutter#41463)" (flutter#43721)

Reverts flutter#43698

A framework tree test started failing on the engine roll with this PR: flutter/flutter#130643

The test failure is in https://ci.chromium.org/ui/p/flutter/builders/prod/Linux_android%20hybrid_android_views_integration_test/8517/overview

```
[2023-07-14 19:33:21.980926] [STDOUT] stdout: [   +6 ms] I/PlatformViewsController( 9988): Using hybrid composition for platform view: 5
[2023-07-14 19:33:22.767236] [STDOUT] stdout: [ +786 ms] 00:19 �[32m+4�[0m�[31m -1�[0m: Flutter surface with hybrid composition Uses FlutterImageView when Android view is on the screen �[1m�[31m[E]�[0m�[0m
[2023-07-14 19:33:22.767765] [STDOUT] stdout: [        ]   Expected: '|-FlutterView\n'
[2023-07-14 19:33:22.767815] [STDOUT] stdout: [        ]               '  |-FlutterSurfaceView\n'
[2023-07-14 19:33:22.767924] [STDOUT] stdout: [        ]               '  |-FlutterImageView\n'
[2023-07-14 19:33:22.768084] [STDOUT] stdout: [        ]               '  |-ViewGroup\n'
[2023-07-14 19:33:22.768162] [STDOUT] stdout: [        ]               '    |-ViewGroup\n'
[2023-07-14 19:33:22.768800] [STDOUT] stdout: [        ]               '  |-FlutterImageView\n'
[2023-07-14 19:33:22.768835] [STDOUT] stdout: [        ]               ''
[2023-07-14 19:33:22.768853] [STDOUT] stdout: [        ]     Actual: '|-FlutterView\n'
[2023-07-14 19:33:22.768882] [STDOUT] stdout: [        ]               '  |-FlutterSurfaceView\n'
[2023-07-14 19:33:22.768900] [STDOUT] stdout: [        ]               '  |-FlutterImageView\n'
[2023-07-14 19:33:22.768917] [STDOUT] stdout: [        ]               '  |-ViewGroup\n'
[2023-07-14 19:33:22.768956] [STDOUT] stdout: [        ]               '    |-ViewGroup\n'
[2023-07-14 19:33:22.769119] [STDOUT] stdout: [        ]               ''
[2023-07-14 19:33:22.769156] [STDOUT] stdout: [        ]      Which: is different. Both strings start the same, but the actual value is missing the following trailing characters:   |-Flutte ...
[2023-07-14 19:33:22.779280] [STDOUT] stdout: [  +10 ms]   package:matcher/src/expect/expect.dart 149:31     fail
[2023-07-14 19:33:22.779326] [STDOUT] stdout: [        ]   package:matcher/src/expect/expect.dart 144:3      _expect
[2023-07-14 19:33:22.780315] [STDOUT] stdout: [        ]   package:matcher/src/expect/expect.dart 56:3       expect
[2023-07-14 19:33:22.780345] [STDOUT] stdout: [        ]   test_driver/main_test.dart 124:7                  main.<fn>.<fn>
[2023-07-14 19:33:22.780356] [STDOUT] stdout: [        ]   ===== asynchronous gap ===========================
[2023-07-14 19:33:22.780365] [STDOUT] stdout: [        ]   package:test_api/src/backend/declarer.dart 215:9  Declarer.test.<fn>.<fn>
[2023-07-14 19:33:22.780376] [STDOUT] stdout: [        ]   ===== asynchronous gap ===========================
[2023-07-14 19:33:22.780385] [STDOUT] stdout: [        ]   package:test_api/src/backend/declarer.dart 213:7  Declarer.test.<fn>
[2023-07-14 19:33:22.780395] [STDOUT] stdout: [        ]   ===== asynchronous gap ===========================
[2023-07-14 19:33:22.780405] [STDOUT] stdout: [        ]   package:test_api/src/backend/invoker.dart 258:9   Invoker._waitForOutstandingCallbacks.<fn>
[2023-07-14 19:33:22.780415] [STDOUT] stdout: [        ] 00:19 �[32m+4�[0m�[31m -1�[0m: Flutter surface with hybrid composition (tearDownAll)�[0m
[2023-07-14 19:33:22.907295] [STDOUT] stdout: [ +126 ms] 00:19 �[32m+4�[0m�[31m -1�[0m: (tearDownAll)�[0m
[2023-07-14 19:33:22.947855] [STDOUT] stdout: [  +41 ms] 00:19 �[32m+4�[0m�[31m -1�[0m: �[31mSome tests failed.�[0m
```

This change in that roll looks like it may be related.
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
@engine-flutter-autoroll @LouiseHsu
Manual roll Flutter Engine from 403866d16137 to 683087731feb (16 revi…
522e358 
…sions) (flutter#130666)

Manual roll requested by zra@google.com

flutter/engine@403866d...6830877

2023-07-15 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from
DEENqWMCYI1SMYsYH... to rmzZN2ZAgpbjAFi5_... (flutter/engine#43722)
2023-07-15 zanderso@users.noreply.github.com Revert "Reland "add
non-rendering operation culling to DisplayListBuilder" (flutter#41463)"
(flutter/engine#43721)
2023-07-15 skia-flutter-autoroll@skia.org Roll Clang from 6d667d4b261e
to ebd0b8a0472b (flutter/engine#43673)
2023-07-15 skia-flutter-autoroll@skia.org Roll Dart SDK from
671bbdf6c542 to 0bd185c282d2 (1 revision) (flutter/engine#43719)
2023-07-15 26625149+0xZOne@users.noreply.github.com Optimizing
performance by avoiding multiple GC operations caused by multiple
surface destruction notifications (flutter/engine#43587)
2023-07-15 skia-flutter-autoroll@skia.org Roll Skia from 975eb1250431 to
6fb535aede4e (1 revision) (flutter/engine#43716)
2023-07-15 skia-flutter-autoroll@skia.org Roll Dart SDK from
d1fcadf22aad to 671bbdf6c542 (2 revisions) (flutter/engine#43715)
2023-07-15 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from
Z-1lzZAOYHvVrdjQ8... to oeYLDNShuD-FTgGwU... (flutter/engine#43714)
2023-07-15 skia-flutter-autoroll@skia.org Roll Skia from 271b2b6d5aaa to
975eb1250431 (2 revisions) (flutter/engine#43712)
2023-07-15 goderbauer@google.com Move ViewConfiguration ownership to
FlutterView (flutter/engine#43701)
2023-07-14 yjbanov@google.com [web] always add secondary role managers
(flutter/engine#43663)
2023-07-14 dnfield@google.com [Impeller] Fix text scaling issues again,
this time with perspective when a save layer is involved
(flutter/engine#43695)
2023-07-14 flar@google.com Reland "add non-rendering operation culling
to DisplayListBuilder" (flutter#41463) (flutter/engine#43698)
2023-07-14 skia-flutter-autoroll@skia.org Roll Skia from 5f6578398870 to
271b2b6d5aaa (2 revisions) (flutter/engine#43706)
2023-07-14 jonahwilliams@google.com [Impeller] Ensure that missing color
attachment 0u does not cause crash in embedder API
(flutter/engine#43705)
2023-07-14 skia-flutter-autoroll@skia.org Roll Skia from 315c7f08c731 to
5f6578398870 (4 revisions) (flutter/engine#43704)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from DEENqWMCYI1S to rmzZN2ZAgpbj
  fuchsia/sdk/core/mac-amd64 from Z-1lzZAOYHvV to oeYLDNShuD-F

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 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fluttergithubbot fluttergithubbot fluttergithubbot approved these changes

@bdero bdero Awaiting requested review from bdero

@flar flar Awaiting requested review from flar

Assignees
No one assigned
Labels
e: impeller revert Label used to revert changes in a closed and merged pull request.
Projects
No open projects
Impeller
Archived in project
Milestone
No milestone
2 participants
@zanderso @fluttergithubbot