-
Notifications
You must be signed in to change notification settings - Fork 6k
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] Add CPU implementations for all color filters #42692
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -1931,238 +1931,6 @@ TEST_P(EntityTest, SrgbToLinearFilter) { | |||
ASSERT_TRUE(OpenPlaygroundHere(callback)); | |||
} | |||
|
|||
TEST_P(EntityTest, TTTBlendColor) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's replacing these tests?
See also flutter/flutter#128606. It's possible some of these tests are encoding bad behavior, but it seems veyr likely to me that if they are not encoding bad behavior they're missing cases where there's bad behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I see below the new tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I just moved this existing test into GeometryTest.ColorBlend
. And yes, some of them are wrong -- we used to use these for absorbing the DrawPaints, which introduced bugs. Some blends are also missing from the test at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test also has too many cases and needs to be rewritten in general, or at least be annotated to state why each cases is useful and not redundant.
…128612) flutter/engine@071e1fb...488876e 2023-06-09 skia-flutter-autoroll@skia.org Roll ANGLE from a185cb8c8924 to 3e4f4caebcb0 (2 revisions) (flutter/engine#42701) 2023-06-09 bdero@google.com [Impeller] Add CPU implementations for all color filters (flutter/engine#42692) 2023-06-09 jonahwilliams@google.com [Impeller] add explicit VMA flush to device memory writes. (flutter/engine#42685) 2023-06-09 30870216+gaaclarke@users.noreply.github.com [Impeller] Makes validation layers flag work for android (flutter/engine#42625) 2023-06-09 jmccandless@google.com Platform channel for predictive back (flutter/engine#39208) 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://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
Part of flutter/flutter#127232.
Adds a CPU implementation for ColorMatrix and the color space conversions. Also changes the blend signature for consistency.
These will be necessary to make the mask blur fast path continue working in the presence of color filters. In general, we can use these to eliminate a needlessly expensive image-based color filter for the non-image color sources.