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

[Impeller] relax conditions for SkRRect.isSimple conversion to impeller::RRect. #53083

Merged
merged 4 commits into from
May 29, 2024

Conversation

jonahwilliams
Copy link
Member

The flickering in flutter/flutter#148412 is caused by us switching between the RRect fast path and a gaussian blur. The reason is that the SkRect.isSimple check doesn't handle fp precision very well. On one of the frames the difference was :

D/skia (18362): SkRect::MakeLTRB(74, 179.666672f, 374, 479.666656f);
D/skia (18362): const SkPoint corners[] = {
D/skia (18362): { 150, 149.999969f },
D/skia (18362): { 150, 150 },
D/skia (18362): { 150, 149.999969f },
D/skia (18362): { 150, 150 },
D/skia (18362): };

So lets used a relaxed check for RRect.isSimple instead.

Fixes flutter/flutter#148412

@jonahwilliams jonahwilliams changed the title Fix is simple [Impeller] relax conditions for SkRRect.isSimple conversion to impeller::RRect. May 29, 2024
Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

lgtm!

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label May 29, 2024
@auto-submit auto-submit bot merged commit a72a9d8 into flutter:main May 29, 2024
29 checks passed
@jonahwilliams jonahwilliams deleted the fix_is_simple branch May 29, 2024 16:52
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 30, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 30, 2024
…149290)

flutter/engine@8d5d14a...4adf453

2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from aecf76e8eb9a to f23183f041fe (1 revision) (flutter/engine#53102)
2024-05-29 john@mcdole.org Update CONTRIBUTING.md links (flutter/engine#53043)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from 32746679f7be to aecf76e8eb9a (1 revision) (flutter/engine#53100)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from b18b7d6af6e8 to 32746679f7be (2 revisions) (flutter/engine#53097)
2024-05-29 jonahwilliams@google.com [Impeller] revert changes to IO thread upload scheduling. (flutter/engine#53096)
2024-05-29 jonahwilliams@google.com [Impeller] fix barriers prior to mip generation. (flutter/engine#53082)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from e11a5c5b402e to b18b7d6af6e8 (2 revisions) (flutter/engine#53095)
2024-05-29 goderbauer@google.com Switch to FilterQuality.medium for images (flutter/engine#52984)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from a4f58c9dabc3 to e11a5c5b402e (1 revision) (flutter/engine#53092)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from ca77972f659e to a4f58c9dabc3 (2 revisions) (flutter/engine#53091)
2024-05-29 jonahwilliams@google.com [Impeller] relax conditions for SkRRect.isSimple conversion to impeller::RRect. (flutter/engine#53083)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from d1a6967c48a5 to ca77972f659e (1 revision) (flutter/engine#53090)

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 jacksongardner@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
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request May 31, 2024
…lutter#149290)

flutter/engine@8d5d14a...4adf453

2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from aecf76e8eb9a to f23183f041fe (1 revision) (flutter/engine#53102)
2024-05-29 john@mcdole.org Update CONTRIBUTING.md links (flutter/engine#53043)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from 32746679f7be to aecf76e8eb9a (1 revision) (flutter/engine#53100)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from b18b7d6af6e8 to 32746679f7be (2 revisions) (flutter/engine#53097)
2024-05-29 jonahwilliams@google.com [Impeller] revert changes to IO thread upload scheduling. (flutter/engine#53096)
2024-05-29 jonahwilliams@google.com [Impeller] fix barriers prior to mip generation. (flutter/engine#53082)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from e11a5c5b402e to b18b7d6af6e8 (2 revisions) (flutter/engine#53095)
2024-05-29 goderbauer@google.com Switch to FilterQuality.medium for images (flutter/engine#52984)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from a4f58c9dabc3 to e11a5c5b402e (1 revision) (flutter/engine#53092)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from ca77972f659e to a4f58c9dabc3 (2 revisions) (flutter/engine#53091)
2024-05-29 jonahwilliams@google.com [Impeller] relax conditions for SkRRect.isSimple conversion to impeller::RRect. (flutter/engine#53083)
2024-05-29 skia-flutter-autoroll@skia.org Roll Skia from d1a6967c48a5 to ca77972f659e (1 revision) (flutter/engine#53090)

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 jacksongardner@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
@jonahwilliams jonahwilliams added the cp: stable cherry pick to the stable release candidate branch label Jun 5, 2024
@flutteractionsbot
Copy link

Failed to create CP due to merge conflicts.
You will need to create the PR manually. See the cherrypick wiki for more info.

jonahwilliams added a commit to jonahwilliams/engine that referenced this pull request Jun 5, 2024
…er::RRect. (flutter#53083)

The flickering in flutter/flutter#148412 is caused by us switching between the RRect fast path and a gaussian blur. The reason is that the SkRect.isSimple check doesn't handle fp precision very well. On one of the frames the difference was :

```
D/skia (18362): SkRect::MakeLTRB(74, 179.666672f, 374, 479.666656f);
D/skia (18362): const SkPoint corners[] = {
D/skia (18362): { 150, 149.999969f },
D/skia (18362): { 150, 150 },
D/skia (18362): { 150, 149.999969f },
D/skia (18362): { 150, 150 },
D/skia (18362): };
```

So lets used a relaxed check for RRect.isSimple instead.

Fixes flutter/flutter#148412
auto-submit bot pushed a commit that referenced this pull request Jul 1, 2024
…pell… (#53208)

�er::RRect. (#53083)

The flickering in flutter/flutter#148412 is caused by us switching between the RRect fast path and a gaussian blur. The reason is that the SkRect.isSimple check doesn't handle fp precision very well. On one of the frames the difference was :

```
D/skia (18362): SkRect::MakeLTRB(74, 179.666672f, 374, 479.666656f);
D/skia (18362): const SkPoint corners[] = {
D/skia (18362): { 150, 149.999969f },
D/skia (18362): { 150, 150 },
D/skia (18362): { 150, 149.999969f },
D/skia (18362): { 150, 150 },
D/skia (18362): };
```

So lets used a relaxed check for RRect.isSimple instead.

Fixes flutter/flutter#148412
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 cp: stable cherry pick to the stable release candidate branch e: impeller
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] BoxShadow flickers when scrolling
4 participants