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 Apr 19, 2023

Resolves flutter/flutter#124956.

Fixes some recent transform issues that mainly apply to chained filters (like when both a color + image filter has been set).

  • For all filters, the returned entity should just be an identity matrix because the snapshot transform gets fully absorbed.
  • For all filters, given entity's transform should be applied in the coverage proc (although this currently makes no difference because of the way coverage is queried when snapshotting).
  • For filters drawing with coverage + transforming UVs, the transform handed to the contents of the deferred entity needs to be applied. Otherwise the entity won't get repositioned correctly while snapshotting.

New golden: Play/AiksTest.TranslucentSaveLayerWithColorFilterAndImageFilterDrawsCorrectly/Metal

autoninja -C ../out/host_debug_unopt/ -j 1000 flutter/impeller:impeller_unittests && ../out/host_debug_unopt/impeller_unittests --timeout=0 --gtest_filter="Play/AiksTest.TranslucentSaveLayerWithColorFilterAndImageFilterDrawsCorrectly/Metal" --enable_playground

Before:
Screenshot 2023-04-19 at 12 03 57 AM
Screenshot 2023-04-19 at 3 02 06 AM

After:
Screenshot 2023-04-19 at 12 00 34 AM
Screenshot 2023-04-19 at 2 59 56 AM

@bdero bdero self-assigned this Apr 19, 2023
@flutter-dashboard

This comment was marked as outdated.

@bdero bdero force-pushed the bdero/porterduff-transform branch from c249398 to 5cdd279 Compare April 19, 2023 08:52
@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 #41332 at sha 5cdd279

@bdero bdero force-pushed the bdero/porterduff-transform branch 2 times, most recently from 5f7f0be to 84e7e97 Compare April 19, 2023 09:41
@bdero bdero requested a review from jonahwilliams April 19, 2023 10:04
@bdero bdero removed the needs tests label Apr 19, 2023
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 😅

@bdero bdero force-pushed the bdero/porterduff-transform branch from 0c86905 to 0754f38 Compare April 19, 2023 17:50
@flutter-dashboard
Copy link

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #41332 at sha 0754f38

@bdero bdero added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 19, 2023
@bdero bdero merged commit 2cc615f into flutter:main Apr 19, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 19, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 19, 2023
flutter/engine@0a53cf6...d726345

2023-04-19 [email protected] [Impeller] Comprehensively label snapshots (flutter/engine#41325)
2023-04-19 [email protected] [Impeller] Fix transform regressions for chained filters (flutter/engine#41332)

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
Copy link
Member Author

bdero commented Apr 19, 2023

The regressing changes (#41098 and #40973) landed after the April 6th branch cut, so this doesn't need a 3.10 cherry-pick:

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 e: impeller will affect goldens

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

Inner Shadows from well known packages not rendering at all with Impeller

3 participants