Compat: Reject non-matching blend state / write mask

Bug: dawn:1839
Change-Id: I1284fb0c5b6a0d21603d9a9806d31e931b17b615
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/134061
Reviewed-by: Stephen White <senorblanco@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Gregg Tavares <gman@chromium.org>

src/dawn/native/Device.cpp

@@ -1450,7 +1450,7 @@ bool DeviceBase::IsRobustnessEnabled() const {
 }
 
 bool DeviceBase::IsCompatibilityMode() const {
-    return mAdapter->GetFeatureLevel() == FeatureLevel::Compatibility;
+    return mAdapter != nullptr && mAdapter->GetFeatureLevel() == FeatureLevel::Compatibility;
 }
 
 bool DeviceBase::AllowUnsafeAPIs() const {

src/dawn/native/RenderPipeline.cpp

@@ -352,6 +352,61 @@ MaybeError ValidateColorTargetState(
     return {};
 }
 
+MaybeError ValidateCompatibilityColorTargetState(
+    const uint8_t firstColorTargetIndex,
+    const ColorTargetState* const firstColorTargetState,
+    const uint8_t targetIndex,
+    const ColorTargetState* target) {
+    DAWN_INVALID_IF(firstColorTargetState->writeMask != target->writeMask,
+                    "targets[%u].writeMask (%s) does not match targets[%u].writeMask (%s).",
+                    targetIndex, target->writeMask, firstColorTargetIndex,
+                    firstColorTargetState->writeMask);
+    if (!firstColorTargetState->blend) {
+        DAWN_INVALID_IF(target->blend,
+                        "targets[%u].blend has a blend state but targets[%u].blend does not.",
+                        targetIndex, firstColorTargetIndex);
+    } else {
+        DAWN_INVALID_IF(!target->blend,
+                        "targets[%u].blend has a blend state but targets[%u].blend does not.",
+                        firstColorTargetIndex, targetIndex);
+
+        const BlendState& currBlendState = *target->blend;
+        const BlendState& firstBlendState = *firstColorTargetState->blend;
+
+        DAWN_INVALID_IF(
+            firstBlendState.color.operation != currBlendState.color.operation,
+            "targets[%u].color.operation (%s) does not match targets[%u].color.operation (%s).",
+            firstColorTargetIndex, firstBlendState.color.operation, targetIndex,
+            currBlendState.color.operation);
+        DAWN_INVALID_IF(
+            firstBlendState.color.srcFactor != currBlendState.color.srcFactor,
+            "targets[%u].color.srcFactor (%s) does not match targets[%u].color.srcFactor (%s).",
+            firstColorTargetIndex, firstBlendState.color.srcFactor, targetIndex,
+            currBlendState.color.srcFactor);
+        DAWN_INVALID_IF(
+            firstBlendState.color.dstFactor != currBlendState.color.dstFactor,
+            "targets[%u].color.dstFactor (%s) does not match targets[%u].color.dstFactor (%s).",
+            firstColorTargetIndex, firstBlendState.color.dstFactor, targetIndex,
+            currBlendState.color.dstFactor);
+        DAWN_INVALID_IF(
+            firstBlendState.alpha.operation != currBlendState.alpha.operation,
+            "targets[%u].alpha.operation (%s) does not match targets[%u].alpha.operation (%s).",
+            firstColorTargetIndex, firstBlendState.alpha.operation, targetIndex,
+            currBlendState.alpha.operation);
+        DAWN_INVALID_IF(
+            firstBlendState.alpha.srcFactor != currBlendState.alpha.srcFactor,
+            "targets[%u].alpha.srcFactor (%s) does not match targets[%u].alpha.srcFactor (%s).",
+            firstColorTargetIndex, firstBlendState.alpha.srcFactor, targetIndex,
+            currBlendState.alpha.srcFactor);
+        DAWN_INVALID_IF(
+            firstBlendState.alpha.dstFactor != currBlendState.alpha.dstFactor,
+            "targets[%u].alpha.dstFactor (%s) does not match targets[%u].alpha.dstFactor (%s).",
+            firstColorTargetIndex, firstBlendState.alpha.dstFactor, targetIndex,
+            currBlendState.alpha.dstFactor);
+    }
+    return {};
+}
+
 MaybeError ValidateFragmentState(DeviceBase* device,
                                  const FragmentState* descriptor,
                                  const PipelineLayoutBase* layout,
@@ -388,20 +443,36 @@ MaybeError ValidateFragmentState(DeviceBase* device,
                         depthStencil->format, descriptor->module, descriptor->entryPoint);
     }
 
+    uint8_t firstColorTargetIndex = 0;
+    const ColorTargetState* firstColorTargetState = nullptr;
     ColorAttachmentFormats colorAttachmentFormats;
-    for (ColorAttachmentIndex i(uint8_t(0));
+
-         i < ColorAttachmentIndex(static_cast<uint8_t>(descriptor->targetCount)); ++i) {
+    for (ColorAttachmentIndex attachmentIndex(uint8_t(0));
-        const ColorTargetState* target = &descriptor->targets[static_cast<uint8_t>(i)];
+         attachmentIndex < ColorAttachmentIndex(static_cast<uint8_t>(descriptor->targetCount));
+         ++attachmentIndex) {
+        const uint8_t i = static_cast<uint8_t>(attachmentIndex);
+        const ColorTargetState* target = &descriptor->targets[i];
+
         if (target->format != wgpu::TextureFormat::Undefined) {
             DAWN_TRY_CONTEXT(
-                ValidateColorTargetState(device, target, fragmentMetadata.fragmentOutputsWritten[i],
+                ValidateColorTargetState(device, target,
-                                         fragmentMetadata.fragmentOutputVariables[i]),
+                                         fragmentMetadata.fragmentOutputsWritten[attachmentIndex],
-                "validating targets[%u].", static_cast<uint8_t>(i));
+                                         fragmentMetadata.fragmentOutputVariables[attachmentIndex]),
+                "validating targets[%u].", i);
             colorAttachmentFormats->push_back(&device->GetValidInternalFormat(target->format));
+            if (device->IsCompatibilityMode()) {
+                if (!firstColorTargetState) {
+                    firstColorTargetState = target;
+                    firstColorTargetIndex = i;
+                } else {
+                    DAWN_TRY_CONTEXT(ValidateCompatibilityColorTargetState(
+                                         firstColorTargetIndex, firstColorTargetState, i, target),
+                                     "validating targets[%u] in compatibility mode.", i);
+                }
+            }
         } else {
             DAWN_INVALID_IF(target->blend,
-                            "Color target[%u] blend state is set when the format is undefined.",
+                            "Color target[%u] blend state is set when the format is undefined.", i);
-                            static_cast<uint8_t>(i));
         }
     }
     DAWN_TRY(ValidateColorAttachmentBytesPerSample(device, colorAttachmentFormats));

src/dawn/tests/end2end/PipelineCachingTests.cpp

@@ -460,6 +460,9 @@ TEST_P(SinglePipelineCachingTests, RenderPipelineBlobCacheShaderNegativeCases) {
 // Tests that pipeline creation wouldn't hit the cache if the pipelines are not exactly the same
 // (fragment color targets differences).
 TEST_P(SinglePipelineCachingTests, RenderPipelineBlobCacheNegativeCasesFragmentColorTargets) {
+    // In compat, all targets must have the same writeMask
+    DAWN_TEST_UNSUPPORTED_IF(IsCompatibilityMode());
+
     // First time should create and write out to the cache.
     {
         wgpu::Device device = CreateDevice();

src/dawn/tests/unittests/validation/CompatValidationTests.cpp

@@ -54,5 +54,87 @@ TEST_F(CompatValidationTest, CanNotCreateCubeArrayTextureView) {
     cubeTexture.Destroy();
 }
 
+TEST_F(CompatValidationTest, CanNotCreatePipelineWithDifferentPerTargetBlendStateOrWriteMask) {
+    wgpu::ShaderModule module = utils::CreateShaderModule(device, R"(
+        @vertex fn vs() -> @builtin(position) vec4f {
+            return vec4f(0);
+        }
+
+        struct FragmentOut {
+            @location(0) fragColor0 : vec4f,
+            @location(1) fragColor1 : vec4f,
+            @location(2) fragColor2 : vec4f,
+        }
+
+        @fragment fn fs() -> FragmentOut {
+            var output : FragmentOut;
+            output.fragColor0 = vec4f(0);
+            output.fragColor1 = vec4f(0);
+            output.fragColor2 = vec4f(0);
+            return output;
+        }
+    )");
+
+    utils::ComboRenderPipelineDescriptor testDescriptor;
+    testDescriptor.layout = {};
+    testDescriptor.vertex.module = module;
+    testDescriptor.vertex.entryPoint = "vs";
+    testDescriptor.cFragment.module = module;
+    testDescriptor.cFragment.entryPoint = "fs";
+    testDescriptor.cFragment.targetCount = 3;
+    testDescriptor.cTargets[1].format = wgpu::TextureFormat::Undefined;
+
+    for (int i = 0; i < 10; ++i) {
+        wgpu::BlendState blend0;
+        wgpu::BlendState blend2;
+
+        // Blend state intentionally omitted for target 1
+        testDescriptor.cTargets[0].blend = &blend0;
+        testDescriptor.cTargets[2].blend = &blend2;
+
+        bool expectError = true;
+        switch (i) {
+            case 0:  // default
+                expectError = false;
+                break;
+            case 1:  // no blend
+                testDescriptor.cTargets[0].blend = nullptr;
+                break;
+            case 2:  // no blend second target
+                testDescriptor.cTargets[2].blend = nullptr;
+                break;
+            case 3:  // color.operation
+                blend2.color.operation = wgpu::BlendOperation::Subtract;
+                break;
+            case 4:  // color.srcFactor
+                blend2.color.srcFactor = wgpu::BlendFactor::SrcAlpha;
+                break;
+            case 5:  // color.dstFactor
+                blend2.color.dstFactor = wgpu::BlendFactor::DstAlpha;
+                break;
+            case 6:  // alpha.operation
+                blend2.alpha.operation = wgpu::BlendOperation::Subtract;
+                break;
+            case 7:  // alpha.srcFactor
+                blend2.alpha.srcFactor = wgpu::BlendFactor::SrcAlpha;
+                break;
+            case 8:  // alpha.dstFactor
+                blend2.alpha.dstFactor = wgpu::BlendFactor::DstAlpha;
+                break;
+            case 9:  // writeMask
+                testDescriptor.cTargets[2].writeMask = wgpu::ColorWriteMask::Green;
+                break;
+            default:
+                UNREACHABLE();
+        }
+
+        if (expectError) {
+            ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&testDescriptor));
+        } else {
+            device.CreateRenderPipeline(&testDescriptor);
+        }
+    }
+}
+
 }  // anonymous namespace
 }  // namespace dawn