From bed3ae87ee9410281a9b7d44ad7fac2f6aecc630 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Tue, 19 Sep 2023 16:25:07 -0700 Subject: [PATCH] [Impeller] Fix validation errors in RendererTest. (flutter/engine#46076) Vulkan validation was tripping on the fact that renderer tests were rendering to the root render pass. This root render pass doesn't contain a stencil buffer. However, `MakeDefaultPipelineDescriptor` assumes a stencil and color attachment. The other backends are resilient to this mismatch since there is no compat render pass created upfront. But Vulkan was sad. This should only happen in the low level tests. In the higher levels of the framework, we have variants that make sure there is a pipeline pass and render pass compatibility. Fixes validations of the kind: ``` --- Vulkan Debug Report ---------------------------------------- | Severity: Error | Type: { Validation } | ID Name: VUID-vkCmdDraw-renderPass-02684 | ID Number: 1349015333 | Queue Breadcrumbs: [NONE] | CMD Buffer Breadcrumbs: [NONE] | Related Objects: RenderPass [16305153808034431137] [Playground Render Pass], RenderPass [18100546345029861533] [Compat Render Pass: BoxFade Pipeline] | Trigger: Validation Error: [ VUID-vkCmdDraw-renderPass-02684 ] Object 0: handle = 0xe2478b00000000a1, name = Playground Render Pass, type = VK_OBJECT_TYPE_RENDER_PASS; Object 1: handle = 0xfb320f000000009d, name = Compat Render Pass: BoxFade Pipeline, type = VK_OBJECT_TYPE_RENDER_PASS; | MessageID = 0x50685725 | vkCmdDraw: RenderPasses incompatible between active render pass w/ VkRenderPass 0xe2478b00000000a1[Playground Render Pass] and pipeline state object w/ VkRenderPass 0xfb320f000000009d[Compat Render Pass: BoxFade Pipeline] Attachment 4294967295 is not compatible with 1: The first is unused while the second is not.. The Vulkan spec states: The current render pass must be compatible with the renderPass member of the VkGraphicsPipelineCreateInfo structure specified when creating the VkPipeline bound to VK_PIPELINE_BIND_POINT_GRAPHICS (https://vulkan.lunarg.com/doc/view/1.3.224.1/mac/1.3-extensions/vkspec.html#VUID-vkCmdDraw-renderPass-02684) ----------------------------------------------------------------- ``` --- .../backend/vulkan/pipeline_library_vk.cc | 15 ++++++++++++--- .../impeller/renderer/renderer_unittests.cc | 14 +++++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index b47c9d57237..830d2d08120 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -75,8 +75,9 @@ static vk::AttachmentDescription CreatePlaceholderAttachmentDescription( /// spec: /// https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/chap8.html#renderpass-compatibility /// -static vk::UniqueRenderPass CreateRenderPass(const vk::Device& device, - const PipelineDescriptor& desc) { +static vk::UniqueRenderPass CreateCompatRenderPassForPipeline( + const vk::Device& device, + const PipelineDescriptor& desc) { std::vector attachments; std::vector color_refs; @@ -128,6 +129,13 @@ static vk::UniqueRenderPass CreateRenderPass(const vk::Device& device, return {}; } + // This pass is not used with the render pass. It is only necessary to tell + // Vulkan the expected render pass layout. The actual pass will be created + // later during render pass setup and will need to be compatible with this + // one. + ContextVK::SetDebugName(device, pass.get(), + "Compat Render Pass: " + desc.GetLabel()); + return std::move(pass); } @@ -332,7 +340,8 @@ std::unique_ptr PipelineLibraryVK::CreatePipeline( return nullptr; } - auto render_pass = CreateRenderPass(strong_device->GetDevice(), desc); + auto render_pass = + CreateCompatRenderPassForPipeline(strong_device->GetDevice(), desc); if (render_pass) { pipeline_info.setBasePipelineHandle(VK_NULL_HANDLE); pipeline_info.setSubpass(0); diff --git a/engine/src/flutter/impeller/renderer/renderer_unittests.cc b/engine/src/flutter/impeller/renderer/renderer_unittests.cc index 575178f229b..48772d09198 100644 --- a/engine/src/flutter/impeller/renderer/renderer_unittests.cc +++ b/engine/src/flutter/impeller/renderer/renderer_unittests.cc @@ -58,6 +58,7 @@ TEST_P(RendererTest, CanCreateBoxPrimitive) { auto desc = BoxPipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); + desc->SetStencilAttachmentDescriptors(std::nullopt); // Vertex buffer. VertexBufferBuilder vertex_builder; @@ -130,6 +131,7 @@ TEST_P(RendererTest, CanRenderPerspectiveCube) { desc->SetCullMode(CullMode::kBackFace); desc->SetWindingOrder(WindingOrder::kCounterClockwise); desc->SetSampleCount(SampleCount::kCount4); + desc->SetStencilAttachmentDescriptors(std::nullopt); auto pipeline = context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(pipeline); @@ -219,6 +221,7 @@ TEST_P(RendererTest, CanRenderMultiplePrimitives) { auto desc = BoxPipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); + desc->SetStencilAttachmentDescriptors(std::nullopt); auto box_pipeline = context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(box_pipeline); @@ -420,7 +423,9 @@ TEST_P(RendererTest, CanRenderInstanced) { ->GetPipelineLibrary() ->GetPipeline(PipelineBuilder::MakeDefaultPipelineDescriptor( *GetContext()) - ->SetSampleCount(SampleCount::kCount4)) + ->SetSampleCount(SampleCount::kCount4) + .SetStencilAttachmentDescriptors(std::nullopt)) + .Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); @@ -459,6 +464,7 @@ TEST_P(RendererTest, CanBlitTextureToTexture) { auto desc = PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); + desc->SetStencilAttachmentDescriptors(std::nullopt); auto mipmaps_pipeline = context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(mipmaps_pipeline); @@ -571,6 +577,7 @@ TEST_P(RendererTest, CanBlitTextureToBuffer) { auto desc = PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); + desc->SetStencilAttachmentDescriptors(std::nullopt); auto mipmaps_pipeline = context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(mipmaps_pipeline); @@ -702,6 +709,7 @@ TEST_P(RendererTest, CanGenerateMipmaps) { auto desc = PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(desc.has_value()); desc->SetSampleCount(SampleCount::kCount4); + desc->SetStencilAttachmentDescriptors(std::nullopt); auto mipmaps_pipeline = context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get(); ASSERT_TRUE(mipmaps_pipeline); @@ -818,6 +826,7 @@ TEST_P(RendererTest, TheImpeller) { PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); + pipeline_descriptor->SetStencilAttachmentDescriptors(std::nullopt); auto pipeline = context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); @@ -878,6 +887,7 @@ TEST_P(RendererTest, ArrayUniforms) { PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); + pipeline_descriptor->SetStencilAttachmentDescriptors(std::nullopt); auto pipeline = context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); @@ -934,6 +944,7 @@ TEST_P(RendererTest, InactiveUniforms) { PipelineBuilder::MakeDefaultPipelineDescriptor(*context); ASSERT_TRUE(pipeline_descriptor.has_value()); pipeline_descriptor->SetSampleCount(SampleCount::kCount4); + pipeline_descriptor->SetStencilAttachmentDescriptors(std::nullopt); auto pipeline = context->GetPipelineLibrary()->GetPipeline(pipeline_descriptor).Get(); ASSERT_TRUE(pipeline && pipeline->IsValid()); @@ -1120,6 +1131,7 @@ TEST_P(RendererTest, StencilMask) { ASSERT_TRUE(vertex_buffer); desc->SetSampleCount(SampleCount::kCount4); + desc->SetStencilAttachmentDescriptors(std::nullopt); auto bridge = CreateTextureForFixture("bay_bridge.jpg"); auto boston = CreateTextureForFixture("boston.jpg");