[Impeller] Correct advanced blending behavior. #43283
Conversation
|
The malioc diff is looking kinda happy about this. I guess removing the premultiplying ceremony was more impactful than the additional dst.a lerp. |
|
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. |
chinmaygarde
changed the title
|
This fix to the blend modes does exacerbate the problem that I'm ultimately working on fixing visible in this issue. Prior to this change, we're multiplying the destination alpha into the result for the advanced blend modes, which is wrong for computing the final color. |
|
We have a couple of goldens ( |
…129687) flutter/engine@25a5850...a6d9d12 2023-06-28 [email protected] Include the SkRTreeFactory headers in the skwasm picture recorder (flutter/engine#43292) 2023-06-28 [email protected] Roll Dart SDK from 7a233f843402 to abfa59f66c42 (3 revisions) (flutter/engine#43291) 2023-06-27 [email protected] [Impeller] Correct advanced blending behavior. (flutter/engine#43283) 2023-06-27 [email protected] Manual roll Dart SDK from 81cdbe69a16b to 7a233f843402 (4 revisions) (flutter/engine#43288) 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
Resolves flutter/flutter#128606. All CPU blends now match the behavior of Impeller's GPU blends, which (after #43283) closely matches Skia's blends! Now we can finally use these. https://github.com/flutter/engine/assets/919017/f9dc7323-fd14-4cdc-ba8b-930622be4206
Investigated as part of flutter/flutter#127232. This took some time to work out, but: * The amount of blending applied to the source color needs to be weighted by the destination alpha, which is pretty sensible behavior. This is in addition to applying source-over behavior with the resulting blended color. * All of the blend functions assume that the color is already premultiplied, so remove the unpremultiply/premultiply surrounding the blend call. All of the blend modes now visually match up with the [Flutter docs](https://api.flutter.dev/flutter/dart-ui/BlendMode.html), except for ColorBurn and Saturation, which appear to have a slight miscalculation going on with the red channel.
Resolves flutter/flutter#128606. All CPU blends now match the behavior of Impeller's GPU blends, which (after flutter#43283) closely matches Skia's blends! Now we can finally use these. https://github.com/flutter/engine/assets/919017/f9dc7323-fd14-4cdc-ba8b-930622be4206
2 participants
Investigated as part of flutter/flutter#127232.
This took some time to work out, but:
All of the blend modes now visually match up with the Flutter docs, except for ColorBurn and Saturation, which appear to have a slight miscalculation going on with the red channel. Going to investigate and fix in a follow-up.