From 603b9157539e00212c30acceb1470ab21fd3e605 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Wed, 1 Dec 2021 10:21:38 -0800 Subject: [PATCH] Add render pass and pipeline sample count validation. --- .../renderer/backend/metal/pipeline_library_mtl.mm | 5 +---- impeller/renderer/backend/metal/render_pass_mtl.mm | 11 +++++++++++ impeller/renderer/pipeline_builder.h | 6 +++--- impeller/renderer/pipeline_descriptor.cc | 2 +- impeller/renderer/pipeline_descriptor.h | 6 +++--- impeller/renderer/render_target.cc | 7 +++++++ impeller/renderer/render_target.h | 6 ++++++ 7 files changed, 32 insertions(+), 11 deletions(-) diff --git a/impeller/renderer/backend/metal/pipeline_library_mtl.mm b/impeller/renderer/backend/metal/pipeline_library_mtl.mm index 9dcd5c5349447..ce0fadebbb69b 100644 --- a/impeller/renderer/backend/metal/pipeline_library_mtl.mm +++ b/impeller/renderer/backend/metal/pipeline_library_mtl.mm @@ -16,12 +16,11 @@ PipelineLibraryMTL::~PipelineLibraryMTL() = default; -// TODO(csg): Make PipelineDescriptor a struct and move this to formats_mtl. static MTLRenderPipelineDescriptor* GetMTLRenderPipelineDescriptor( const PipelineDescriptor& desc) { auto descriptor = [[MTLRenderPipelineDescriptor alloc] init]; descriptor.label = @(desc.GetLabel().c_str()); - descriptor.sampleCount = desc.GetSampleCount(); + descriptor.sampleCount = static_cast(desc.GetSampleCount()); for (const auto& entry : desc.GetStageEntrypoints()) { if (entry.first == ShaderStage::kVertex) { @@ -53,8 +52,6 @@ descriptor.stencilAttachmentPixelFormat = ToMTLPixelFormat(desc.GetStencilPixelFormat()); - descriptor.sampleCount = 4u; - return descriptor; } diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm index 972820c2eb286..cba003d628776 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -388,6 +388,8 @@ static bool Bind(PassBindingsCache& pass, return true; }; + const auto target_sample_count = render_target_.GetSampleCount(); + fml::closure pop_debug_marker = [encoder]() { [encoder popDebugGroup]; }; for (const auto& command : commands_) { if (command.index_count == 0u) { @@ -401,6 +403,14 @@ static bool Bind(PassBindingsCache& pass, } else { auto_pop_debug_marker.Release(); } + + if (target_sample_count != + command.pipeline->GetDescriptor().GetSampleCount()) { + VALIDATION_LOG << "Pipeline for command and the render target disagree " + "on sample counts."; + return false; + } + pass_bindings.SetRenderPipelineState( PipelineMTL::Cast(*command.pipeline).GetMTLRenderPipelineState()); pass_bindings.SetDepthStencilState( @@ -447,6 +457,7 @@ static bool Bind(PassBindingsCache& pass, bool RenderPassMTL::AddCommand(Command command) { if (!command) { + VALIDATION_LOG << "Attempted to add an invalid command to the render pass."; return false; } diff --git a/impeller/renderer/pipeline_builder.h b/impeller/renderer/pipeline_builder.h index 84712c4705eca..82e6a742d33a2 100644 --- a/impeller/renderer/pipeline_builder.h +++ b/impeller/renderer/pipeline_builder.h @@ -69,10 +69,10 @@ struct PipelineBuilder { FragmentShader::kEntrypointName, ShaderStage::kFragment); if (!vertex_function || !fragment_function) { - FML_LOG(ERROR) << "Could not resolve pipeline entrypoint(s) '" + VALIDATION_LOG << "Could not resolve pipeline entrypoint(s) '" << VertexShader::kEntrypointName << "' and '" << FragmentShader::kEntrypointName - << "' for pipline named '" << VertexShader::kLabel + << "' for pipeline named '" << VertexShader::kLabel << "'."; return false; } @@ -86,7 +86,7 @@ struct PipelineBuilder { auto vertex_descriptor = std::make_shared(); if (!vertex_descriptor->SetStageInputs( VertexShader::kAllShaderStageInputs)) { - FML_LOG(ERROR) + VALIDATION_LOG << "Could not configure vertex descriptor for pipeline named '" << VertexShader::kLabel << "'."; return false; diff --git a/impeller/renderer/pipeline_descriptor.cc b/impeller/renderer/pipeline_descriptor.cc index eb24bb483ab3a..ca0a06ea2e602 100644 --- a/impeller/renderer/pipeline_descriptor.cc +++ b/impeller/renderer/pipeline_descriptor.cc @@ -61,7 +61,7 @@ PipelineDescriptor& PipelineDescriptor::SetLabel(std::string label) { return *this; } -PipelineDescriptor& PipelineDescriptor::SetSampleCount(size_t samples) { +PipelineDescriptor& PipelineDescriptor::SetSampleCount(SampleCount samples) { sample_count_ = samples; return *this; } diff --git a/impeller/renderer/pipeline_descriptor.h b/impeller/renderer/pipeline_descriptor.h index 225b0faf5f2b8..c26579b020a6f 100644 --- a/impeller/renderer/pipeline_descriptor.h +++ b/impeller/renderer/pipeline_descriptor.h @@ -32,9 +32,9 @@ class PipelineDescriptor final : public Comparable { const std::string& GetLabel() const; - PipelineDescriptor& SetSampleCount(size_t samples); + PipelineDescriptor& SetSampleCount(SampleCount samples); - size_t GetSampleCount() const { return sample_count_; } + SampleCount GetSampleCount() const { return sample_count_; } PipelineDescriptor& AddStageEntrypoint( std::shared_ptr function); @@ -97,7 +97,7 @@ class PipelineDescriptor final : public Comparable { private: std::string label_; - size_t sample_count_ = 1; + SampleCount sample_count_ = SampleCount::kCount1; std::map> entrypoints_; std::map color_attachment_descriptors_; diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 666ddece17c03..0e6f8bcf20e14 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -101,6 +101,13 @@ void RenderTarget::IterateAllAttachments( } } +SampleCount RenderTarget::GetSampleCount() const { + if (auto found = colors_.find(0u); found != colors_.end()) { + return found->second.texture->GetTextureDescriptor().sample_count; + } + return SampleCount::kCount1; +} + bool RenderTarget::HasColorAttachment(size_t index) const { if (auto found = colors_.find(index); found != colors_.end()) { return true; diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index 3ce7a5a480e4a..4fed906696155 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -22,12 +22,18 @@ class RenderTarget { ISize size, std::string label = "Offscreen"); + static RenderTarget CreateMSAA(const Context& context, + std::shared_ptr resolve_texture, + std::string label = "Offscreen"); + RenderTarget(); ~RenderTarget(); bool IsValid() const; + SampleCount GetSampleCount() const; + bool HasColorAttachment(size_t index) const; ISize GetRenderTargetSize() const;