From 3619a2b017988154ce2aea42431ee8d35b18cd88 Mon Sep 17 00:00:00 2001
From: Jiawei Shao <jiawei.shao@intel.com>
Date: Fri, 21 Jan 2022 02:05:47 +0000
Subject: [PATCH] Add validation on blend factors and blend operations

This patch adds validations on blend factors and blend
operations according to the latest WebGPU SPEC:
If component.operation is "min" or "max":
component.srcFactor and component.dstFactor must both
be "one".

BUG=dawn:1257
TEST=dawn_unittests

Change-Id: Id17c06044900eb0fa8d2ebab6fd3132f9deb157a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/76480
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
---
 src/dawn_native/RenderPipeline.cpp            | 15 ++++++
 .../RenderPipelineValidationTests.cpp         | 48 +++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp
index 715f8efe30..200795a01c 100644
--- a/src/dawn_native/RenderPipeline.cpp
+++ b/src/dawn_native/RenderPipeline.cpp
@@ -317,6 +317,18 @@ namespace dawn::native {
             return {};
         }
 
+        MaybeError ValidateBlendComponent(BlendComponent blendComponent) {
+            if (blendComponent.operation == wgpu::BlendOperation::Min ||
+                blendComponent.operation == wgpu::BlendOperation::Max) {
+                DAWN_INVALID_IF(blendComponent.srcFactor != wgpu::BlendFactor::One ||
+                                    blendComponent.dstFactor != wgpu::BlendFactor::One,
+                                "Blend factor is not %s when blend operation is %s.",
+                                wgpu::BlendFactor::One, blendComponent.operation);
+            }
+
+            return {};
+        }
+
         MaybeError ValidateBlendState(DeviceBase* device, const BlendState* descriptor) {
             DAWN_TRY(ValidateBlendOperation(descriptor->alpha.operation));
             DAWN_TRY(ValidateBlendFactor(descriptor->alpha.srcFactor));
@@ -324,6 +336,9 @@ namespace dawn::native {
             DAWN_TRY(ValidateBlendOperation(descriptor->color.operation));
             DAWN_TRY(ValidateBlendFactor(descriptor->color.srcFactor));
             DAWN_TRY(ValidateBlendFactor(descriptor->color.dstFactor));
+            DAWN_TRY(ValidateBlendComponent(descriptor->alpha));
+            DAWN_TRY(ValidateBlendComponent(descriptor->color));
+
             return {};
         }
 
diff --git a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp
index b53378fd92..c8c9da9a5f 100644
--- a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp
+++ b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp
@@ -403,6 +403,54 @@ TEST_F(RenderPipelineValidationTest, FragmentOutputComponentCountCompatibility)
     }
 }
 
+// Tests that when blendOperationMinOrMax is "min" or "max", both srcBlendFactor and dstBlendFactor
+// must be "one".
+TEST_F(RenderPipelineValidationTest, BlendOperationAndBlendFactors) {
+    constexpr std::array<wgpu::BlendFactor, 8> kBlendFactors = {wgpu::BlendFactor::Zero,
+                                                                wgpu::BlendFactor::One,
+                                                                wgpu::BlendFactor::SrcAlpha,
+                                                                wgpu::BlendFactor::OneMinusSrcAlpha,
+                                                                wgpu::BlendFactor::Src,
+                                                                wgpu::BlendFactor::DstAlpha,
+                                                                wgpu::BlendFactor::OneMinusDstAlpha,
+                                                                wgpu::BlendFactor::Dst};
+
+    constexpr std::array<wgpu::BlendOperation, 2> kBlendOperationsForTest = {
+        wgpu::BlendOperation::Max, wgpu::BlendOperation::Min};
+
+    for (wgpu::BlendOperation blendOperationMinOrMax : kBlendOperationsForTest) {
+        for (wgpu::BlendFactor srcFactor : kBlendFactors) {
+            for (wgpu::BlendFactor dstFactor : kBlendFactors) {
+                utils::ComboRenderPipelineDescriptor descriptor;
+                descriptor.vertex.module = vsModule;
+                descriptor.cFragment.module = fsModule;
+                descriptor.cTargets[0].format = wgpu::TextureFormat::RGBA8Unorm;
+                descriptor.cTargets[0].blend = &descriptor.cBlends[0];
+                descriptor.cBlends[0].color.srcFactor = srcFactor;
+                descriptor.cBlends[0].color.dstFactor = dstFactor;
+                descriptor.cBlends[0].alpha.srcFactor = srcFactor;
+                descriptor.cBlends[0].alpha.dstFactor = dstFactor;
+
+                descriptor.cBlends[0].color.operation = blendOperationMinOrMax;
+                descriptor.cBlends[0].alpha.operation = wgpu::BlendOperation::Add;
+                if (srcFactor == wgpu::BlendFactor::One && dstFactor == wgpu::BlendFactor::One) {
+                    device.CreateRenderPipeline(&descriptor);
+                } else {
+                    ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
+                }
+
+                descriptor.cBlends[0].color.operation = wgpu::BlendOperation::Add;
+                descriptor.cBlends[0].alpha.operation = blendOperationMinOrMax;
+                if (srcFactor == wgpu::BlendFactor::One && dstFactor == wgpu::BlendFactor::One) {
+                    device.CreateRenderPipeline(&descriptor);
+                } else {
+                    ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
+                }
+            }
+        }
+    }
+}
+
 /// Tests that the sample count of the render pipeline must be valid.
 TEST_F(RenderPipelineValidationTest, SampleCount) {
     {