Deprecate renderpass color/depth .attachment

As of https://github.com/gpuweb/gpuweb/pull/1352 the spec indicates that
GPURenderPassColorAttachmentDescriptor and
GPURenderPassDepthStencilAttachmentDescriptor should use .view rather
than .attachment to indicate the TextureView associated with the
render pass attachment.

Bug: dawn:762
Change-Id: I70d615e19d8e7aae5b26aa5965c7109289ab868b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/47902
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>

dawn.json

@@ -1423,21 +1423,27 @@
         ]
     },
 
-    "render pass color attachment descriptor": {
+    "render pass color attachment": {
         "category": "structure",
         "members": [
-            {"name": "attachment", "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"},
-            {"name": "clear color", "type": "color"}
+            {"name": "clear color", "type": "color"},
+            {"name": "attachment", "type": "texture view", "optional": true}
         ]
     },
 
-    "render pass depth stencil attachment descriptor": {
+    "render pass color attachment descriptor": {
+        "category": "typedef",
+        "type": "render pass color attachment"
+    },
+
+    "render pass depth stencil attachment": {
         "category": "structure",
         "members": [
-            {"name": "attachment", "type": "texture view"},
+            {"name": "view", "type": "texture view", "optional": true},
             {"name": "depth load op", "type": "load op"},
             {"name": "depth store op", "type": "store op"},
             {"name": "clear depth", "type": "float"},
@@ -1445,18 +1451,24 @@
             {"name": "stencil load op", "type": "load op"},
             {"name": "stencil store op", "type": "store op"},
             {"name": "clear stencil", "type": "uint32_t", "default": "0"},
-            {"name": "stencil read only", "type": "bool", "default": "false"}
+            {"name": "stencil read only", "type": "bool", "default": "false"},
+            {"name": "attachment", "type": "texture view", "optional": true}
         ]
     },
 
+    "render pass depth stencil attachment descriptor": {
+        "category": "typedef",
+        "type": "render pass depth stencil attachment"
+    },
+
     "render pass descriptor": {
         "category": "structure",
         "extensible": true,
         "members": [
             {"name": "label", "type": "char", "annotation": "const*", "length": "strlen", "optional": true},
             {"name": "color attachment count", "type": "uint32_t"},
-            {"name": "color attachments", "type": "render pass color attachment descriptor", "annotation": "const*", "length": "color attachment count"},
+            {"name": "color attachments", "type": "render pass color attachment", "annotation": "const*", "length": "color attachment count"},
-            {"name": "depth stencil attachment", "type": "render pass depth stencil attachment descriptor", "annotation": "const*", "optional": true},
+            {"name": "depth stencil attachment", "type": "render pass depth stencil attachment", "annotation": "const*", "optional": true},
             {"name": "occlusion query set", "type": "query set", "optional": true}
         ]
     },

src/dawn_native/AttachmentState.cpp

@@ -65,7 +65,10 @@ namespace dawn_native {
              i < ColorAttachmentIndex(static_cast<uint8_t>(descriptor->colorAttachmentCount));
              ++i) {
             TextureViewBase* attachment =
-                descriptor->colorAttachments[static_cast<uint8_t>(i)].attachment;
+                descriptor->colorAttachments[static_cast<uint8_t>(i)].view;
+            if (attachment == nullptr) {
+                attachment = descriptor->colorAttachments[static_cast<uint8_t>(i)].attachment;
+            }
             mColorAttachmentsSet.set(i);
             mColorFormats[i] = attachment->GetFormat().format;
             if (mSampleCount == 0) {
@@ -75,7 +78,10 @@ namespace dawn_native {
             }
         }
         if (descriptor->depthStencilAttachment != nullptr) {
-            TextureViewBase* attachment = descriptor->depthStencilAttachment->attachment;
+            TextureViewBase* attachment = descriptor->depthStencilAttachment->view;
+            if (attachment == nullptr) {
+                attachment = descriptor->depthStencilAttachment->attachment;
+            }
             mDepthStencilFormat = attachment->GetFormat().format;
             if (mSampleCount == 0) {
                 mSampleCount = attachment->GetTexture()->GetSampleCount();

src/dawn_native/CommandEncoder.cpp

@@ -155,7 +155,8 @@ namespace dawn_native {
             }
 
             const TextureViewBase* resolveTarget = colorAttachment.resolveTarget;
-            const TextureViewBase* attachment = colorAttachment.attachment;
+            const TextureViewBase* attachment =
+                colorAttachment.view != nullptr ? colorAttachment.view : colorAttachment.attachment;
             DAWN_TRY(device->ValidateObject(colorAttachment.resolveTarget));
             DAWN_TRY(ValidateCanUseAs(colorAttachment.resolveTarget->GetTexture(),
                                       wgpu::TextureUsage::RenderAttachment));
@@ -203,16 +204,33 @@ namespace dawn_native {
         }
 
         MaybeError ValidateRenderPassColorAttachment(
-            const DeviceBase* device,
+            DeviceBase* device,
             const RenderPassColorAttachmentDescriptor& colorAttachment,
             uint32_t* width,
             uint32_t* height,
             uint32_t* sampleCount) {
-            DAWN_TRY(device->ValidateObject(colorAttachment.attachment));
+            TextureViewBase* attachment;
-            DAWN_TRY(ValidateCanUseAs(colorAttachment.attachment->GetTexture(),
+            if (colorAttachment.view != nullptr) {
-                                      wgpu::TextureUsage::RenderAttachment));
+                if (colorAttachment.attachment != nullptr) {
+                    return DAWN_VALIDATION_ERROR(
+                        "Cannot specify both a attachment and view. attachment is deprecated, "
+                        "favor view instead.");
+                }
+                attachment = colorAttachment.view;
+            } else if (colorAttachment.attachment != nullptr) {
+                device->EmitDeprecationWarning(
+                    "RenderPassColorAttachmentDescriptor.attachment has been deprecated. Use "
+                    "RenderPassColorAttachmentDescriptor.view instead.");
+                attachment = colorAttachment.attachment;
+            } else {
+                return DAWN_VALIDATION_ERROR(
+                    "Must specify a view for RenderPassColorAttachmentDescriptor");
+            }
+
+            DAWN_TRY(device->ValidateObject(attachment));
+            DAWN_TRY(
+                ValidateCanUseAs(attachment->GetTexture(), wgpu::TextureUsage::RenderAttachment));
 
-            const TextureViewBase* attachment = colorAttachment.attachment;
             if (!(attachment->GetAspects() & Aspect::Color) ||
                 !attachment->GetFormat().isRenderable) {
                 return DAWN_VALIDATION_ERROR(
@@ -243,18 +261,35 @@ namespace dawn_native {
         }
 
         MaybeError ValidateRenderPassDepthStencilAttachment(
-            const DeviceBase* device,
+            DeviceBase* device,
             const RenderPassDepthStencilAttachmentDescriptor* depthStencilAttachment,
             uint32_t* width,
             uint32_t* height,
             uint32_t* sampleCount) {
             DAWN_ASSERT(depthStencilAttachment != nullptr);
 
-            DAWN_TRY(device->ValidateObject(depthStencilAttachment->attachment));
+            TextureViewBase* attachment;
-            DAWN_TRY(ValidateCanUseAs(depthStencilAttachment->attachment->GetTexture(),
+            if (depthStencilAttachment->view != nullptr) {
-                                      wgpu::TextureUsage::RenderAttachment));
+                if (depthStencilAttachment->attachment != nullptr) {
+                    return DAWN_VALIDATION_ERROR(
+                        "Cannot specify both a attachment and view. attachment is deprecated, "
+                        "favor view instead.");
+                }
+                attachment = depthStencilAttachment->view;
+            } else if (depthStencilAttachment->attachment != nullptr) {
+                device->EmitDeprecationWarning(
+                    "RenderPassDepthStencilAttachmentDescriptor.attachment has been deprecated. "
+                    "Use RenderPassDepthStencilAttachmentDescriptor.view instead.");
+                attachment = depthStencilAttachment->attachment;
+            } else {
+                return DAWN_VALIDATION_ERROR(
+                    "Must specify a view for RenderPassDepthStencilAttachmentDescriptor");
+            }
+
+            DAWN_TRY(device->ValidateObject(attachment));
+            DAWN_TRY(
+                ValidateCanUseAs(attachment->GetTexture(), wgpu::TextureUsage::RenderAttachment));
 
-            const TextureViewBase* attachment = depthStencilAttachment->attachment;
             if ((attachment->GetAspects() & (Aspect::Depth | Aspect::Stencil)) == Aspect::None ||
                 !attachment->GetFormat().isRenderable) {
                 return DAWN_VALIDATION_ERROR(
@@ -313,7 +348,7 @@ namespace dawn_native {
             return {};
         }
 
-        MaybeError ValidateRenderPassDescriptor(const DeviceBase* device,
+        MaybeError ValidateRenderPassDescriptor(DeviceBase* device,
                                                 const RenderPassDescriptor* descriptor,
                                                 uint32_t* width,
                                                 uint32_t* height,
@@ -521,7 +556,10 @@ namespace dawn_native {
                 for (ColorAttachmentIndex index :
                      IterateBitSet(cmd->attachmentState->GetColorAttachmentsMask())) {
                     uint8_t i = static_cast<uint8_t>(index);
-                    TextureViewBase* view = descriptor->colorAttachments[i].attachment;
+                    TextureViewBase* view = descriptor->colorAttachments[i].view;
+                    if (view == nullptr) {
+                        view = descriptor->colorAttachments[i].attachment;
+                    }
                     TextureViewBase* resolveTarget = descriptor->colorAttachments[i].resolveTarget;
 
                     cmd->colorAttachments[index].view = view;
@@ -540,7 +578,10 @@ namespace dawn_native {
                 }
 
                 if (cmd->attachmentState->HasDepthStencilAttachment()) {
-                    TextureViewBase* view = descriptor->depthStencilAttachment->attachment;
+                    TextureViewBase* view = descriptor->depthStencilAttachment->view;
+                    if (view == nullptr) {
+                        view = descriptor->depthStencilAttachment->attachment;
+                    }
 
                     cmd->depthStencilAttachment.view = view;
                     cmd->depthStencilAttachment.clearDepth =

src/dawn_native/CopyTextureForBrowserHelper.cpp

@@ -329,7 +329,7 @@ namespace dawn_native {
         // Prepare render pass color attachment descriptor.
         RenderPassColorAttachmentDescriptor colorAttachmentDesc;
 
-        colorAttachmentDesc.attachment = dstView.Get();
+        colorAttachmentDesc.view = dstView.Get();
         colorAttachmentDesc.loadOp = wgpu::LoadOp::Load;
         colorAttachmentDesc.storeOp = wgpu::StoreOp::Store;
         colorAttachmentDesc.clearColor = {0.0, 0.0, 0.0, 1.0};

src/tests/end2end/DeprecatedAPITests.cpp

@@ -63,6 +63,44 @@ TEST_P(DeprecationTests, SetSetBlendColor) {
     pass.EndPass();
 }
 
+// Test that setting attachment rather than view for render pass color and depth/stencil attachments
+// is deprecated.
+TEST_P(DeprecationTests, SetAttachmentDescriptorAttachment) {
+    utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 1, 1);
+    wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+    wgpu::RenderPassEncoder pass;
+
+    // Check that using .attachment with color attachments gives the warning.
+    wgpu::RenderPassColorAttachmentDescriptor* colorAttachment =
+        &renderPass.renderPassInfo.cColorAttachments[0];
+    colorAttachment->attachment = colorAttachment->view;
+    colorAttachment->view = nullptr;
+
+    EXPECT_DEPRECATION_WARNING(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+    pass.EndPass();
+
+    colorAttachment->view = colorAttachment->attachment;
+    colorAttachment->attachment = nullptr;
+
+    // Check that using .attachment with depth/stencil attachments gives the warning.
+    wgpu::TextureDescriptor descriptor;
+    descriptor.dimension = wgpu::TextureDimension::e2D;
+    descriptor.size = {1, 1, 1};
+    descriptor.sampleCount = 1;
+    descriptor.format = wgpu::TextureFormat::Depth24PlusStencil8;
+    descriptor.mipLevelCount = 1;
+    descriptor.usage = wgpu::TextureUsage::RenderAttachment;
+    wgpu::Texture depthStencil = device.CreateTexture(&descriptor);
+
+    wgpu::RenderPassDepthStencilAttachmentDescriptor* depthAttachment =
+        &renderPass.renderPassInfo.cDepthStencilAttachmentInfo;
+    renderPass.renderPassInfo.depthStencilAttachment = depthAttachment;
+    depthAttachment->attachment = depthStencil.CreateView();
+
+    EXPECT_DEPRECATION_WARNING(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+    pass.EndPass();
+}
+
 // Test that BindGroupLayoutEntry cannot have a type if buffer, sampler, texture, or storageTexture
 // are defined.
 TEST_P(DeprecationTests, BindGroupLayoutEntryTypeConflict) {

src/tests/end2end/MultisampledRenderingTests.cpp

@@ -172,7 +172,7 @@ class MultisampledRenderingTest : public DawnTest {
         renderPass.cDepthStencilAttachmentInfo.depthLoadOp = depthStencilLoadOp;
 
         if (hasDepthStencilAttachment) {
-            renderPass.cDepthStencilAttachmentInfo.attachment = mDepthStencilView;
+            renderPass.cDepthStencilAttachmentInfo.view = mDepthStencilView;
             renderPass.depthStencilAttachment = &renderPass.cDepthStencilAttachmentInfo;
         }
 

src/tests/end2end/TextureZeroInitTests.cpp

@@ -203,7 +203,7 @@ TEST_P(TextureZeroInitTest, RenderingMipMapClearsToZero) {
     renderPass.renderPassInfo.cColorAttachments[0].loadOp = wgpu::LoadOp::Load;
     // Specify non-zero clear color. It should still be cleared to zero.
     renderPass.renderPassInfo.cColorAttachments[0].clearColor = {0.5f, 0.5f, 0.5f, 0.5f};
-    renderPass.renderPassInfo.cColorAttachments[0].attachment = view;
+    renderPass.renderPassInfo.cColorAttachments[0].view = view;
 
     wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
     {
@@ -248,7 +248,7 @@ TEST_P(TextureZeroInitTest, RenderingArrayLayerClearsToZero) {
     renderPass.renderPassInfo.cColorAttachments[0].loadOp = wgpu::LoadOp::Load;
     // Specify non-zero clear color. It should still be cleared to zero.
     renderPass.renderPassInfo.cColorAttachments[0].clearColor = {0.5f, 0.5f, 0.5f, 0.5f};
-    renderPass.renderPassInfo.cColorAttachments[0].attachment = view;
+    renderPass.renderPassInfo.cColorAttachments[0].view = view;
 
     wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
     {

src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp

@@ -119,23 +119,23 @@ namespace {
             // We cannot use utils::ComboRenderPassDescriptor here because it only supports at most
             // kMaxColorAttachments(4) color attachments.
             std::array<wgpu::RenderPassColorAttachmentDescriptor, 5> colorAttachments;
-            colorAttachments[0].attachment = color0;
+            colorAttachments[0].view = color0;
             colorAttachments[0].resolveTarget = nullptr;
             colorAttachments[0].clearColor = {0.0f, 0.0f, 0.0f, 0.0f};
             colorAttachments[0].loadOp = wgpu::LoadOp::Clear;
             colorAttachments[0].storeOp = wgpu::StoreOp::Store;
 
             colorAttachments[1] = colorAttachments[0];
-            colorAttachments[1].attachment = color1;
+            colorAttachments[1].view = color1;
 
             colorAttachments[2] = colorAttachments[0];
-            colorAttachments[2].attachment = color2;
+            colorAttachments[2].view = color2;
 
             colorAttachments[3] = colorAttachments[0];
-            colorAttachments[3].attachment = color3;
+            colorAttachments[3].view = color3;
 
             colorAttachments[4] = colorAttachments[0];
-            colorAttachments[4].attachment =
+            colorAttachments[4].view =
                 Create2DAttachment(device, 1, 1, wgpu::TextureFormat::RGBA8Unorm);
 
             wgpu::RenderPassDescriptor renderPass;

src/tests/unittests/validation/ValidationTest.cpp

@@ -220,7 +220,7 @@ ValidationTest::DummyRenderPass::DummyRenderPass(const wgpu::Device& device)
     attachment = device.CreateTexture(&descriptor);
 
     wgpu::TextureView view = attachment.CreateView();
-    mColorAttachment.attachment = view;
+    mColorAttachment.view = view;
     mColorAttachment.resolveTarget = nullptr;
     mColorAttachment.clearColor = {0.0f, 0.0f, 0.0f, 0.0f};
     mColorAttachment.loadOp = wgpu::LoadOp::Clear;

src/utils/WGPUHelpers.cpp

@@ -104,14 +104,14 @@ namespace utils {
         uint32_t colorAttachmentIndex = 0;
         for (const wgpu::TextureView& colorAttachment : colorAttachmentInfo) {
             if (colorAttachment.Get() != nullptr) {
-                cColorAttachments[colorAttachmentIndex].attachment = colorAttachment;
+                cColorAttachments[colorAttachmentIndex].view = colorAttachment;
             }
             ++colorAttachmentIndex;
         }
         colorAttachments = cColorAttachments.data();
 
         if (depthStencil.Get() != nullptr) {
-            cDepthStencilAttachmentInfo.attachment = depthStencil;
+            cDepthStencilAttachmentInfo.view = depthStencil;
             depthStencilAttachment = &cDepthStencilAttachmentInfo;
         } else {
             depthStencilAttachment = nullptr;