From c9ebe9104d79fdf8abf7b6f15e9056820e19f3ae Mon Sep 17 00:00:00 2001
From: Corentin Wallez <cwallez@chromium.org>
Date: Thu, 12 May 2022 18:09:46 +0000
Subject: [PATCH] Allow writeMask==0 && format==undefined when the shader
 outputs the target

The WebGPU spec says that a validation error is produced if there is a
color target in the pipeline descriptor that has writeMask!=0 while
there are no shader outputs for this target. In Dawn format==undefined
is used as a tag that the color target is not present, so writeMask is
allowed to be 0 if format==undefined. (the validation in the case where
format!=undefined was already present).

Fixed: dawn:1376
Change-Id: I7737b6557223e0fc31740fd4ec2cbfaa54b77a71
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/90040
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
Reviewed-by: Loko Kung <lokokung@google.com>
---
 src/dawn/native/RenderPipeline.cpp            |  4 ---
 .../RenderPipelineValidationTests.cpp         | 35 +++++++++++--------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/src/dawn/native/RenderPipeline.cpp b/src/dawn/native/RenderPipeline.cpp
index 6894bdbac6..75f6420634 100644
--- a/src/dawn/native/RenderPipeline.cpp
+++ b/src/dawn/native/RenderPipeline.cpp
@@ -348,10 +348,6 @@ MaybeError ValidateFragmentState(DeviceBase* device,
             DAWN_INVALID_IF(target->blend,
                             "Color target[%u] blend state is set when the format is undefined.",
                             static_cast<uint8_t>(i));
-            DAWN_INVALID_IF(
-                target->writeMask != wgpu::ColorWriteMask::None,
-                "Color target[%u] write mask is set to (%s) when the format is undefined.",
-                static_cast<uint8_t>(i), target->writeMask);
         }
     }
 
diff --git a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
index 079f9b352d..203599bf29 100644
--- a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
@@ -194,8 +194,8 @@ TEST_F(RenderPipelineValidationTest, ColorTargetStateRequired) {
     }
 }
 
-// Tests that target blend and writeMasks must not be set if the format is undefined.
-TEST_F(RenderPipelineValidationTest, UndefinedColorStateFormatWithBlendOrWriteMask) {
+// Tests that target blend must not be set if the format is undefined.
+TEST_F(RenderPipelineValidationTest, UndefinedColorStateFormatWithBlend) {
     {
         // Control case: Valid undefined format target.
         utils::ComboRenderPipelineDescriptor descriptor;
@@ -203,7 +203,6 @@ TEST_F(RenderPipelineValidationTest, UndefinedColorStateFormatWithBlendOrWriteMa
         descriptor.cFragment.module = fsModule;
         descriptor.cFragment.targetCount = 1;
         descriptor.cTargets[0].format = wgpu::TextureFormat::Undefined;
-        descriptor.cTargets[0].writeMask = wgpu::ColorWriteMask::None;
 
         device.CreateRenderPipeline(&descriptor);
     }
@@ -221,19 +220,25 @@ TEST_F(RenderPipelineValidationTest, UndefinedColorStateFormatWithBlendOrWriteMa
             device.CreateRenderPipeline(&descriptor),
             testing::HasSubstr("Color target[0] blend state is set when the format is undefined."));
     }
-    {
-        // Error case: undefined format target with write masking not being none.
-        utils::ComboRenderPipelineDescriptor descriptor;
-        descriptor.vertex.module = vsModule;
-        descriptor.cFragment.module = fsModule;
-        descriptor.cFragment.targetCount = 1;
-        descriptor.cTargets[0].format = wgpu::TextureFormat::Undefined;
-        descriptor.cTargets[0].blend = nullptr;
-        descriptor.cTargets[0].writeMask = wgpu::ColorWriteMask::All;
+}
 
-        ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor),
-                            testing::HasSubstr("Color target[0] write mask is set to"));
-    }
+// Tests that a color target that's present in the pipeline descriptor but not in the shader must
+// have its writeMask set to 0.
+TEST_F(RenderPipelineValidationTest, WriteMaskMustBeZeroForColorTargetWithNoShaderOutput) {
+    utils::ComboRenderPipelineDescriptor descriptor;
+    descriptor.vertex.module = vsModule;
+    descriptor.cFragment.module = fsModule;
+    descriptor.cFragment.targetCount = 2;
+    descriptor.cTargets[0].format = wgpu::TextureFormat::RGBA8Unorm;
+    descriptor.cTargets[1].format = wgpu::TextureFormat::RGBA8Unorm;
+
+    // Control case: Target 1 not output by the shader but has writeMask = 0
+    descriptor.cTargets[1].writeMask = wgpu::ColorWriteMask::None;
+    device.CreateRenderPipeline(&descriptor);
+
+    // Error case: the writeMask is not 0.
+    descriptor.cTargets[1].writeMask = wgpu::ColorWriteMask::Red;
+    ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
 }
 
 // Tests that the color formats must be renderable.