Set writemask to 0 when no fs output matches color state on Metal and Vulkan
This patch fixes an undefined behaviour on Metal and Vulkan when there
is a color state whose corresponding fragment output is not declared in
the fragment shader.
According to Vulkan SPEC (Chapter 14.3), the input values to blending or
color attachment writes are undefined for components which do not
correspond to a fragment shader output. Vulkan validation layer follows
the SPEC that it only allows the shader to not produce a matching output
if the writemask is 0, or it will report a warning when the application
is against this rule.
When no fragment output matches the color state in a render pipeline,
the output differs on different Metal devices. On some Metal devices the
fragment output will be (0, 0, 0, 0) even if it is not declared in the
shader, while on others there will be no fragment outputs and the content
in the color attachments is not changed.
This patch fixes this issue by setting the color write mask to 0 to
prevent the undefined values being written into the color attachments.
With this patch, the following end2end tests will not report warnings
any more when we enable the Vulkan validation layer:
ObjectCachingTest.RenderPipelineDeduplicationOnLayout/Vulkan
ObjectCachingTest.RenderPipelineDeduplicationOnVertexModule/Vulkan
ObjectCachingTest.RenderPipelineDeduplicationOnFragmentModule/Vulkan
BUG=dawn:209
TEST=dawn_end2end_tests
Change-Id: I5613daa1b9a45349ea1459fbdfe4a12d6149f0f7
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/11581
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
diff --git a/src/dawn_native/metal/RenderPipelineMTL.mm b/src/dawn_native/metal/RenderPipelineMTL.mm
index c00d488..a4ea8c6 100644
--- a/src/dawn_native/metal/RenderPipelineMTL.mm
+++ b/src/dawn_native/metal/RenderPipelineMTL.mm
@@ -182,7 +182,12 @@
}
}
- MTLColorWriteMask MetalColorWriteMask(dawn::ColorWriteMask writeMask) {
+ MTLColorWriteMask MetalColorWriteMask(dawn::ColorWriteMask writeMask,
+ bool isDeclaredInFragmentShader) {
+ if (!isDeclaredInFragmentShader) {
+ return MTLColorWriteMaskNone;
+ }
+
MTLColorWriteMask mask = MTLColorWriteMaskNone;
if (writeMask & dawn::ColorWriteMask::Red) {
@@ -202,7 +207,8 @@
}
void ComputeBlendDesc(MTLRenderPipelineColorAttachmentDescriptor* attachment,
- const ColorStateDescriptor* descriptor) {
+ const ColorStateDescriptor* descriptor,
+ bool isDeclaredInFragmentShader) {
attachment.blendingEnabled = BlendEnabled(descriptor);
attachment.sourceRGBBlendFactor =
MetalBlendFactor(descriptor->colorBlend.srcFactor, false);
@@ -214,7 +220,8 @@
attachment.destinationAlphaBlendFactor =
MetalBlendFactor(descriptor->alphaBlend.dstFactor, true);
attachment.alphaBlendOperation = MetalBlendOperation(descriptor->alphaBlend.operation);
- attachment.writeMask = MetalColorWriteMask(descriptor->writeMask);
+ attachment.writeMask =
+ MetalColorWriteMask(descriptor->writeMask, isDeclaredInFragmentShader);
}
MTLStencilOperation MetalStencilOperation(dawn::StencilOperation stencilOperation) {
@@ -339,11 +346,15 @@
descriptorMTL.stencilAttachmentPixelFormat = MetalPixelFormat(depthStencilFormat);
}
+ const ShaderModuleBase::FragmentOutputBaseTypes& fragmentOutputBaseTypes =
+ descriptor->fragmentStage->module->GetFragmentOutputBaseTypes();
for (uint32_t i : IterateBitSet(GetColorAttachmentsMask())) {
descriptorMTL.colorAttachments[i].pixelFormat =
MetalPixelFormat(GetColorAttachmentFormat(i));
const ColorStateDescriptor* descriptor = GetColorStateDescriptor(i);
- ComputeBlendDesc(descriptorMTL.colorAttachments[i], descriptor);
+ bool isDeclaredInFragmentShader = fragmentOutputBaseTypes[i] != Format::Other;
+ ComputeBlendDesc(descriptorMTL.colorAttachments[i], descriptor,
+ isDeclaredInFragmentShader);
}
descriptorMTL.inputPrimitiveTopology = MTLInputPrimitiveTopology(GetPrimitiveTopology());
diff --git a/src/dawn_native/vulkan/RenderPipelineVk.cpp b/src/dawn_native/vulkan/RenderPipelineVk.cpp
index 8b8f5bf..eed883e 100644
--- a/src/dawn_native/vulkan/RenderPipelineVk.cpp
+++ b/src/dawn_native/vulkan/RenderPipelineVk.cpp
@@ -207,7 +207,8 @@
}
}
- VkColorComponentFlagBits VulkanColorWriteMask(dawn::ColorWriteMask mask) {
+ VkColorComponentFlags VulkanColorWriteMask(dawn::ColorWriteMask mask,
+ bool isDeclaredInFragmentShader) {
// Vulkan and Dawn color write masks match, static assert it and return the mask
static_assert(static_cast<VkColorComponentFlagBits>(dawn::ColorWriteMask::Red) ==
VK_COLOR_COMPONENT_R_BIT,
@@ -222,11 +223,16 @@
VK_COLOR_COMPONENT_A_BIT,
"");
- return static_cast<VkColorComponentFlagBits>(mask);
+ // According to Vulkan SPEC (Chapter 14.3): "The input values to blending or color
+ // attachment writes are undefined for components which do not correspond to a fragment
+ // shader outputs", we set the color write mask to 0 to prevent such undefined values
+ // being written into the color attachments.
+ return isDeclaredInFragmentShader ? static_cast<VkColorComponentFlags>(mask)
+ : static_cast<VkColorComponentFlags>(0);
}
- VkPipelineColorBlendAttachmentState ComputeColorDesc(
- const ColorStateDescriptor* descriptor) {
+ VkPipelineColorBlendAttachmentState ComputeColorDesc(const ColorStateDescriptor* descriptor,
+ bool isDeclaredInFragmentShader) {
VkPipelineColorBlendAttachmentState attachment;
attachment.blendEnable = BlendEnabled(descriptor) ? VK_TRUE : VK_FALSE;
attachment.srcColorBlendFactor = VulkanBlendFactor(descriptor->colorBlend.srcFactor);
@@ -235,7 +241,8 @@
attachment.srcAlphaBlendFactor = VulkanBlendFactor(descriptor->alphaBlend.srcFactor);
attachment.dstAlphaBlendFactor = VulkanBlendFactor(descriptor->alphaBlend.dstFactor);
attachment.alphaBlendOp = VulkanBlendOperation(descriptor->alphaBlend.operation);
- attachment.colorWriteMask = VulkanColorWriteMask(descriptor->writeMask);
+ attachment.colorWriteMask =
+ VulkanColorWriteMask(descriptor->writeMask, isDeclaredInFragmentShader);
return attachment;
}
@@ -400,9 +407,13 @@
// Initialize the "blend state info" that will be chained in the "create info" from the data
// pre-computed in the ColorState
std::array<VkPipelineColorBlendAttachmentState, kMaxColorAttachments> colorBlendAttachments;
+ const ShaderModuleBase::FragmentOutputBaseTypes& fragmentOutputBaseTypes =
+ descriptor->fragmentStage->module->GetFragmentOutputBaseTypes();
for (uint32_t i : IterateBitSet(GetColorAttachmentsMask())) {
- const ColorStateDescriptor* descriptor = GetColorStateDescriptor(i);
- colorBlendAttachments[i] = ComputeColorDesc(descriptor);
+ const ColorStateDescriptor* colorStateDescriptor = GetColorStateDescriptor(i);
+ bool isDeclaredInFragmentShader = fragmentOutputBaseTypes[i] != Format::Other;
+ colorBlendAttachments[i] =
+ ComputeColorDesc(colorStateDescriptor, isDeclaredInFragmentShader);
}
VkPipelineColorBlendStateCreateInfo colorBlend;
colorBlend.sType = VK_STRUCTURE_TYPE_PIPELINE_COLOR_BLEND_STATE_CREATE_INFO;
diff --git a/src/tests/end2end/RenderPassTests.cpp b/src/tests/end2end/RenderPassTests.cpp
index 2676df6..8b147c5 100644
--- a/src/tests/end2end/RenderPassTests.cpp
+++ b/src/tests/end2end/RenderPassTests.cpp
@@ -26,8 +26,7 @@
DawnTest::SetUp();
// Shaders to draw a bottom-left triangle in blue.
- dawn::ShaderModule vsModule =
- utils::CreateShaderModule(device, utils::SingleShaderStage::Vertex, R"(
+ mVSModule = utils::CreateShaderModule(device, utils::SingleShaderStage::Vertex, R"(
#version 450
void main() {
const vec2 pos[3] = vec2[3](
@@ -44,7 +43,7 @@
})");
utils::ComboRenderPipelineDescriptor descriptor(device);
- descriptor.vertexStage.module = vsModule;
+ descriptor.vertexStage.module = mVSModule;
descriptor.cFragmentStage.module = fsModule;
descriptor.primitiveTopology = dawn::PrimitiveTopology::TriangleStrip;
descriptor.cColorStates[0].format = kFormat;
@@ -66,6 +65,7 @@
return device.CreateTexture(&descriptor);
}
+ dawn::ShaderModule mVSModule;
dawn::RenderPipeline pipeline;
};
@@ -119,4 +119,56 @@
EXPECT_PIXEL_RGBA8_EQ(kGreen, renderTarget2, kRTSize - 1, 1);
}
+// Verify that the content in the color attachment will not be changed if there is no corresponding
+// fragment shader outputs in the render pipeline, the load operation is LoadOp::Load and the store
+// operation is StoreOp::Store.
+TEST_P(RenderPassTest, NoCorrespondingFragmentShaderOutputs) {
+ dawn::Texture renderTarget = CreateDefault2DTexture();
+ dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+
+ dawn::TextureView renderTargetView = renderTarget.CreateView();
+
+ utils::ComboRenderPassDescriptor renderPass({renderTargetView});
+ renderPass.cColorAttachments[0].clearColor = {1.0f, 0.0f, 0.0f, 1.0f};
+ renderPass.cColorAttachments[0].loadOp = dawn::LoadOp::Clear;
+ renderPass.cColorAttachments[0].storeOp = dawn::StoreOp::Store;
+ dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+
+ {
+ // First we draw a blue triangle in the bottom left of renderTarget.
+ pass.SetPipeline(pipeline);
+ pass.Draw(3, 1, 0, 0);
+ }
+
+ {
+ // Next we use a pipeline whose fragment shader has no outputs.
+ dawn::ShaderModule fsModule =
+ utils::CreateShaderModule(device, utils::SingleShaderStage::Fragment, R"(
+ #version 450
+ void main() {
+ })");
+ utils::ComboRenderPipelineDescriptor descriptor(device);
+ descriptor.vertexStage.module = mVSModule;
+ descriptor.cFragmentStage.module = fsModule;
+ descriptor.primitiveTopology = dawn::PrimitiveTopology::TriangleStrip;
+ descriptor.cColorStates[0].format = kFormat;
+
+ dawn::RenderPipeline pipelineWithNoFragmentOutput =
+ device.CreateRenderPipeline(&descriptor);
+
+ pass.SetPipeline(pipelineWithNoFragmentOutput);
+ pass.Draw(3, 1, 0, 0);
+ }
+
+ pass.EndPass();
+
+ dawn::CommandBuffer commands = encoder.Finish();
+ queue.Submit(1, &commands);
+
+ constexpr RGBA8 kRed(255, 0, 0, 255);
+ constexpr RGBA8 kBlue(0, 0, 255, 255);
+ EXPECT_PIXEL_RGBA8_EQ(kBlue, renderTarget, 2, kRTSize - 1);
+ EXPECT_PIXEL_RGBA8_EQ(kRed, renderTarget, kRTSize - 1, 1);
+}
+
DAWN_INSTANTIATE_TEST(RenderPassTest, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend);
