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

[Impeller] Don't use unnecessary stencil attachments #39537

Merged
merged 2 commits into from
Feb 10, 2023
Merged

[Impeller] Don't use unnecessary stencil attachments #39537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3821085
[Impeller] Don't use unnecessary stencil attachments
bdero Feb 10, 2023
9ee1ec2
Address comments
bdero Feb 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions impeller/entity/contents/content_context.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ void ContentContextOptions::ApplyToPipelineDescriptor(
}
desc.SetColorAttachmentDescriptor(0u, color0);

if (!has_stencil_attachment) {
desc.ClearStencilAttachments();
}

if (desc.GetFrontStencilAttachmentDescriptor().has_value()) {
StencilAttachmentDescriptor stencil =
desc.GetFrontStencilAttachmentDescriptor().value();
Expand Down
17 changes: 15 additions & 2 deletions impeller/entity/contents/content_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,17 +179,29 @@ using GeometryColorPipeline =
using YUVToRGBFilterPipeline =
RenderPipelineT<YuvToRgbFilterVertexShader, YuvToRgbFilterFragmentShader>;

/// Pipeline state configuration.
///
/// Each unique combination of these options requires a different pipeline state
/// object to be built. This struct is used as a key for the per-pipeline
/// variant cache.
///
/// When adding fields to this key, reliant features should take care to limit
/// the combinatorical explosion of variations. A sufficiently complicated
/// Flutter application may easily require building hundreds of PSOs in total,
/// but they shouldn't require e.g. 10s of thousands.
struct ContentContextOptions {
SampleCount sample_count = SampleCount::kCount1;
BlendMode blend_mode = BlendMode::kSourceOver;
CompareFunction stencil_compare = CompareFunction::kEqual;
StencilOperation stencil_operation = StencilOperation::kKeep;
PrimitiveType primitive_type = PrimitiveType::kTriangle;
bool has_stencil_attachment = true;
Copy link
Member

Choose a reason for hiding this comment

The 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?

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. 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);
}
};

Expand All @@ -200,7 +212,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;
}
};

Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/contents/contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ namespace impeller {
ContentContextOptions OptionsFromPass(const RenderPass& pass) {
ContentContextOptions opts;
opts.sample_count = pass.GetRenderTarget().GetSampleCount();
opts.has_stencil_attachment =
pass.GetRenderTarget().GetStencilAttachment().has_value();
return opts;
}

ContentContextOptions OptionsFromPassAndEntity(const RenderPass& pass,
const Entity& entity) {
ContentContextOptions opts;
opts.sample_count = pass.GetRenderTarget().GetSampleCount();
opts.has_stencil_attachment =
pass.GetRenderTarget().GetStencilAttachment().has_value();
opts.blend_mode = entity.GetBlendMode();
return opts;
}
Expand Down
24 changes: 14 additions & 10 deletions impeller/entity/contents/scene_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,20 @@ bool SceneContents::Render(const ContentContext& renderer,
}

RenderTarget subpass_target = RenderTarget::CreateOffscreenMSAA(
*renderer.GetContext(), // context
ISize(coverage.value().size), // size
"SceneContents", // label
StorageMode::kDeviceTransient, // color_storage_mode
StorageMode::kDevicePrivate, // color_resolve_storage_mode
LoadAction::kClear, // color_load_action
StoreAction::kMultisampleResolve, // color_store_action
StorageMode::kDeviceTransient, // stencil_storage_mode
LoadAction::kDontCare, // stencil_load_action
StoreAction::kDontCare // stencil_store_action
*renderer.GetContext(), // context
ISize(coverage.value().size), // size
"SceneContents", // label
RenderTarget::AttachmentConfigMSAA{
.storage_mode = StorageMode::kDeviceTransient,
.resolve_storage_mode = StorageMode::kDevicePrivate,
.load_action = LoadAction::kClear,
.store_action = StoreAction::kMultisampleResolve,
}, // color_attachment_config
RenderTarget::AttachmentConfig{
.storage_mode = StorageMode::kDeviceTransient,
.load_action = LoadAction::kDontCare,
.store_action = StoreAction::kDontCare,
} // stencil_attachment_config
);
if (!subpass_target.IsValid()) {
return false;
Expand Down
50 changes: 29 additions & 21 deletions impeller/entity/entity_pass.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,31 +155,39 @@ static RenderTarget CreateRenderTarget(ContentContext& renderer,

if (context->SupportsOffscreenMSAA()) {
return RenderTarget::CreateOffscreenMSAA(
*context, // context
size, // size
"EntityPass", // label
StorageMode::kDeviceTransient, // color_storage_mode
StorageMode::kDevicePrivate, // color_resolve_storage_mode
LoadAction::kDontCare, // color_load_action
StoreAction::kMultisampleResolve, // color_store_action
readable ? StorageMode::kDevicePrivate
: StorageMode::kDeviceTransient, // stencil_storage_mode
LoadAction::kDontCare, // stencil_load_action
StoreAction::kDontCare // stencil_store_action
*context, // context
size, // size
"EntityPass", // label
RenderTarget::AttachmentConfigMSAA{
.storage_mode = StorageMode::kDeviceTransient,
.resolve_storage_mode = StorageMode::kDevicePrivate,
.load_action = LoadAction::kDontCare,
.store_action = StoreAction::kMultisampleResolve,
}, // color_attachment_config
RenderTarget::AttachmentConfig{
.storage_mode = readable ? StorageMode::kDevicePrivate
: StorageMode::kDeviceTransient,
.load_action = LoadAction::kDontCare,
.store_action = StoreAction::kDontCare,
} // stencil_attachment_config
);
}

return RenderTarget::CreateOffscreen(
*context, // context
size, // size
"EntityPass", // label
StorageMode::kDevicePrivate, // color_storage_mode
LoadAction::kDontCare, // color_load_action
StoreAction::kDontCare, // color_store_action
readable ? StorageMode::kDevicePrivate
: StorageMode::kDeviceTransient, // stencil_storage_mode
LoadAction::kDontCare, // stencil_load_action
StoreAction::kDontCare // stencil_store_action
*context, // context
size, // size
"EntityPass", // label
RenderTarget::AttachmentConfig{
.storage_mode = StorageMode::kDevicePrivate,
.load_action = LoadAction::kDontCare,
.store_action = StoreAction::kDontCare,
}, // color_attachment_config
RenderTarget::AttachmentConfig{
.storage_mode = readable ? StorageMode::kDevicePrivate
: StorageMode::kDeviceTransient,
.load_action = LoadAction::kDontCare,
.store_action = StoreAction::kDontCare,
} // stencil_attachment_config
);
}

Expand Down
6 changes: 2 additions & 4 deletions impeller/playground/imgui/imgui_impl_impeller.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,8 @@ bool ImGui_ImplImpeller_Init(
auto desc = impeller::PipelineBuilder<impeller::ImguiRasterVertexShader,
impeller::ImguiRasterFragmentShader>::
MakeDefaultPipelineDescriptor(*context);
desc->SetStencilPixelFormat(impeller::PixelFormat::kUnknown);
desc->SetStencilAttachmentDescriptors(std::nullopt);
desc->SetDepthPixelFormat(impeller::PixelFormat::kUnknown);
desc->SetDepthStencilAttachmentDescriptor(std::nullopt);
desc->ClearStencilAttachments();
desc->ClearDepthAttachment();

bd->pipeline =
context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get();
Expand Down
20 changes: 20 additions & 0 deletions impeller/renderer/pipeline_descriptor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,26 @@ PipelineDescriptor& PipelineDescriptor::SetStencilAttachmentDescriptors(
return *this;
}

void PipelineDescriptor::ClearStencilAttachments() {
back_stencil_attachment_descriptor_.reset();
front_stencil_attachment_descriptor_.reset();
SetStencilPixelFormat(impeller::PixelFormat::kUnknown);
}

void PipelineDescriptor::ClearDepthAttachment() {
depth_attachment_descriptor_.reset();
SetDepthPixelFormat(impeller::PixelFormat::kUnknown);
}

void PipelineDescriptor::ClearColorAttachment(size_t index) {
if (color_attachment_descriptors_.find(index) ==
color_attachment_descriptors_.end()) {
return;
}

color_attachment_descriptors_.erase(index);
}

void PipelineDescriptor::ResetAttachments() {
color_attachment_descriptors_.clear();
depth_attachment_descriptor_.reset();
Expand Down
6 changes: 6 additions & 0 deletions impeller/renderer/pipeline_descriptor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ class PipelineDescriptor final : public Comparable<PipelineDescriptor> {
std::optional<StencilAttachmentDescriptor> front,
std::optional<StencilAttachmentDescriptor> back);

void ClearStencilAttachments();

void ClearDepthAttachment();

void ClearColorAttachment(size_t index);

std::optional<StencilAttachmentDescriptor>
GetFrontStencilAttachmentDescriptor() const;

Expand Down
Loading