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

Revert "Remove GN staging flag for save layer bounds" #42026

Merged
merged 1 commit into from
May 14, 2023

Conversation

CaseyHillers
Copy link
Contributor

Reverts #41940

See b/282115120

This caused a golden failure on Google Testing where a rectangular avatar became circular.

@CaseyHillers CaseyHillers added the autosubmit Merge PR when tree becomes green via auto submit App label May 14, 2023
@auto-submit auto-submit bot merged commit dd8d76d into main May 14, 2023
@auto-submit auto-submit bot deleted the revert-41940-patch-1 branch May 14, 2023 17:01
@flar
Copy link
Contributor

flar commented May 14, 2023

First, this change was expected to cause golden file failures. The new behavior is intentional, but not backwards compatible.

Second, it would seem to me that this kind of golden failure is exactly why this behavior has been bad. Those avatars are set inside a circular clip for a reason and this change fills that circular clip. The prior behavior failed to fill the circular clip because the ColorFilter did not fill past the implicit bounds of the children.

@flar
Copy link
Contributor

flar commented May 14, 2023

cc @lhkbob

@lhkbob
Copy link
Contributor

lhkbob commented May 14, 2023

@flar's assessment is correct. The golden failure is expected and the reference image should have been marked incorrect, while the new fully-filled circle is correct.

auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 14, 2023
…126783)

flutter/engine@a1ccf90...326de1d

2023-05-14 [email protected] Roll Dart SDK from 005648f027a3 to 7d6324d5488b (1 revision) (flutter/engine#42027)
2023-05-14 [email protected] Revert "Remove GN staging flag for save layer bounds" (flutter/engine#42026)

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
CaseyHillers added a commit that referenced this pull request May 15, 2023
@CaseyHillers
Copy link
Contributor Author

Reland available in #42029. Thanks for investigating!

auto-submit bot pushed a commit that referenced this pull request May 15, 2023
Reverts #42026

After investigating, this is a bug fix for the scubas.
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
…lutter#126783)

flutter/engine@a1ccf90...326de1d

2023-05-14 [email protected] Roll Dart SDK from 005648f027a3 to 7d6324d5488b (1 revision) (flutter/engine#42027)
2023-05-14 [email protected] Revert "Remove GN staging flag for save layer bounds" (flutter/engine#42026)

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
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.

4 participants