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

Conversation

bdero
Copy link
Member

@bdero bdero commented Aug 24, 2023

Resolves flutter/flutter#132936.

  • In some cases, mask blurs were getting applied twice.
  • Coverage hint clipping was too aggressive on the second pass of the gaussian blur.
  • Rework the mask blur applicator to support gracefully falling back to GPU-based color filters when necessary.

This fix requires us to cherry-pick: #43519
Going a revert-based route to fix all of these problems would also be pretty complex.

I've been testing this against 3.13 and still need to cross reference the other issues. I haven't rigorously tested this against HEAD yet.

Untitled.mov

@bdero bdero self-assigned this Aug 24, 2023
@bdero bdero force-pushed the bdero/3.13-blur-fixes branch 4 times, most recently from f0e533d to 3a02a99 Compare August 24, 2023 22:14
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM


entity.SetContents(
paint.WithFilters(std::move(color_text_contents), false));
// TODO(bdero): This mask blur application is a hack. It will always wind up
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have bugs tracking these?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, thanks for the reminder. Filed flutter/flutter#133297.

@bdero bdero force-pushed the bdero/3.13-blur-fixes branch 2 times, most recently from dd6f749 to 818e0f3 Compare August 25, 2023 00:08
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #45079 at sha 818e0f3

@bdero
Copy link
Member Author

bdero commented Aug 25, 2023

Where are the dang goldens?

@jonahwilliams
Copy link
Contributor

Yeah this isn't working for me either in #45090

@bdero bdero force-pushed the bdero/3.13-blur-fixes branch from 818e0f3 to 367310c Compare August 25, 2023 19:09
@bdero
Copy link
Member Author

bdero commented Aug 25, 2023

Trying to rebase to see if the goldens will pop out this time.

@bdero
Copy link
Member Author

bdero commented Aug 25, 2023

I went through the goldens manually and things are looking mostly good.

CanRenderForegroundAdvancedBlendWithMaskBlur is rendering wrong -- but I think that's a negative test I landed to track progress and am double-checking that this patch doesn't introduce a regression.

@bdero
Copy link
Member Author

bdero commented Aug 25, 2023

Yeah we're not handling that case correctly before/after. This patch doesn't introduce a regression. Filed a bug to track it -- I thought I had already fixed this problem: flutter/flutter#133354

@bdero bdero merged commit 53595c9 into flutter:main Aug 25, 2023
bdero added a commit to bdero/flutter-engine that referenced this pull request Aug 25, 2023
…er#45079)

Resolves flutter/flutter#132936.

* In some cases, mask blurs were getting applied twice.
* Coverage hint clipping was too aggressive on the second pass of the
gaussian blur.
* Rework the mask blur applicator to support gracefully falling back to
GPU-based color filters when necessary.

This fix requires us to cherry-pick:
flutter#43519
Going a revert-based route to fix all of these problems would also be
pretty complex.

https://github.com/flutter/engine/assets/919017/9ee5e856-af7a-42b9-b135-ea268c2ba53f
(cherry picked from commit 53595c9)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 25, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 25, 2023
…133362)

flutter/engine@1ec7b89...53595c9

2023-08-25 [email protected] [Impeller] Fix mask blurs and the Gaussian blur coverage hint. (flutter/engine#45079)
2023-08-25 [email protected] ios: remove shared_application and support app extension build (flutter/engine#44732)
2023-08-25 [email protected] Roll Skia from 76672468e8d7 to 40f4e01fca40 (3 revisions) (flutter/engine#45126)
2023-08-25 [email protected] Roll Fuchsia Mac SDK from 4LHUJjNlDT21v_pdT... to Qj4BGsKtF7EJssKIK... (flutter/engine#45127)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from 4LHUJjNlDT21 to Qj4BGsKtF7EJ

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
bdero added a commit to bdero/flutter-engine that referenced this pull request Aug 28, 2023
…er#45079)

Resolves flutter/flutter#132936.

* In some cases, mask blurs were getting applied twice.
* Coverage hint clipping was too aggressive on the second pass of the
gaussian blur.
* Rework the mask blur applicator to support gracefully falling back to
GPU-based color filters when necessary.

This fix requires us to cherry-pick:
flutter#43519
Going a revert-based route to fix all of these problems would also be
pretty complex.

https://github.com/flutter/engine/assets/919017/9ee5e856-af7a-42b9-b135-ea268c2ba53f
(cherry picked from commit 53595c9)
auto-submit bot pushed a commit that referenced this pull request Aug 29, 2023
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…er#45079)

Resolves flutter/flutter#132936.

* In some cases, mask blurs were getting applied twice.
* Coverage hint clipping was too aggressive on the second pass of the
gaussian blur.
* Rework the mask blur applicator to support gracefully falling back to
GPU-based color filters when necessary.

This fix requires us to cherry-pick:
flutter#43519
Going a revert-based route to fix all of these problems would also be
pretty complex.

https://github.com/flutter/engine/assets/919017/9ee5e856-af7a-42b9-b135-ea268c2ba53f
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

[Impeller] Flutter 3.13.0 regression of box shadow from container with partial border radius drawn over scrollable

2 participants