Allow sparse color attachments

Add implementations and related tests reflecting the spec update.

Bug: dawn:1294
Change-Id: I2c20af313259e1d6d6049189cb8adebe4c2436af
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/81922
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Shrek Shao <shrekshao@google.com>

dawn.json

@@ -1802,7 +1802,7 @@
     "render pass color attachment": {
         "category": "structure",
         "members": [
-            {"name": "view", "type": "texture view"},
+            {"name": "view", "type": "texture view", "optional": true},
             {"name": "resolve target", "type": "texture view", "optional": true},
             {"name": "load op", "type": "load op"},
             {"name": "store op", "type": "store op"},

src/dawn/common/ityp_bitset.h

@@ -131,4 +131,56 @@ namespace ityp {
 
 }  // namespace ityp
 
+// Assume we have bitset of at most 64 bits
+// Returns i which is the next integer of the index of the highest bit
+// i == 0 if there is no bit set to true
+// i == 1 if only the least significant bit (at index 0) is the bit set to true with the
+// highest index
+// ...
+// i == 64 if the most significant bit (at index 64) is the bit set to true with the highest
+// index
+template <typename Index, size_t N>
+Index GetHighestBitIndexPlusOne(const ityp::bitset<Index, N>& bitset) {
+    using I = UnderlyingType<Index>;
+#if defined(DAWN_COMPILER_MSVC)
+    if constexpr (N > 32) {
+#    if defined(DAWN_PLATFORM_64_BIT)
+        unsigned long firstBitIndex = 0ul;
+        unsigned char ret = _BitScanReverse64(&firstBitIndex, bitset.to_ullong());
+        if (ret == 0) {
+            return Index(static_cast<I>(0));
+        }
+        return Index(static_cast<I>(firstBitIndex + 1));
+#    else   // defined(DAWN_PLATFORM_64_BIT)
+        if (bitset.none()) {
+            return Index(static_cast<I>(0));
+        }
+        for (size_t i = 0u; i < N; i++) {
+            if (bitset.test(Index(static_cast<I>(N - 1 - i)))) {
+                return Index(static_cast<I>(N - i));
+            }
+        }
+        UNREACHABLE();
+#    endif  // defined(DAWN_PLATFORM_64_BIT)
+    } else {
+        unsigned long firstBitIndex = 0ul;
+        unsigned char ret = _BitScanReverse(&firstBitIndex, bitset.to_ulong());
+        if (ret == 0) {
+            return Index(static_cast<I>(0));
+        }
+        return Index(static_cast<I>(firstBitIndex + 1));
+    }
+#else   // defined(DAWN_COMPILER_MSVC)
+    if (bitset.none()) {
+        return Index(static_cast<I>(0));
+    }
+    if constexpr (N > 32) {
+        return Index(
+            static_cast<I>(64 - static_cast<uint32_t>(__builtin_clzll(bitset.to_ullong()))));
+    } else {
+        return Index(static_cast<I>(32 - static_cast<uint32_t>(__builtin_clz(bitset.to_ulong()))));
+    }
+#endif  // defined(DAWN_COMPILER_MSVC)
+}
+
 #endif  // COMMON_ITYP_BITSET_H_

src/dawn/native/AttachmentState.cpp

@@ -27,8 +27,11 @@ namespace dawn::native {
         ASSERT(descriptor->colorFormatsCount <= kMaxColorAttachments);
         for (ColorAttachmentIndex i(uint8_t(0));
              i < ColorAttachmentIndex(static_cast<uint8_t>(descriptor->colorFormatsCount)); ++i) {
+            wgpu::TextureFormat format = descriptor->colorFormats[static_cast<uint8_t>(i)];
+            if (format != wgpu::TextureFormat::Undefined) {
                 mColorAttachmentsSet.set(i);
-            mColorFormats[i] = descriptor->colorFormats[static_cast<uint8_t>(i)];
+                mColorFormats[i] = format;
+            }
         }
         mDepthStencilFormat = descriptor->depthStencilFormat;
     }
@@ -40,8 +43,12 @@ namespace dawn::native {
             for (ColorAttachmentIndex i(uint8_t(0));
                  i < ColorAttachmentIndex(static_cast<uint8_t>(descriptor->fragment->targetCount));
                  ++i) {
+                wgpu::TextureFormat format =
+                    descriptor->fragment->targets[static_cast<uint8_t>(i)].format;
+                if (format != wgpu::TextureFormat::Undefined) {
                     mColorAttachmentsSet.set(i);
-                mColorFormats[i] = descriptor->fragment->targets[static_cast<uint8_t>(i)].format;
+                    mColorFormats[i] = format;
+                }
             }
         }
         if (descriptor->depthStencil != nullptr) {
@@ -55,6 +62,9 @@ namespace dawn::native {
              ++i) {
             TextureViewBase* attachment =
                 descriptor->colorAttachments[static_cast<uint8_t>(i)].view;
+            if (attachment == nullptr) {
+                continue;
+            }
             mColorAttachmentsSet.set(i);
             mColorFormats[i] = attachment->GetFormat().format;
             if (mSampleCount == 0) {

src/dawn/native/CommandEncoder.cpp

@@ -227,6 +227,9 @@ namespace dawn::native {
             uint32_t* sampleCount,
             UsageValidationMode usageValidationMode) {
             TextureViewBase* attachment = colorAttachment.view;
+            if (attachment == nullptr) {
+                return {};
+            }
             DAWN_TRY(device->ValidateObject(attachment));
             DAWN_TRY(ValidateCanUseAs(attachment->GetTexture(),
                                       wgpu::TextureUsage::RenderAttachment, usageValidationMode));
@@ -390,11 +393,15 @@ namespace dawn::native {
                 "Color attachment count (%u) exceeds the maximum number of color attachments (%u).",
                 descriptor->colorAttachmentCount, kMaxColorAttachments);
 
+            bool isAllColorAttachmentNull = true;
             for (uint32_t i = 0; i < descriptor->colorAttachmentCount; ++i) {
                 DAWN_TRY_CONTEXT(ValidateRenderPassColorAttachment(
                                      device, descriptor->colorAttachments[i], width, height,
                                      sampleCount, usageValidationMode),
                                  "validating colorAttachments[%u].", i);
+                if (descriptor->colorAttachments[i].view) {
+                    isAllColorAttachmentNull = false;
+                }
             }
 
             if (descriptor->depthStencilAttachment != nullptr) {
@@ -402,6 +409,10 @@ namespace dawn::native {
                                      device, descriptor->depthStencilAttachment, width, height,
                                      sampleCount, usageValidationMode),
                                  "validating depthStencilAttachment.");
+            } else {
+                DAWN_INVALID_IF(
+                    isAllColorAttachmentNull,
+                    "No color or depthStencil attachments specified. At least one is required.");
             }
 
             if (descriptor->occlusionQuerySet != nullptr) {

src/dawn/native/RenderBundleEncoder.cpp

@@ -66,13 +66,14 @@ namespace dawn::native {
             "Color formats count (%u) exceeds maximum number of color attachements (%u).",
             descriptor->colorFormatsCount, kMaxColorAttachments);
 
-        DAWN_INVALID_IF(descriptor->colorFormatsCount == 0 &&
+        bool allColorFormatsUndefined = true;
-                            descriptor->depthStencilFormat == wgpu::TextureFormat::Undefined,
-                        "No color or depth/stencil attachment formats specified.");
-
         for (uint32_t i = 0; i < descriptor->colorFormatsCount; ++i) {
-            DAWN_TRY_CONTEXT(ValidateColorAttachmentFormat(device, descriptor->colorFormats[i]),
+            wgpu::TextureFormat format = descriptor->colorFormats[i];
+            if (format != wgpu::TextureFormat::Undefined) {
+                DAWN_TRY_CONTEXT(ValidateColorAttachmentFormat(device, format),
                                  "validating colorFormats[%u]", i);
+                allColorFormatsUndefined = false;
+            }
         }
 
         if (descriptor->depthStencilFormat != wgpu::TextureFormat::Undefined) {
@@ -80,6 +81,10 @@ namespace dawn::native {
                                  device, descriptor->depthStencilFormat, descriptor->depthReadOnly,
                                  descriptor->stencilReadOnly),
                              "validating depthStencilFormat");
+        } else {
+            DAWN_INVALID_IF(
+                allColorFormatsUndefined,
+                "No color or depthStencil attachments specified. At least one is required.");
         }
 
         return {};

src/dawn/native/RenderPipeline.cpp

@@ -430,11 +430,22 @@ namespace dawn::native {
                 descriptor->module->GetEntryPoint(descriptor->entryPoint);
             for (ColorAttachmentIndex i(uint8_t(0));
                  i < ColorAttachmentIndex(static_cast<uint8_t>(descriptor->targetCount)); ++i) {
-                DAWN_TRY_CONTEXT(
+                const ColorTargetState* target = &descriptor->targets[static_cast<uint8_t>(i)];
-                    ValidateColorTargetState(device, &descriptor->targets[static_cast<uint8_t>(i)],
+                if (target->format != wgpu::TextureFormat::Undefined) {
-                                             fragmentMetadata.fragmentOutputsWritten[i],
+                    DAWN_TRY_CONTEXT(ValidateColorTargetState(
+                                         device, target, fragmentMetadata.fragmentOutputsWritten[i],
                                          fragmentMetadata.fragmentOutputVariables[i]),
                                      "validating targets[%u].", static_cast<uint8_t>(i));
+                } else {
+                    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);
+                }
             }
 
             return {};

src/dawn/native/d3d12/CommandBufferD3D12.cpp

@@ -1197,8 +1197,14 @@ namespace dawn::native::d3d12 {
                                               RenderPassBuilder* renderPassBuilder) {
         Device* device = ToBackend(GetDevice());
 
-        for (ColorAttachmentIndex i :
+        CPUDescriptorHeapAllocation nullRTVAllocation;
-             IterateBitSet(renderPass->attachmentState->GetColorAttachmentsMask())) {
+        D3D12_CPU_DESCRIPTOR_HANDLE nullRTV;
+
+        const auto& colorAttachmentsMaskBitSet =
+            renderPass->attachmentState->GetColorAttachmentsMask();
+        for (ColorAttachmentIndex i(uint8_t(0)); i < ColorAttachmentIndex(kMaxColorAttachments);
+             i++) {
+            if (colorAttachmentsMaskBitSet.test(i)) {
                 RenderPassColorAttachmentInfo& attachmentInfo = renderPass->colorAttachments[i];
                 TextureView* view = ToBackend(attachmentInfo.view.Get());
 
@@ -1209,12 +1215,13 @@ namespace dawn::native::d3d12 {
                     device->GetRenderTargetViewAllocator()->AllocateTransientCPUDescriptors());
 
                 const D3D12_RENDER_TARGET_VIEW_DESC viewDesc = view->GetRTVDescriptor();
-            const D3D12_CPU_DESCRIPTOR_HANDLE baseDescriptor = rtvAllocation.GetBaseDescriptor();
+                const D3D12_CPU_DESCRIPTOR_HANDLE baseDescriptor =
+                    rtvAllocation.GetBaseDescriptor();
 
                 device->GetD3D12Device()->CreateRenderTargetView(
                     ToBackend(view->GetTexture())->GetD3D12Resource(), &viewDesc, baseDescriptor);
 
-            renderPassBuilder->SetRenderTargetView(i, baseDescriptor);
+                renderPassBuilder->SetRenderTargetView(i, baseDescriptor, false);
 
                 // Set color load operation.
                 renderPassBuilder->SetRenderTargetBeginningAccess(
@@ -1222,7 +1229,8 @@ namespace dawn::native::d3d12 {
 
                 // Set color store operation.
                 if (attachmentInfo.resolveTarget != nullptr) {
-                TextureView* resolveDestinationView = ToBackend(attachmentInfo.resolveTarget.Get());
+                    TextureView* resolveDestinationView =
+                        ToBackend(attachmentInfo.resolveTarget.Get());
                     Texture* resolveDestinationTexture =
                         ToBackend(resolveDestinationView->GetTexture());
 
@@ -1230,11 +1238,28 @@ namespace dawn::native::d3d12 {
                         commandContext, D3D12_RESOURCE_STATE_RESOLVE_DEST,
                         resolveDestinationView->GetSubresourceRange());
 
-                renderPassBuilder->SetRenderTargetEndingAccessResolve(i, attachmentInfo.storeOp,
+                    renderPassBuilder->SetRenderTargetEndingAccessResolve(
-                                                                      view, resolveDestinationView);
+                        i, attachmentInfo.storeOp, view, resolveDestinationView);
                 } else {
                     renderPassBuilder->SetRenderTargetEndingAccess(i, attachmentInfo.storeOp);
                 }
+            } else {
+                if (!nullRTVAllocation.IsValid()) {
+                    DAWN_TRY_ASSIGN(
+                        nullRTVAllocation,
+                        device->GetRenderTargetViewAllocator()->AllocateTransientCPUDescriptors());
+                    nullRTV = nullRTVAllocation.GetBaseDescriptor();
+                    D3D12_RENDER_TARGET_VIEW_DESC nullRTVDesc;
+                    nullRTVDesc.Format = DXGI_FORMAT_R8G8B8A8_UNORM;
+                    nullRTVDesc.ViewDimension = D3D12_RTV_DIMENSION_TEXTURE2D;
+                    nullRTVDesc.Texture2D.MipSlice = 0;
+                    nullRTVDesc.Texture2D.PlaneSlice = 0;
+                    device->GetD3D12Device()->CreateRenderTargetView(nullptr, &nullRTVDesc,
+                                                                     nullRTV);
+                }
+
+                renderPassBuilder->SetRenderTargetView(i, nullRTV, true);
+            }
         }
 
         if (renderPass->attachmentState->HasDepthStencilAttachment()) {
@@ -1290,15 +1315,14 @@ namespace dawn::native::d3d12 {
 
         // Clear framebuffer attachments as needed.
         {
-            for (ColorAttachmentIndex i(uint8_t(0));
+            for (const auto& attachment :
-                 i < renderPassBuilder->GetColorAttachmentCount(); i++) {
+                 renderPassBuilder->GetRenderPassRenderTargetDescriptors()) {
                 // Load op - color
-                if (renderPassBuilder->GetRenderPassRenderTargetDescriptors()[i]
+                if (attachment.cpuDescriptor.ptr != 0 &&
-                        .BeginningAccess.Type == D3D12_RENDER_PASS_BEGINNING_ACCESS_TYPE_CLEAR) {
+                    attachment.BeginningAccess.Type ==
+                        D3D12_RENDER_PASS_BEGINNING_ACCESS_TYPE_CLEAR) {
                     commandList->ClearRenderTargetView(
-                        renderPassBuilder->GetRenderPassRenderTargetDescriptors()[i].cpuDescriptor,
+                        attachment.cpuDescriptor, attachment.BeginningAccess.Clear.ClearValue.Color,
-                        renderPassBuilder->GetRenderPassRenderTargetDescriptors()[i]
-                            .BeginningAccess.Clear.ClearValue.Color,
                         0, nullptr);
                 }
             }
@@ -1333,7 +1357,7 @@ namespace dawn::native::d3d12 {
         }
 
         commandList->OMSetRenderTargets(
-            static_cast<uint8_t>(renderPassBuilder->GetColorAttachmentCount()),
+            static_cast<uint8_t>(renderPassBuilder->GetHighestColorAttachmentIndexPlusOne()),
             renderPassBuilder->GetRenderTargetViews(), FALSE,
             renderPassBuilder->HasDepth()
                 ? &renderPassBuilder->GetRenderPassDepthStencilDescriptor()->cpuDescriptor
@@ -1358,7 +1382,7 @@ namespace dawn::native::d3d12 {
         // beginning and ending access operations.
         if (useRenderPass) {
             commandContext->GetCommandList4()->BeginRenderPass(
-                static_cast<uint8_t>(renderPassBuilder.GetColorAttachmentCount()),
+                static_cast<uint8_t>(renderPassBuilder.GetHighestColorAttachmentIndexPlusOne()),
                 renderPassBuilder.GetRenderPassRenderTargetDescriptors().data(),
                 renderPassBuilder.HasDepth()
                     ? renderPassBuilder.GetRenderPassDepthStencilDescriptor()

src/dawn/native/d3d12/RenderPassBuilderD3D12.cpp

@@ -116,19 +116,24 @@ namespace dawn::native::d3d12 {
     }
 
     void RenderPassBuilder::SetRenderTargetView(ColorAttachmentIndex attachmentIndex,
-                                                D3D12_CPU_DESCRIPTOR_HANDLE baseDescriptor) {
+                                                D3D12_CPU_DESCRIPTOR_HANDLE baseDescriptor,
-        ASSERT(mColorAttachmentCount < kMaxColorAttachmentsTyped);
+                                                bool isNullRTV) {
         mRenderTargetViews[attachmentIndex] = baseDescriptor;
         mRenderPassRenderTargetDescriptors[attachmentIndex].cpuDescriptor = baseDescriptor;
-        mColorAttachmentCount++;
+        if (!isNullRTV) {
+            mHighestColorAttachmentIndexPlusOne =
+                std::max(mHighestColorAttachmentIndexPlusOne,
+                         ColorAttachmentIndex{
+                             static_cast<uint8_t>(static_cast<uint8_t>(attachmentIndex) + 1u)});
+        }
     }
 
     void RenderPassBuilder::SetDepthStencilView(D3D12_CPU_DESCRIPTOR_HANDLE baseDescriptor) {
         mRenderPassDepthStencilDesc.cpuDescriptor = baseDescriptor;
     }
 
-    ColorAttachmentIndex RenderPassBuilder::GetColorAttachmentCount() const {
+    ColorAttachmentIndex RenderPassBuilder::GetHighestColorAttachmentIndexPlusOne() const {
-        return mColorAttachmentCount;
+        return mHighestColorAttachmentIndexPlusOne;
     }
 
     bool RenderPassBuilder::HasDepth() const {
@@ -137,7 +142,7 @@ namespace dawn::native::d3d12 {
 
     ityp::span<ColorAttachmentIndex, const D3D12_RENDER_PASS_RENDER_TARGET_DESC>
     RenderPassBuilder::GetRenderPassRenderTargetDescriptors() const {
-        return {mRenderPassRenderTargetDescriptors.data(), mColorAttachmentCount};
+        return {mRenderPassRenderTargetDescriptors.data(), mHighestColorAttachmentIndexPlusOne};
     }
 
     const D3D12_RENDER_PASS_DEPTH_STENCIL_DESC*

src/dawn/native/d3d12/RenderPassBuilderD3D12.h

@@ -37,7 +37,9 @@ namespace dawn::native::d3d12 {
       public:
         RenderPassBuilder(bool hasUAV);
 
-        ColorAttachmentIndex GetColorAttachmentCount() const;
+        // Returns the highest color attachment index + 1. If there is no color attachment, returns
+        // 0. Range: [0, kMaxColorAttachments + 1)
+        ColorAttachmentIndex GetHighestColorAttachmentIndexPlusOne() const;
 
         // Returns descriptors that are fed directly to BeginRenderPass, or are used as parameter
         // storage if D3D12 render pass API is unavailable.
@@ -75,11 +77,12 @@ namespace dawn::native::d3d12 {
         void SetStencilNoAccess();
 
         void SetRenderTargetView(ColorAttachmentIndex attachmentIndex,
-                                 D3D12_CPU_DESCRIPTOR_HANDLE baseDescriptor);
+                                 D3D12_CPU_DESCRIPTOR_HANDLE baseDescriptor,
+                                 bool isNullRTV);
         void SetDepthStencilView(D3D12_CPU_DESCRIPTOR_HANDLE baseDescriptor);
 
       private:
-        ColorAttachmentIndex mColorAttachmentCount{uint8_t(0)};
+        ColorAttachmentIndex mHighestColorAttachmentIndexPlusOne{uint8_t(0)};
         bool mHasDepth = false;
         D3D12_RENDER_PASS_FLAGS mRenderPassFlags = D3D12_RENDER_PASS_FLAG_NONE;
         D3D12_RENDER_PASS_DEPTH_STENCIL_DESC mRenderPassDepthStencilDesc;

src/dawn/native/d3d12/RenderPipelineD3D12.cpp

@@ -393,13 +393,24 @@ namespace dawn::native::d3d12 {
             descriptorD3D12.DSVFormat = D3D12TextureFormat(GetDepthStencilFormat());
         }
 
+        static_assert(kMaxColorAttachments == 8);
+        for (uint8_t i = 0; i < kMaxColorAttachments; i++) {
+            descriptorD3D12.RTVFormats[i] = DXGI_FORMAT_UNKNOWN;
+            descriptorD3D12.BlendState.RenderTarget[i].BlendEnable = false;
+            descriptorD3D12.BlendState.RenderTarget[i].RenderTargetWriteMask = 0;
+            descriptorD3D12.BlendState.RenderTarget[i].LogicOpEnable = false;
+            descriptorD3D12.BlendState.RenderTarget[i].LogicOp = D3D12_LOGIC_OP_NOOP;
+        }
+        ColorAttachmentIndex highestColorAttachmentIndexPlusOne =
+            GetHighestBitIndexPlusOne(GetColorAttachmentsMask());
         for (ColorAttachmentIndex i : IterateBitSet(GetColorAttachmentsMask())) {
             descriptorD3D12.RTVFormats[static_cast<uint8_t>(i)] =
                 D3D12TextureFormat(GetColorAttachmentFormat(i));
             descriptorD3D12.BlendState.RenderTarget[static_cast<uint8_t>(i)] =
                 ComputeColorDesc(GetColorTargetState(i));
         }
-        descriptorD3D12.NumRenderTargets = static_cast<uint32_t>(GetColorAttachmentsMask().count());
+        ASSERT(highestColorAttachmentIndexPlusOne <= kMaxColorAttachmentsTyped);
+        descriptorD3D12.NumRenderTargets = static_cast<uint8_t>(highestColorAttachmentIndexPlusOne);
 
         descriptorD3D12.BlendState.AlphaToCoverageEnable = IsAlphaToCoverageEnabled();
         descriptorD3D12.BlendState.IndependentBlendEnable = TRUE;

src/dawn/native/vulkan/CommandBufferVk.cpp

@@ -246,6 +246,9 @@ namespace dawn::native::vulkan {
                      IterateBitSet(renderPass->attachmentState->GetColorAttachmentsMask())) {
                     auto& attachmentInfo = renderPass->colorAttachments[i];
                     TextureView* view = ToBackend(attachmentInfo.view.Get());
+                    if (view == nullptr) {
+                        continue;
+                    }
 
                     attachments[attachmentCount] = view->GetHandle();
 

src/dawn/native/vulkan/RenderPassCache.cpp

@@ -115,11 +115,23 @@ namespace dawn::native::vulkan {
     ResultOrError<VkRenderPass> RenderPassCache::CreateRenderPassForQuery(
         const RenderPassCacheQuery& query) const {
         // The Vulkan subpasses want to know the layout of the attachments with VkAttachmentRef.
-        // Precompute them as they must be pointer-chained in VkSubpassDescription
+        // Precompute them as they must be pointer-chained in VkSubpassDescription.
-        std::array<VkAttachmentReference, kMaxColorAttachments> colorAttachmentRefs;
+        // Note that both colorAttachmentRefs and resolveAttachmentRefs can be sparse with holes
-        std::array<VkAttachmentReference, kMaxColorAttachments> resolveAttachmentRefs;
+        // filled with VK_ATTACHMENT_UNUSED.
+        ityp::array<ColorAttachmentIndex, VkAttachmentReference, kMaxColorAttachments>
+            colorAttachmentRefs;
+        ityp::array<ColorAttachmentIndex, VkAttachmentReference, kMaxColorAttachments>
+            resolveAttachmentRefs;
         VkAttachmentReference depthStencilAttachmentRef;
 
+        for (ColorAttachmentIndex i(uint8_t(0)); i < kMaxColorAttachmentsTyped; i++) {
+            colorAttachmentRefs[i].attachment = VK_ATTACHMENT_UNUSED;
+            resolveAttachmentRefs[i].attachment = VK_ATTACHMENT_UNUSED;
+            // The Khronos Vulkan validation layer will complain if not set
+            colorAttachmentRefs[i].layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
+            resolveAttachmentRefs[i].layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
+        }
+
         // Contains the attachment description that will be chained in the create info
         // The order of all attachments in attachmentDescs is "color-depthstencil-resolve".
         constexpr uint8_t kMaxAttachmentCount = kMaxColorAttachments * 2 + 1;
@@ -127,12 +139,13 @@ namespace dawn::native::vulkan {
 
         VkSampleCountFlagBits vkSampleCount = VulkanSampleCount(query.sampleCount);
 
-        uint32_t colorAttachmentIndex = 0;
+        uint32_t attachmentCount = 0;
+        ColorAttachmentIndex highestColorAttachmentIndexPlusOne(static_cast<uint8_t>(0));
         for (ColorAttachmentIndex i : IterateBitSet(query.colorMask)) {
-            auto& attachmentRef = colorAttachmentRefs[colorAttachmentIndex];
+            auto& attachmentRef = colorAttachmentRefs[i];
-            auto& attachmentDesc = attachmentDescs[colorAttachmentIndex];
+            auto& attachmentDesc = attachmentDescs[attachmentCount];
 
-            attachmentRef.attachment = colorAttachmentIndex;
+            attachmentRef.attachment = attachmentCount;
             attachmentRef.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
 
             attachmentDesc.flags = 0;
@@ -143,10 +156,11 @@ namespace dawn::native::vulkan {
             attachmentDesc.initialLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
             attachmentDesc.finalLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
 
-            ++colorAttachmentIndex;
+            attachmentCount++;
+            highestColorAttachmentIndexPlusOne =
+                ColorAttachmentIndex(static_cast<uint8_t>(static_cast<uint8_t>(i) + 1u));
         }
 
-        uint32_t attachmentCount = colorAttachmentIndex;
         VkAttachmentReference* depthStencilAttachment = nullptr;
         if (query.hasDepthStencil) {
             auto& attachmentDesc = attachmentDescs[attachmentCount];
@@ -172,12 +186,11 @@ namespace dawn::native::vulkan {
             attachmentDesc.initialLayout = depthStencilAttachmentRef.layout;
             attachmentDesc.finalLayout = depthStencilAttachmentRef.layout;
 
-            ++attachmentCount;
+            attachmentCount++;
         }
 
-        uint32_t resolveAttachmentIndex = 0;
         for (ColorAttachmentIndex i : IterateBitSet(query.resolveTargetMask)) {
-            auto& attachmentRef = resolveAttachmentRefs[resolveAttachmentIndex];
+            auto& attachmentRef = resolveAttachmentRefs[i];
             auto& attachmentDesc = attachmentDescs[attachmentCount];
 
             attachmentRef.attachment = attachmentCount;
@@ -191,31 +204,18 @@ namespace dawn::native::vulkan {
             attachmentDesc.initialLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
             attachmentDesc.finalLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
 
-            ++attachmentCount;
+            attachmentCount++;
-            ++resolveAttachmentIndex;
         }
 
-        // All color attachments without a corresponding resolve attachment must be set to
-        // VK_ATTACHMENT_UNUSED
-        for (; resolveAttachmentIndex < colorAttachmentIndex; resolveAttachmentIndex++) {
-            auto& attachmentRef = resolveAttachmentRefs[resolveAttachmentIndex];
-            attachmentRef.attachment = VK_ATTACHMENT_UNUSED;
-            // The Khronos Vulkan validation layer will complain if not set
-            attachmentRef.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
-        }
-
-        VkAttachmentReference* resolveTargetAttachmentRefs =
-            query.resolveTargetMask.any() ? resolveAttachmentRefs.data() : nullptr;
-
         // Create the VkSubpassDescription that will be chained in the VkRenderPassCreateInfo
         VkSubpassDescription subpassDesc;
         subpassDesc.flags = 0;
         subpassDesc.pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS;
         subpassDesc.inputAttachmentCount = 0;
         subpassDesc.pInputAttachments = nullptr;
-        subpassDesc.colorAttachmentCount = colorAttachmentIndex;
+        subpassDesc.colorAttachmentCount = static_cast<uint8_t>(highestColorAttachmentIndexPlusOne);
         subpassDesc.pColorAttachments = colorAttachmentRefs.data();
-        subpassDesc.pResolveAttachments = resolveTargetAttachmentRefs;
+        subpassDesc.pResolveAttachments = resolveAttachmentRefs.data();
         subpassDesc.pDepthStencilAttachment = depthStencilAttachment;
         subpassDesc.preserveAttachmentCount = 0;
         subpassDesc.pPreserveAttachments = nullptr;

src/dawn/native/vulkan/RenderPipelineVk.cpp

@@ -457,8 +457,21 @@ namespace dawn::native::vulkan {
         if (GetStageMask() & wgpu::ShaderStage::Fragment) {
             // Initialize the "blend state info" that will be chained in the "create info" from the
             // data pre-computed in the ColorState
+            for (auto& blend : colorBlendAttachments) {
+                blend.blendEnable = VK_FALSE;
+                blend.srcColorBlendFactor = VK_BLEND_FACTOR_ONE;
+                blend.dstColorBlendFactor = VK_BLEND_FACTOR_ZERO;
+                blend.colorBlendOp = VK_BLEND_OP_ADD;
+                blend.srcAlphaBlendFactor = VK_BLEND_FACTOR_ONE;
+                blend.dstAlphaBlendFactor = VK_BLEND_FACTOR_ZERO;
+                blend.alphaBlendOp = VK_BLEND_OP_ADD;
+                blend.colorWriteMask = 0;
+            }
+
             const auto& fragmentOutputsWritten =
                 GetStage(SingleShaderStage::Fragment).metadata->fragmentOutputsWritten;
+            ColorAttachmentIndex highestColorAttachmentIndexPlusOne =
+                GetHighestBitIndexPlusOne(GetColorAttachmentsMask());
             for (ColorAttachmentIndex i : IterateBitSet(GetColorAttachmentsMask())) {
                 const ColorTargetState* target = GetColorTargetState(i);
                 colorBlendAttachments[i] = ComputeColorDesc(target, fragmentOutputsWritten[i]);
@@ -470,7 +483,7 @@ namespace dawn::native::vulkan {
             // LogicOp isn't supported so we disable it.
             colorBlend.logicOpEnable = VK_FALSE;
             colorBlend.logicOp = VK_LOGIC_OP_CLEAR;
-            colorBlend.attachmentCount = static_cast<uint32_t>(GetColorAttachmentsMask().count());
+            colorBlend.attachmentCount = static_cast<uint8_t>(highestColorAttachmentIndexPlusOne);
             colorBlend.pAttachments = colorBlendAttachments.data();
             // The blend constant is always dynamic so we fill in a dummy value
             colorBlend.blendConstants[0] = 0.0f;

src/dawn/tests/end2end/ColorStateTests.cpp

@@ -1095,6 +1095,69 @@ TEST_P(ColorStateTest, ColorWriteMaskDoesNotAffectRenderPassLoadOpClear) {
     EXPECT_PIXEL_RGBA8_EQ(expected, renderPass.color, kRTSize / 2, kRTSize / 2);
 }
 
+TEST_P(ColorStateTest, SparseAttachmentsDifferentColorMask) {
+    DAWN_TEST_UNSUPPORTED_IF(HasToggleEnabled("disable_indexed_draw_buffers"));
+
+    wgpu::ShaderModule fsModule = utils::CreateShaderModule(device, R"(
+        struct Outputs {
+            @location(1) o1 : vec4<f32>;
+            @location(3) o3 : vec4<f32>;
+        }
+
+        @stage(fragment) fn main() -> Outputs {
+            return Outputs(vec4<f32>(1.0), vec4<f32>(0.0, 1.0, 1.0, 1.0));
+        }
+    )");
+
+    utils::ComboRenderPipelineDescriptor pipelineDesc;
+    pipelineDesc.vertex.module = vsModule;
+    pipelineDesc.cFragment.module = fsModule;
+    pipelineDesc.cFragment.targetCount = 4;
+    pipelineDesc.cTargets[0].format = wgpu::TextureFormat::Undefined;
+    pipelineDesc.cTargets[0].writeMask = wgpu::ColorWriteMask::None;
+    pipelineDesc.cTargets[1].format = wgpu::TextureFormat::RGBA8Unorm;
+    pipelineDesc.cTargets[2].format = wgpu::TextureFormat::Undefined;
+    pipelineDesc.cTargets[2].writeMask = wgpu::ColorWriteMask::None;
+    pipelineDesc.cTargets[3].format = wgpu::TextureFormat::RGBA8Unorm;
+    pipelineDesc.cTargets[3].writeMask = wgpu::ColorWriteMask::Green | wgpu::ColorWriteMask::Alpha;
+    wgpu::RenderPipeline pipeline = device.CreateRenderPipeline(&pipelineDesc);
+
+    wgpu::TextureDescriptor texDesc;
+    texDesc.dimension = wgpu::TextureDimension::e2D;
+    texDesc.format = wgpu::TextureFormat::RGBA8Unorm;
+    texDesc.usage = wgpu::TextureUsage::RenderAttachment | wgpu::TextureUsage::CopySrc;
+    texDesc.size = {1, 1};
+    wgpu::Texture attachment1 = device.CreateTexture(&texDesc);
+    wgpu::Texture attachment3 = device.CreateTexture(&texDesc);
+
+    wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+    {
+        wgpu::RenderPassColorAttachment colorAttachments[4]{};
+        colorAttachments[0].view = nullptr;
+        colorAttachments[1].view = attachment1.CreateView();
+        colorAttachments[1].loadOp = wgpu::LoadOp::Load;
+        colorAttachments[1].storeOp = wgpu::StoreOp::Store;
+        colorAttachments[2].view = nullptr;
+        colorAttachments[3].view = attachment3.CreateView();
+        colorAttachments[3].loadOp = wgpu::LoadOp::Load;
+        colorAttachments[3].storeOp = wgpu::StoreOp::Store;
+
+        wgpu::RenderPassDescriptor rpDesc;
+        rpDesc.colorAttachmentCount = 4;
+        rpDesc.colorAttachments = colorAttachments;
+
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&rpDesc);
+        pass.SetPipeline(pipeline);
+        pass.Draw(3);
+        pass.End();
+    }
+    wgpu::CommandBuffer commands = encoder.Finish();
+    queue.Submit(1, &commands);
+
+    EXPECT_PIXEL_RGBA8_EQ(RGBA8::kWhite, attachment1, 0, 0);
+    EXPECT_PIXEL_RGBA8_EQ(RGBA8::kGreen, attachment3, 0, 0);
+}
+
 DAWN_INSTANTIATE_TEST(ColorStateTest,
                       D3D12Backend(),
                       MetalBackend(),

src/dawn/tests/end2end/RenderPassTests.cpp

@@ -159,7 +159,7 @@ TEST_P(RenderPassTest, NoCorrespondingFragmentShaderOutputs) {
     wgpu::CommandBuffer commands = encoder.Finish();
     queue.Submit(1, &commands);
 
-    EXPECT_PIXEL_RGBA8_EQ(RGBA8::kBlue, renderTarget, 2, kRTSize - 1);
+    EXPECT_PIXEL_RGBA8_EQ(RGBA8::kBlue, renderTarget, 1, kRTSize - 1);
     EXPECT_PIXEL_RGBA8_EQ(RGBA8::kRed, renderTarget, kRTSize - 1, 1);
 }
 

src/dawn/tests/unittests/ITypBitsetTests.cpp

@@ -23,6 +23,7 @@ class ITypBitsetTest : public testing::Test {
   protected:
     using Key = TypedInteger<struct KeyT, size_t>;
     using Bitset = ityp::bitset<Key, 9>;
+    using Bitset40 = ityp::bitset<Key, 40>;
 
     // Test that the expected bitset methods can be constexpr
     struct ConstexprTest {
@@ -176,3 +177,33 @@ TEST_F(ITypBitsetTest, Xor) {
     bits ^= Bitset{1 << 1 | 1 << 6};
     ExpectBits(bits, {2, 6, 7});
 }
+
+// Testing the GetHighestBitIndexPlusOne function
+TEST_F(ITypBitsetTest, GetHighestBitIndexPlusOne) {
+    // <= 32 bit
+    EXPECT_EQ(0u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset(0b00))));
+    EXPECT_EQ(1u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset(0b01))));
+    EXPECT_EQ(2u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset(0b10))));
+    EXPECT_EQ(2u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset(0b11))));
+
+    EXPECT_EQ(3u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset{1 << 2})));
+    EXPECT_EQ(9u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset{1 << 8})));
+    EXPECT_EQ(9u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset{1 << 8 | 1 << 2})));
+
+    // > 32 bit
+    EXPECT_EQ(0u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0b00))));
+    EXPECT_EQ(1u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0b01))));
+    EXPECT_EQ(2u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0b10))));
+    EXPECT_EQ(2u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0b11))));
+
+    EXPECT_EQ(5u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0x10))));
+    EXPECT_EQ(5u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0x1F))));
+    EXPECT_EQ(16u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0xF000))));
+    EXPECT_EQ(16u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0xFFFF))));
+    EXPECT_EQ(32u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0xF0000000))));
+    EXPECT_EQ(32u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0xFFFFFFFF))));
+    EXPECT_EQ(36u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0xF00000000))));
+    EXPECT_EQ(36u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0xFFFFFFFFF))));
+    EXPECT_EQ(40u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0xF000000000))));
+    EXPECT_EQ(40u, static_cast<size_t>(GetHighestBitIndexPlusOne(Bitset40(0xFFFFFFFFFF))));
+}

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

@@ -635,12 +635,35 @@ TEST_F(RenderBundleValidationTest, ColorFormatsCountOutOfBounds) {
     }
 }
 
-// Test that render bundle color formats cannot be set to undefined.
+// Test that render bundle sparse color formats.
-TEST_F(RenderBundleValidationTest, ColorFormatUndefined) {
+TEST_F(RenderBundleValidationTest, SparseColorFormats) {
+    // Sparse color formats is valid.
+    {
+        utils::ComboRenderBundleEncoderDescriptor desc = {};
+        desc.colorFormatsCount = 2;
+        desc.cColorFormats[0] = wgpu::TextureFormat::Undefined;
+        desc.cColorFormats[1] = wgpu::TextureFormat::RGBA8Unorm;
+        device.CreateRenderBundleEncoder(&desc);
+    }
+
+    // When all color formats are undefined, depth stencil format must not be undefined.
+    {
         utils::ComboRenderBundleEncoderDescriptor desc = {};
         desc.colorFormatsCount = 1;
         desc.cColorFormats[0] = wgpu::TextureFormat::Undefined;
-    ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
+        desc.depthStencilFormat = wgpu::TextureFormat::Undefined;
+        ASSERT_DEVICE_ERROR(
+            device.CreateRenderBundleEncoder(&desc),
+            testing::HasSubstr(
+                "No color or depthStencil attachments specified. At least one is required."));
+    }
+    {
+        utils::ComboRenderBundleEncoderDescriptor desc = {};
+        desc.colorFormatsCount = 1;
+        desc.cColorFormats[0] = wgpu::TextureFormat::Undefined;
+        desc.depthStencilFormat = wgpu::TextureFormat::Depth24PlusStencil8;
+        device.CreateRenderBundleEncoder(&desc);
+    }
 }
 
 // Test that the render bundle depth stencil format cannot be set to undefined.

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

@@ -130,6 +130,62 @@ namespace {
         }
     }
 
+    // Test sparse color attachment validations
+    TEST_F(RenderPassDescriptorValidationTest, SparseColorAttachment) {
+        // Having sparse color attachment is valid.
+        {
+            std::array<wgpu::RenderPassColorAttachment, 2> colorAttachments;
+            colorAttachments[0].view = nullptr;
+
+            colorAttachments[1].view =
+                Create2DAttachment(device, 1, 1, wgpu::TextureFormat::RGBA8Unorm);
+            colorAttachments[1].loadOp = wgpu::LoadOp::Load;
+            colorAttachments[1].storeOp = wgpu::StoreOp::Store;
+
+            wgpu::RenderPassDescriptor renderPass;
+            renderPass.colorAttachmentCount = colorAttachments.size();
+            renderPass.colorAttachments = colorAttachments.data();
+            renderPass.depthStencilAttachment = nullptr;
+            AssertBeginRenderPassSuccess(&renderPass);
+        }
+
+        // When all color attachments are null
+        {
+            std::array<wgpu::RenderPassColorAttachment, 2> colorAttachments;
+            colorAttachments[0].view = nullptr;
+            colorAttachments[1].view = nullptr;
+
+            // Control case: depth stencil attachment is not null is valid.
+            {
+                wgpu::TextureView depthStencilView =
+                    Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24PlusStencil8);
+                wgpu::RenderPassDepthStencilAttachment depthStencilAttachment;
+                depthStencilAttachment.view = depthStencilView;
+                depthStencilAttachment.depthClearValue = 1.0f;
+                depthStencilAttachment.stencilClearValue = 0;
+                depthStencilAttachment.depthLoadOp = wgpu::LoadOp::Clear;
+                depthStencilAttachment.depthStoreOp = wgpu::StoreOp::Store;
+                depthStencilAttachment.stencilLoadOp = wgpu::LoadOp::Clear;
+                depthStencilAttachment.stencilStoreOp = wgpu::StoreOp::Store;
+
+                wgpu::RenderPassDescriptor renderPass;
+                renderPass.colorAttachmentCount = colorAttachments.size();
+                renderPass.colorAttachments = colorAttachments.data();
+                renderPass.depthStencilAttachment = &depthStencilAttachment;
+                AssertBeginRenderPassSuccess(&renderPass);
+            }
+
+            // Error case: depth stencil attachment being null is invalid.
+            {
+                wgpu::RenderPassDescriptor renderPass;
+                renderPass.colorAttachmentCount = colorAttachments.size();
+                renderPass.colorAttachments = colorAttachments.data();
+                renderPass.depthStencilAttachment = nullptr;
+                AssertBeginRenderPassError(&renderPass);
+            }
+        }
+    }
+
     // Check that the render pass color attachment must have the RenderAttachment usage.
     TEST_F(RenderPassDescriptorValidationTest, ColorAttachmentInvalidUsage) {
         // Control case: using a texture with RenderAttachment is valid.

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

@@ -193,6 +193,48 @@ TEST_F(RenderPipelineValidationTest, ColorTargetStateRequired) {
     }
 }
 
+// Tests that target blend and writeMasks must not be set if the format is undefined.
+TEST_F(RenderPipelineValidationTest, UndefinedColorStateFormatWithBlendOrWriteMask) {
+    {
+        // Control case: Valid undefined format target.
+        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].writeMask = wgpu::ColorWriteMask::None;
+
+        device.CreateRenderPipeline(&descriptor);
+    }
+    {
+        // Error case: undefined format target with blend state set.
+        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 = &descriptor.cBlends[0];
+        descriptor.cTargets[0].writeMask = wgpu::ColorWriteMask::None;
+
+        ASSERT_DEVICE_ERROR(
+            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 the color formats must be renderable.
 TEST_F(RenderPipelineValidationTest, NonRenderableFormat) {
     {