Skip to content

Commit

Permalink
[Impeller] Fix validation errors in RendererTest. (#46076)
Browse files Browse the repository at this point in the history
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)
-----------------------------------------------------------------

```
  • Loading branch information
chinmaygarde authored Sep 19, 2023
1 parent 0d7db40 commit bf58df8
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
15 changes: 12 additions & 3 deletions impeller/renderer/backend/vulkan/pipeline_library_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<vk::AttachmentDescription> attachments;

std::vector<vk::AttachmentReference> color_refs;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -332,7 +340,8 @@ std::unique_ptr<PipelineVK> 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);
Expand Down
14 changes: 13 additions & 1 deletion impeller/renderer/renderer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<VS::PerVertexData> vertex_builder;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -420,7 +423,9 @@ TEST_P(RendererTest, CanRenderInstanced) {
->GetPipelineLibrary()
->GetPipeline(PipelineBuilder<VS, FS>::MakeDefaultPipelineDescriptor(
*GetContext())
->SetSampleCount(SampleCount::kCount4))
->SetSampleCount(SampleCount::kCount4)
.SetStencilAttachmentDescriptors(std::nullopt))

.Get();
ASSERT_TRUE(pipeline && pipeline->IsValid());

Expand Down Expand Up @@ -459,6 +464,7 @@ TEST_P(RendererTest, CanBlitTextureToTexture) {
auto desc = PipelineBuilder<VS, FS>::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);
Expand Down Expand Up @@ -571,6 +577,7 @@ TEST_P(RendererTest, CanBlitTextureToBuffer) {
auto desc = PipelineBuilder<VS, FS>::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);
Expand Down Expand Up @@ -702,6 +709,7 @@ TEST_P(RendererTest, CanGenerateMipmaps) {
auto desc = PipelineBuilder<VS, FS>::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);
Expand Down Expand Up @@ -818,6 +826,7 @@ TEST_P(RendererTest, TheImpeller) {
PipelineBuilder<VS, FS>::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());
Expand Down Expand Up @@ -878,6 +887,7 @@ TEST_P(RendererTest, ArrayUniforms) {
PipelineBuilder<VS, FS>::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());
Expand Down Expand Up @@ -934,6 +944,7 @@ TEST_P(RendererTest, InactiveUniforms) {
PipelineBuilder<VS, FS>::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());
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit bf58df8

Please sign in to comment.