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

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Jul 18, 2023

Fixes flutter/flutter#130775

On the Skia backend, antiAliasWithSaveLayer is the highest fidelity clipping option. In the Impeller backend, there isn't any difference in how we clip, since the stencil buffer is always used. Nevertheless we were still inserting the save layer, which results in an extra offscreen texture and is wasteful.

Track if impeller is enabled in the diff/preroll/paint context and avoid inserting a save layer.

@jonahwilliams jonahwilliams changed the title Anti save layer [Impeller] Avoid inserting additional save layers based on clip configuration. Jul 18, 2023
@jonahwilliams jonahwilliams marked this pull request as ready for review July 18, 2023 13:56
@jonahwilliams jonahwilliams requested a review from dnfield July 18, 2023 13:56
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM.

nit: consider having some kind of enum rather than bools. https://google.github.io/styleguide/cppguide.html#Function_Argument_Comments

@dnfield
Copy link
Contributor

dnfield commented Jul 18, 2023

Alternatively, rather than naming the enum/argument about impeller, just have it be something like ClipBehavior::kNeverSaveLayer or something.

@jonahwilliams
Copy link
Contributor Author

I think we should keep it impelled named to facilitate an easier cleanup when we eventually delete all of this code.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 18, 2023
@auto-submit auto-submit bot merged commit 0ec9cc8 into flutter:main Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 18, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 18, 2023
…130814)

flutter/engine@831da7e...45851af

2023-07-18 [email protected] Reset Dart VM optimization level to 2 (flutter/engine#43770)
2023-07-18 [email protected] [Impeller] Avoid inserting additional save layers based on clip configuration. (flutter/engine#43759)

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],[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
harryterkelsen pushed a commit to harryterkelsen/engine that referenced this pull request Jul 20, 2023
…guration. (flutter#43759)

Fixes flutter/flutter#130775

On the Skia backend, antiAliasWithSaveLayer is the highest fidelity clipping option. In the Impeller backend, there isn't any difference in how we clip, since the stencil buffer is always used. Nevertheless we were still inserting the save layer, which results in an extra offscreen texture and is wasteful.

Track if impeller is enabled in the diff/preroll/paint context and avoid inserting a save layer.
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
…lutter#130814)

flutter/engine@831da7e...45851af

2023-07-18 [email protected] Reset Dart VM optimization level to 2 (flutter/engine#43770)
2023-07-18 [email protected] [Impeller] Avoid inserting additional save layers based on clip configuration. (flutter/engine#43759)

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],[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
@flar
Copy link
Contributor

flar commented Sep 14, 2023

This leads to a rendering problem - the saveLayer does more than just consolidate the AA edges: flutter/flutter#134705

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Impeller] ClipBehavior.antiAliasWithSaveLayer should probably not create a save layer.

3 participants