-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] Don't use unnecessary stencil attachments #39537
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -185,11 +185,13 @@ struct ContentContextOptions { | |
| CompareFunction stencil_compare = CompareFunction::kEqual; | ||
| StencilOperation stencil_operation = StencilOperation::kKeep; | ||
| PrimitiveType primitive_type = PrimitiveType::kTriangle; | ||
| bool has_stencil_attachment = true; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is fine (especially for non-TBDR archs), but we should be careful when adding more options to this struct as that open up another dimension along which PSO variants can be created (albeit with the same shaders). I'm somewhat uncomfortable we need to do this today already but we already have the prototypes and they are created concurrently. So it works out. Perhaps we should add a comment to this struct explaining the consequences of adding fields here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Added a docstring explaining that complicated Flutter apps could easily be expected to form hundreds of PSOs in total, but should never reach the order of e.g. 10s of thousands. BTW, we'll need another param to set the color attachment pixel format for wide gamut (I suspect this is the major mismatch issue that @gaaclarke has been running into). |
||
|
|
||
| struct Hash { | ||
| constexpr std::size_t operator()(const ContentContextOptions& o) const { | ||
| return fml::HashCombine(o.sample_count, o.blend_mode, o.stencil_compare, | ||
| o.stencil_operation, o.primitive_type); | ||
| o.stencil_operation, o.primitive_type, | ||
| o.has_stencil_attachment); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -200,7 +202,8 @@ struct ContentContextOptions { | |
| lhs.blend_mode == rhs.blend_mode && | ||
| lhs.stencil_compare == rhs.stencil_compare && | ||
| lhs.stencil_operation == rhs.stencil_operation && | ||
| lhs.primitive_type == rhs.primitive_type; | ||
| lhs.primitive_type == rhs.primitive_type && | ||
| lhs.has_stencil_attachment == rhs.has_stencil_attachment; | ||
| } | ||
| }; | ||
|
|
||
|
|
||
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.
We could have a
ClearStencilAttachments,ClearDepthAttachmentsandClearColorAttachment(size_t index)helpers on the descriptor.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.
Done.