Use TextureFormat::None for RenderBundleEncoder and AttachmentState

Bug: dawn:214
Change-Id: Ia9432582b0a5627c00d46ebcfa63ebf1ba246651
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10520
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

dawn.json

@@ -857,7 +857,7 @@
         "members": [
             {"name": "color formats count", "type": "uint32_t"},
             {"name": "color formats", "type": "texture format", "annotation": "const*", "length": "color formats count"},
-            {"name": "depth stencil format", "type": "texture format", "annotation": "const*", "optional": true},
+            {"name": "depth stencil format", "type": "texture format", "default": "none"},
             {"name": "sample count", "type": "uint32_t", "default": "1"}
         ]
     },

src/dawn_native/AttachmentState.cpp

@@ -23,32 +23,27 @@ namespace dawn_native {
 
     AttachmentStateBlueprint::AttachmentStateBlueprint(
         const RenderBundleEncoderDescriptor* descriptor)
-        : mHasDepthStencilAttachment(descriptor->depthStencilFormat != nullptr),
+        : mSampleCount(descriptor->sampleCount) {
-          mSampleCount(descriptor->sampleCount) {
         for (uint32_t i = 0; i < descriptor->colorFormatsCount; ++i) {
             mColorAttachmentsSet.set(i);
             mColorFormats[i] = descriptor->colorFormats[i];
         }
-        if (mHasDepthStencilAttachment) {
+        mDepthStencilFormat = descriptor->depthStencilFormat;
-            mDepthStencilFormat = *descriptor->depthStencilFormat;
-        }
     }
 
     AttachmentStateBlueprint::AttachmentStateBlueprint(const RenderPipelineDescriptor* descriptor)
-        : mHasDepthStencilAttachment(descriptor->depthStencilState != nullptr),
+        : mSampleCount(descriptor->sampleCount) {
-          mSampleCount(descriptor->sampleCount) {
         for (uint32_t i = 0; i < descriptor->colorStateCount; ++i) {
             ASSERT(descriptor->colorStates[i] != nullptr);
             mColorAttachmentsSet.set(i);
             mColorFormats[i] = descriptor->colorStates[i]->format;
         }
-        if (mHasDepthStencilAttachment) {
+        if (descriptor->depthStencilState != nullptr) {
             mDepthStencilFormat = descriptor->depthStencilState->format;
         }
     }
 
-    AttachmentStateBlueprint::AttachmentStateBlueprint(const RenderPassDescriptor* descriptor)
+    AttachmentStateBlueprint::AttachmentStateBlueprint(const RenderPassDescriptor* descriptor) {
-        : mHasDepthStencilAttachment(descriptor->depthStencilAttachment != nullptr) {
         for (uint32_t i = 0; i < descriptor->colorAttachmentCount; ++i) {
             TextureViewBase* attachment = descriptor->colorAttachments[i]->attachment;
             mColorAttachmentsSet.set(i);
@@ -59,7 +54,7 @@ namespace dawn_native {
                 ASSERT(mSampleCount == attachment->GetTexture()->GetSampleCount());
             }
         }
-        if (mHasDepthStencilAttachment) {
+        if (descriptor->depthStencilAttachment != nullptr) {
             TextureViewBase* attachment = descriptor->depthStencilAttachment->attachment;
             mDepthStencilFormat = attachment->GetFormat().format;
             if (mSampleCount == 0) {
@@ -84,10 +79,8 @@ namespace dawn_native {
             HashCombine(&hash, attachmentState->mColorFormats[i]);
         }
 
-        // Hash depth stencil attachments
+        // Hash depth stencil attachment
-        if (attachmentState->mHasDepthStencilAttachment) {
+        HashCombine(&hash, attachmentState->mDepthStencilFormat);
-            HashCombine(&hash, attachmentState->mDepthStencilFormat);
-        }
 
         // Hash sample count
         HashCombine(&hash, attachmentState->mSampleCount);
@@ -99,8 +92,7 @@ namespace dawn_native {
         const AttachmentStateBlueprint* a,
         const AttachmentStateBlueprint* b) const {
         // Check set attachments
-        if (a->mColorAttachmentsSet != b->mColorAttachmentsSet ||
+        if (a->mColorAttachmentsSet != b->mColorAttachmentsSet) {
-            a->mHasDepthStencilAttachment != b->mHasDepthStencilAttachment) {
             return false;
         }
 
@@ -112,10 +104,8 @@ namespace dawn_native {
         }
 
         // Check depth stencil format
-        if (a->mHasDepthStencilAttachment) {
+        if (a->mDepthStencilFormat != b->mDepthStencilFormat) {
-            if (a->mDepthStencilFormat != b->mDepthStencilFormat) {
+            return false;
-                return false;
-            }
         }
 
         // Check sample count
@@ -144,11 +134,11 @@ namespace dawn_native {
     }
 
     bool AttachmentState::HasDepthStencilAttachment() const {
-        return mHasDepthStencilAttachment;
+        return mDepthStencilFormat != dawn::TextureFormat::None;
     }
 
     dawn::TextureFormat AttachmentState::GetDepthStencilFormat() const {
-        ASSERT(mHasDepthStencilAttachment);
+        ASSERT(HasDepthStencilAttachment());
         return mDepthStencilFormat;
     }
 

src/dawn_native/AttachmentState.h

@@ -51,8 +51,8 @@ namespace dawn_native {
       protected:
         std::bitset<kMaxColorAttachments> mColorAttachmentsSet;
         std::array<dawn::TextureFormat, kMaxColorAttachments> mColorFormats;
-        bool mHasDepthStencilAttachment = false;
+        // Default (texture format None) indicates there is no depth stencil attachment.
-        dawn::TextureFormat mDepthStencilFormat;
+        dawn::TextureFormat mDepthStencilFormat = dawn::TextureFormat::None;
         uint32_t mSampleCount = 0;
     };
 

src/dawn_native/RenderBundleEncoder.cpp

@@ -59,7 +59,8 @@ namespace dawn_native {
             return DAWN_VALIDATION_ERROR("Color formats count exceeds maximum");
         }
 
-        if (descriptor->colorFormatsCount == 0 && !descriptor->depthStencilFormat) {
+        if (descriptor->colorFormatsCount == 0 &&
+            descriptor->depthStencilFormat == dawn::TextureFormat::None) {
             return DAWN_VALIDATION_ERROR("Should have at least one attachment format");
         }
 
@@ -67,8 +68,8 @@ namespace dawn_native {
             DAWN_TRY(ValidateColorAttachmentFormat(device, descriptor->colorFormats[i]));
         }
 
-        if (descriptor->depthStencilFormat != nullptr) {
+        if (descriptor->depthStencilFormat != dawn::TextureFormat::None) {
-            DAWN_TRY(ValidateDepthStencilAttachmentFormat(device, *descriptor->depthStencilFormat));
+            DAWN_TRY(ValidateDepthStencilAttachmentFormat(device, descriptor->depthStencilFormat));
         }
 
         return {};

src/tests/unittests/validation/RenderBundleValidationTests.cpp

@@ -580,8 +580,7 @@ TEST_F(RenderBundleValidationTest, RequiresAtLeastOneTextureFormat) {
     // Test success with a depth stencil format.
     {
         utils::ComboRenderBundleEncoderDescriptor desc = {};
-        desc.cDepthStencilFormat = dawn::TextureFormat::Depth24PlusStencil8;
+        desc.depthStencilFormat = dawn::TextureFormat::Depth24PlusStencil8;
-        desc.depthStencilFormat = &desc.cDepthStencilFormat;
         device.CreateRenderBundleEncoder(&desc);
     }
 }
@@ -595,8 +594,7 @@ TEST_F(RenderBundleValidationTest, ColorFormatNone) {
 
 TEST_F(RenderBundleValidationTest, DepthStencilFormatNone) {
     utils::ComboRenderBundleEncoderDescriptor desc = {};
-    const dawn::TextureFormat kFormatNone = dawn::TextureFormat::None;
+    desc.depthStencilFormat = dawn::TextureFormat::None;
-    desc.depthStencilFormat = &kFormatNone;
     ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
 }
 
@@ -750,8 +748,7 @@ TEST_F(RenderBundleValidationTest, PipelineDepthStencilFormatMismatch) {
     utils::ComboRenderBundleEncoderDescriptor renderBundleDesc = {};
     renderBundleDesc.colorFormatsCount = 1;
     renderBundleDesc.cColorFormats[0] = dawn::TextureFormat::RGBA8Unorm;
-    renderBundleDesc.cDepthStencilFormat = dawn::TextureFormat::Depth24PlusStencil8;
+    renderBundleDesc.depthStencilFormat = dawn::TextureFormat::Depth24PlusStencil8;
-    renderBundleDesc.depthStencilFormat = &renderBundleDesc.cDepthStencilFormat;
 
     utils::ComboRenderPipelineDescriptor renderPipelineDesc(device);
     InitializeRenderPipelineDescriptor(&renderPipelineDesc);
@@ -905,8 +902,7 @@ TEST_F(RenderBundleValidationTest, RenderPassDepthStencilFormatMismatch) {
     utils::ComboRenderBundleEncoderDescriptor renderBundleDesc = {};
     renderBundleDesc.colorFormatsCount = 1;
     renderBundleDesc.cColorFormats[0] = dawn::TextureFormat::RGBA8Unorm;
-    renderBundleDesc.cDepthStencilFormat = dawn::TextureFormat::Depth24Plus;
+    renderBundleDesc.depthStencilFormat = dawn::TextureFormat::Depth24Plus;
-    renderBundleDesc.depthStencilFormat = &renderBundleDesc.cDepthStencilFormat;
 
     dawn::RenderBundleEncoder renderBundleEncoder =
         device.CreateRenderBundleEncoder(&renderBundleDesc);
@@ -1024,8 +1020,7 @@ TEST_F(RenderBundleValidationTest, TextureFormats) {
     // Test that depth/stencil formats are validated as depth/stencil.
     {
         utils::ComboRenderBundleEncoderDescriptor desc = {};
-        desc.cDepthStencilFormat = dawn::TextureFormat::RGBA8Unorm;
+        desc.depthStencilFormat = dawn::TextureFormat::RGBA8Unorm;
-        desc.depthStencilFormat = &desc.cDepthStencilFormat;
         ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
     }
 

src/utils/ComboRenderBundleEncoderDescriptor.cpp

@@ -21,8 +21,6 @@ namespace utils {
     ComboRenderBundleEncoderDescriptor::ComboRenderBundleEncoderDescriptor() {
         dawn::RenderBundleEncoderDescriptor* descriptor = this;
 
-        descriptor->sampleCount = 1;
-        descriptor->depthStencilFormat = nullptr;
         descriptor->colorFormatsCount = 0;
         descriptor->colorFormats = &cColorFormats[0];
     }

src/utils/ComboRenderBundleEncoderDescriptor.h

@@ -28,7 +28,6 @@ namespace utils {
         ComboRenderBundleEncoderDescriptor();
 
         std::array<dawn::TextureFormat, kMaxColorAttachments> cColorFormats;
-        dawn::TextureFormat cDepthStencilFormat;
     };
 
 }  // namespace utils