[Impeller] Ensure that missing color attachment 0u does not cause crash in embedder API #43705

jonahwilliams · 2023-07-14T21:18:41Z

flutter-dashboard · 2023-07-14T21:18:43Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

jonahwilliams · 2023-07-14T21:19:25Z

impeller/entity/entity_pass.cc

  auto root_render_target = render_target;

-  if (root_render_target.GetColorAttachments().empty()) {
+  if (root_render_target.GetColorAttachments().find(0u) ==


The linux embedder API was sending a bad value through here, so the map wasn't empty. It doesn't matter that its empty though, it matters that 0u exists because that is what we use below.

It might be worth extracting a named constant for this to avoid the magic number.

chinmaygarde · 2023-07-14T21:38:34Z

shell/gpu/gpu_surface_gl_impeller.cc

      impeller_context_,                            // context
      swap_callback,                                // swap_callback
-      fbo_info.fbo_id,                              // fbo
+      0u,                                           // fbo


Did you mean to leave this in?

auto-submit · 2023-07-14T21:59:23Z

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

… cause crash in embedder API (flutter/engine#43705)

…130643) flutter/engine@403866d...bf6d4bf 2023-07-14 [email protected] [Impeller] Fix text scaling issues again, this time with perspective when a save layer is involved (flutter/engine#43695) 2023-07-14 [email protected] Reland "add non-rendering operation culling to DisplayListBuilder" (#41463) (flutter/engine#43698) 2023-07-14 [email protected] Roll Skia from 5f6578398870 to 271b2b6d5aaa (2 revisions) (flutter/engine#43706) 2023-07-14 [email protected] [Impeller] Ensure that missing color attachment 0u does not cause crash in embedder API (flutter/engine#43705) 2023-07-14 [email protected] Roll Skia from 315c7f08c731 to 5f6578398870 (4 revisions) (flutter/engine#43704) 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 [email protected],[email protected] 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

… cause crash in embedder API (flutter/engine#43705)

…sions) (#130666) Manual roll requested by [email protected] flutter/engine@403866d...6830877 2023-07-15 [email protected] Roll Fuchsia Linux SDK from DEENqWMCYI1SMYsYH... to rmzZN2ZAgpbjAFi5_... (flutter/engine#43722) 2023-07-15 [email protected] Revert "Reland "add non-rendering operation culling to DisplayListBuilder" (#41463)" (flutter/engine#43721) 2023-07-15 [email protected] Roll Clang from 6d667d4b261e to ebd0b8a0472b (flutter/engine#43673) 2023-07-15 [email protected] Roll Dart SDK from 671bbdf6c542 to 0bd185c282d2 (1 revision) (flutter/engine#43719) 2023-07-15 [email protected] Optimizing performance by avoiding multiple GC operations caused by multiple surface destruction notifications (flutter/engine#43587) 2023-07-15 [email protected] Roll Skia from 975eb1250431 to 6fb535aede4e (1 revision) (flutter/engine#43716) 2023-07-15 [email protected] Roll Dart SDK from d1fcadf22aad to 671bbdf6c542 (2 revisions) (flutter/engine#43715) 2023-07-15 [email protected] Roll Fuchsia Mac SDK from Z-1lzZAOYHvVrdjQ8... to oeYLDNShuD-FTgGwU... (flutter/engine#43714) 2023-07-15 [email protected] Roll Skia from 271b2b6d5aaa to 975eb1250431 (2 revisions) (flutter/engine#43712) 2023-07-15 [email protected] Move ViewConfiguration ownership to FlutterView (flutter/engine#43701) 2023-07-14 [email protected] [web] always add secondary role managers (flutter/engine#43663) 2023-07-14 [email protected] [Impeller] Fix text scaling issues again, this time with perspective when a save layer is involved (flutter/engine#43695) 2023-07-14 [email protected] Reland "add non-rendering operation culling to DisplayListBuilder" (#41463) (flutter/engine#43698) 2023-07-14 [email protected] Roll Skia from 5f6578398870 to 271b2b6d5aaa (2 revisions) (flutter/engine#43706) 2023-07-14 [email protected] [Impeller] Ensure that missing color attachment 0u does not cause crash in embedder API (flutter/engine#43705) 2023-07-14 [email protected] 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 [email protected],[email protected] 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

…sh in embedder API (flutter#43705) flutter/flutter#130619

…lutter#130643) flutter/engine@403866d...bf6d4bf 2023-07-14 [email protected] [Impeller] Fix text scaling issues again, this time with perspective when a save layer is involved (flutter/engine#43695) 2023-07-14 [email protected] Reland "add non-rendering operation culling to DisplayListBuilder" (flutter#41463) (flutter/engine#43698) 2023-07-14 [email protected] Roll Skia from 5f6578398870 to 271b2b6d5aaa (2 revisions) (flutter/engine#43706) 2023-07-14 [email protected] [Impeller] Ensure that missing color attachment 0u does not cause crash in embedder API (flutter/engine#43705) 2023-07-14 [email protected] Roll Skia from 315c7f08c731 to 5f6578398870 (4 revisions) (flutter/engine#43704) 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 [email protected],[email protected] 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

…sions) (flutter#130666) Manual roll requested by [email protected] flutter/engine@403866d...6830877 2023-07-15 [email protected] Roll Fuchsia Linux SDK from DEENqWMCYI1SMYsYH... to rmzZN2ZAgpbjAFi5_... (flutter/engine#43722) 2023-07-15 [email protected] Revert "Reland "add non-rendering operation culling to DisplayListBuilder" (flutter#41463)" (flutter/engine#43721) 2023-07-15 [email protected] Roll Clang from 6d667d4b261e to ebd0b8a0472b (flutter/engine#43673) 2023-07-15 [email protected] Roll Dart SDK from 671bbdf6c542 to 0bd185c282d2 (1 revision) (flutter/engine#43719) 2023-07-15 [email protected] Optimizing performance by avoiding multiple GC operations caused by multiple surface destruction notifications (flutter/engine#43587) 2023-07-15 [email protected] Roll Skia from 975eb1250431 to 6fb535aede4e (1 revision) (flutter/engine#43716) 2023-07-15 [email protected] Roll Dart SDK from d1fcadf22aad to 671bbdf6c542 (2 revisions) (flutter/engine#43715) 2023-07-15 [email protected] Roll Fuchsia Mac SDK from Z-1lzZAOYHvVrdjQ8... to oeYLDNShuD-FTgGwU... (flutter/engine#43714) 2023-07-15 [email protected] Roll Skia from 271b2b6d5aaa to 975eb1250431 (2 revisions) (flutter/engine#43712) 2023-07-15 [email protected] Move ViewConfiguration ownership to FlutterView (flutter/engine#43701) 2023-07-14 [email protected] [web] always add secondary role managers (flutter/engine#43663) 2023-07-14 [email protected] [Impeller] Fix text scaling issues again, this time with perspective when a save layer is involved (flutter/engine#43695) 2023-07-14 [email protected] Reland "add non-rendering operation culling to DisplayListBuilder" (flutter#41463) (flutter/engine#43698) 2023-07-14 [email protected] Roll Skia from 5f6578398870 to 271b2b6d5aaa (2 revisions) (flutter/engine#43706) 2023-07-14 [email protected] [Impeller] Ensure that missing color attachment 0u does not cause crash in embedder API (flutter/engine#43705) 2023-07-14 [email protected] 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 [email protected],[email protected] 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

[Impeller] various desktop OpenGL issues.

023ab80

github-actions bot added the e: impeller label Jul 14, 2023

jonahwilliams commented Jul 14, 2023

View reviewed changes

jonahwilliams requested review from cbracken and chinmaygarde July 14, 2023 21:19

chinmaygarde reviewed Jul 14, 2023

View reviewed changes

Update gpu_surface_gl_impeller.cc

cc8f91c

chinmaygarde approved these changes Jul 14, 2023

View reviewed changes

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 14, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 14, 2023

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 14, 2023

auto-submit bot merged commit 92e75cf into flutter:main Jul 14, 2023

engine-flutter-autoroll mentioned this pull request Jul 14, 2023

Roll Flutter Engine from 403866d16137 to bf6d4bfe27cc (5 revisions) flutter/flutter#130643

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 14, 2023

92e75cf96 [Impeller] Ensure that missing color attachment 0u does not…

579072a

… cause crash in embedder API (flutter/engine#43705)

engine-flutter-autoroll mentioned this pull request Jul 15, 2023

Manual roll Flutter Engine from 403866d16137 to 683087731feb (16 revisions) flutter/flutter#130666

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 15, 2023

92e75cf96 [Impeller] Ensure that missing color attachment 0u does not…

7393164

… cause crash in embedder API (flutter/engine#43705)

harryterkelsen pushed a commit to harryterkelsen/engine that referenced this pull request Jul 20, 2023

[Impeller] Ensure that missing color attachment 0u does not cause cra…

eb8f9d8

…sh in embedder API (flutter#43705) flutter/flutter#130619

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Impeller] Ensure that missing color attachment 0u does not cause crash in embedder API #43705

[Impeller] Ensure that missing color attachment 0u does not cause crash in embedder API #43705

Uh oh!

jonahwilliams commented Jul 14, 2023

Uh oh!

flutter-dashboard bot commented Jul 14, 2023

Uh oh!

jonahwilliams Jul 14, 2023

Uh oh!

cbracken Jul 18, 2023

Uh oh!

chinmaygarde Jul 14, 2023

Uh oh!

jonahwilliams Jul 14, 2023

Uh oh!

auto-submit bot commented Jul 14, 2023

Uh oh!

Uh oh!

[Impeller] Ensure that missing color attachment 0u does not cause crash in embedder API #43705

[Impeller] Ensure that missing color attachment 0u does not cause crash in embedder API #43705

Uh oh!

Conversation

jonahwilliams commented Jul 14, 2023

Uh oh!

flutter-dashboard bot commented Jul 14, 2023

Uh oh!

jonahwilliams Jul 14, 2023

Choose a reason for hiding this comment

Uh oh!

cbracken Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

chinmaygarde Jul 14, 2023

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Jul 14, 2023

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Jul 14, 2023

Uh oh!

Uh oh!