[Impeller] Coerce opaque ColorSourceContents to Source #41525
Conversation
| if (entity.GetBlendMode() == BlendMode::kSourceOver && | ||
| entity.GetContents()->IsOpaque()) { | ||
| entity.SetBlendMode(BlendMode::kSource); | ||
| } |
There was a problem hiding this comment.
Does this happen before or after we do the opacity peephole check?
There was a problem hiding this comment.
This happens before (when Aiks pushes an Entity into the pass).
Hmm, it looks like we weren't handling blend modes in the opacity peephole. I added some restrictions and made it so it's able to unfold things with the Source blend mode. There's a new utility where the Entity handles SetInheritedOpacity with the blend mode in mind. Working on test cases for this at the moment.
There was a problem hiding this comment.
Like if the pass has kLuminosity as its blend mode and the child entities have kHue, it'll incorrectly collapse the pass.
There was a problem hiding this comment.
Ah nvm about the pass. I think the entities are still not being checked right, though.
| Entity entity; | ||
| std::shared_ptr<SolidColorContents> contents; | ||
| picture.pass->IterateAllEntities([&e = entity, &contents](Entity& entity) { | ||
| if (ScalarNearlyEqual(entity.GetTransformation().GetScale().x, 1.618f)) { |
There was a problem hiding this comment.
I've been occasionally using this pattern as a sort of poor man's predicate test for inspecting what Aiks + EntityPass is doing, but I don't think it's a sustainable solution in the longterm. @chinmaygarde recall you writing a doc around this kind of thing many moons ago, but I can't seem to find it...
impeller/aiks/paint_pass_delegate.cc
Outdated
| @@ -72,7 +73,8 @@ bool OpacityPeepholePassDelegate::CanElide() { | |||
| // |EntityPassDelgate| | |||
| bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass( | |||
| EntityPass* entity_pass) { | |||
| if (paint_.color.alpha <= 0.0 || paint_.color.alpha >= 1.0 || | |||
| if (paint_.blend_mode != BlendMode::kSourceOver || | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Alrighty, removed this and threw in a comment.
|
Seems reasonable to me, though I suppose its inevitable we're going to have to be careful about the ordering of these various optimizations. It would be good to note for future that we can only re-order the entities after doing the opacity peephole as that can change the opacity and blend mode too |
|
My initial plan is to just have all EntityPass Elements treated as default/translucent and defer solving this problem. I think we'll want to come up with a longer term plan here for pass collapsing that involves some precomputation. Maybe wrangle prioritizing other easy collapsible situations if we find they're common enough, too. Like collapsing if the pass is SourceOver+Opaque and all of its children are SourceOver/Source. |
…125593) flutter/engine@cf97541...d4ca524 2023-04-27 [email protected] Roll Skia from 20a1c61c5597 to d315ab065af3 (5 revisions) (flutter/engine#41535) 2023-04-26 [email protected] Revert "[Impeller] Use a device buffer for SkBitmap allocation, use Linear texture on Metal backend." (flutter/engine#41533) 2023-04-26 [email protected] [codesign] Add pinned xcode version as property to mac android aot engine (flutter/engine#41518) 2023-04-26 [email protected] [Impeller] Coerce opaque ColorSourceContents to Source (flutter/engine#41525) 2023-04-26 [email protected] Roll Skia from 3fea88565a83 to 20a1c61c5597 (3 revisions) (flutter/engine#41530) 2023-04-26 [email protected] [Impeller] partial repaint for Impeller/iOS. (flutter/engine#40959) 2023-04-26 [email protected] Updated todo with github issue link (flutter/engine#41517) 2023-04-26 [email protected] Roll Clang from 20d06c833d83 to 5344d8e10bb7 (flutter/engine#41524) 2023-04-26 [email protected] [Impeller] Use a device buffer for SkBitmap allocation, use Linear texture on Metal backend. (flutter/engine#41374) 2023-04-26 [email protected] Manual clang roll to 5344d8e10bb7d8672d4bfae8adb010465470d51b (flutter/engine#41520) 2023-04-26 [email protected] Download and use the goma client from cipd (flutter/engine#41488) 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
This coercion happens when Entities are appended to a pass, which is the moment where we will need to perform depth sorting.