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

[Impeller] Add ColorBurn, fix SourceOver alpha, make default pipeline initialization robust #33289

Merged
merged 5 commits into from
May 16, 2022

Conversation

bdero
Copy link
Member

@bdero bdero commented May 12, 2022

In addition to adding ColorBurn and activating advanced blends in the display list dispatcher, this change fixes pipeline alpha blend/default initialization problems that were resulting in double-premultiplied alpha for the SourceOver (default) Entities blend mode. This issue is difficult to notice in isolation, but rapidly compounds with overlapping transparent objects.

  • Use SourceOver alpha compositing for advanced/color blends.
  • Correct alpha source factor on SourceOver.
  • For advanced blends, let the pipeline handle alpha compositing and set the pipeline blend mode to SourceOver. The advanced blend shaders just manipulate color channels and pass the source alpha onward for blending. This makes the alpha channel affect the resulting color in the correct way.
  • Add ColorBurn.
  • Activate Screen and ColorBurn in the display list dispatcher.
  • Share vertex shader between advanced blend pipelines.
  • When creating default pipelines, always decorate the descriptor with default ContentContextOptions options. The default pipeline blend options no longer matched kSourceOver (the default entities blend mode). Rather than continuing to sync the default pipeline descriptor in the render layer, it's more robust to just apply the same Entities-specific ContentContextOptions transform to the pipeline descriptor in all cases where a new pipeline is created.
Screen.Recording.2022-05-11.at.10.33.52.PM.mov

All blend modes now closely match this example: https://fiddle.skia.org/c/@BlendModes

Compare with the previous video I recorded of this test, where a dark outer ring can be seen around the color wheel due to the alpha compositing issues.

Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

License file needs to be updated.

"shaders/texture_blend.vert",
"shaders/texture_blend_screen.frag",
"shaders/texture_blend_screen.vert",
"shaders/blend.frag",
Copy link
Member

Choose a reason for hiding this comment

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

nit: please alphabetize this list.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@bdero bdero force-pushed the bdero/colorburn branch from b3efc9c to 5d6188d Compare May 13, 2022 18:44
@bdero
Copy link
Member Author

bdero commented May 13, 2022

Ended up changing the static ContentContext::ApplyOptionsToDescriptor(pipeline, options) to ContentContextOptions::ApplyToPipelineDescriptor(pipeline) const and moved definition out of the header.

@bdero bdero merged commit f75ea56 into flutter:main May 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 16, 2022
houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants