Deprecated StoreOp::Clear in favor of Discard

Change-Id: Ifac3b980f23c3476d91bb89759b2a60ed5efbc17
Bug: dawn:937
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/56045
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>

dawn.json

@@ -1067,7 +1067,8 @@
         "category": "enum",
         "values": [
             {"value": 0, "name": "store"},
-            {"value": 1, "name": "clear"}
+            {"value": 1, "name": "discard"},
+            {"value": 2, "name": "clear"}
         ]
     },
     "origin 3D": {

src/dawn_native/CommandBuffer.cpp

@@ -125,6 +125,7 @@ namespace dawn_native {
                     view->GetTexture()->SetIsSubresourceContentInitialized(true, range);
                     break;
 
+                case wgpu::StoreOp::Discard:
                 case wgpu::StoreOp::Clear:
                     view->GetTexture()->SetIsSubresourceContentInitialized(false, range);
                     break;

src/dawn_native/CommandEncoder.cpp

@@ -41,6 +41,14 @@ namespace dawn_native {
 
     namespace {
 
+        MaybeError ValidateDeprecatedStoreOp(DeviceBase* device, wgpu::StoreOp value) {
+            if (value == wgpu::StoreOp::Clear) {
+                device->EmitDeprecationWarning(
+                    "The 'clear' storeOp is deprecated. Use 'discard' instead.");
+            }
+            return ValidateStoreOp(value);
+        }
+
         MaybeError ValidateB2BCopyAlignment(uint64_t dataSize,
                                             uint64_t srcOffset,
                                             uint64_t dstOffset) {
@@ -244,7 +252,7 @@ namespace dawn_native {
             }
 
             DAWN_TRY(ValidateLoadOp(colorAttachment.loadOp));
-            DAWN_TRY(ValidateStoreOp(colorAttachment.storeOp));
+            DAWN_TRY(ValidateDeprecatedStoreOp(device, colorAttachment.storeOp));
 
             if (colorAttachment.loadOp == wgpu::LoadOp::Clear) {
                 if (std::isnan(colorAttachment.clearColor.r) ||
@@ -315,8 +323,8 @@ namespace dawn_native {
 
             DAWN_TRY(ValidateLoadOp(depthStencilAttachment->depthLoadOp));
             DAWN_TRY(ValidateLoadOp(depthStencilAttachment->stencilLoadOp));
-            DAWN_TRY(ValidateStoreOp(depthStencilAttachment->depthStoreOp));
+            DAWN_TRY(ValidateDeprecatedStoreOp(device, depthStencilAttachment->depthStoreOp));
-            DAWN_TRY(ValidateStoreOp(depthStencilAttachment->stencilStoreOp));
+            DAWN_TRY(ValidateDeprecatedStoreOp(device, depthStencilAttachment->stencilStoreOp));
 
             if (attachment->GetAspects() == (Aspect::Depth | Aspect::Stencil) &&
                 depthStencilAttachment->depthReadOnly != depthStencilAttachment->stencilReadOnly) {

src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp

@@ -35,6 +35,7 @@ namespace dawn_native { namespace d3d12 {
 
         D3D12_RENDER_PASS_ENDING_ACCESS_TYPE D3D12EndingAccessType(wgpu::StoreOp storeOp) {
             switch (storeOp) {
+                case wgpu::StoreOp::Discard:
                 case wgpu::StoreOp::Clear:
                     return D3D12_RENDER_PASS_ENDING_ACCESS_TYPE_DISCARD;
                 case wgpu::StoreOp::Store:
@@ -55,7 +56,7 @@ namespace dawn_native { namespace d3d12 {
                 ToBackend(resolveDestination->GetTexture())->GetD3D12Resource();
 
             // Clear or preserve the resolve source.
-            if (storeOp == wgpu::StoreOp::Clear) {
+            if (storeOp == wgpu::StoreOp::Discard || storeOp == wgpu::StoreOp::Clear) {
                 resolveParameters.PreserveResolveSource = false;
             } else if (storeOp == wgpu::StoreOp::Store) {
                 resolveParameters.PreserveResolveSource = true;

src/dawn_native/metal/CommandBufferMTL.mm

@@ -98,6 +98,7 @@ namespace dawn_native { namespace metal {
                             descriptor.colorAttachments[i].storeAction =
                                 kMTLStoreActionStoreAndMultisampleResolve;
                             break;
+                        case wgpu::StoreOp::Discard:
                         case wgpu::StoreOp::Clear:
                             descriptor.colorAttachments[i].storeAction =
                                 MTLStoreActionMultisampleResolve;
@@ -108,6 +109,7 @@ namespace dawn_native { namespace metal {
                         case wgpu::StoreOp::Store:
                             descriptor.colorAttachments[i].storeAction = MTLStoreActionStore;
                             break;
+                        case wgpu::StoreOp::Discard:
                         case wgpu::StoreOp::Clear:
                             descriptor.colorAttachments[i].storeAction = MTLStoreActionDontCare;
                             break;
@@ -132,6 +134,7 @@ namespace dawn_native { namespace metal {
                             descriptor.depthAttachment.storeAction = MTLStoreActionStore;
                             break;
 
+                        case wgpu::StoreOp::Discard:
                         case wgpu::StoreOp::Clear:
                             descriptor.depthAttachment.storeAction = MTLStoreActionDontCare;
                             break;
@@ -159,6 +162,7 @@ namespace dawn_native { namespace metal {
                             descriptor.stencilAttachment.storeAction = MTLStoreActionStore;
                             break;
 
+                        case wgpu::StoreOp::Discard:
                         case wgpu::StoreOp::Clear:
                             descriptor.stencilAttachment.storeAction = MTLStoreActionDontCare;
                             break;

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -1054,7 +1054,8 @@ namespace dawn_native { namespace opengl {
                     }
                 }
 
-                if (attachmentInfo->storeOp == wgpu::StoreOp::Clear) {
+                if (attachmentInfo->storeOp == wgpu::StoreOp::Discard ||
+                    attachmentInfo->storeOp == wgpu::StoreOp::Clear) {
                     // TODO(natlee@microsoft.com): call glDiscard to do optimization
                 }
             }

src/tests/end2end/DeprecatedAPITests.cpp

@@ -97,6 +97,46 @@ TEST_P(DeprecationTests, ComputeStage) {
     EXPECT_DEPRECATION_WARNING(pipeline = device.CreateComputePipeline(&csDesc));
 }
 
+// Test that StoreOp::Clear is deprecated.
+TEST_P(DeprecationTests, StoreOpClear) {
+    utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 1, 1);
+    wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+    wgpu::RenderPassEncoder pass;
+
+    // Check that a storeOp of Clear for color attachments raises a validation warning.
+    renderPass.renderPassInfo.cColorAttachments[0].storeOp = wgpu::StoreOp::Clear;
+
+    EXPECT_DEPRECATION_WARNING(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+    pass.EndPass();
+
+    // Check that a storeOp of Clear for depth/stencil attachments raises a validation 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->view = depthStencil.CreateView();
+
+    renderPass.renderPassInfo.cColorAttachments[0].storeOp = wgpu::StoreOp::Discard;
+    depthAttachment->depthStoreOp = wgpu::StoreOp::Clear;
+
+    EXPECT_DEPRECATION_WARNING(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+    pass.EndPass();
+
+    depthAttachment->depthStoreOp = wgpu::StoreOp::Discard;
+    depthAttachment->stencilStoreOp = wgpu::StoreOp::Clear;
+
+    EXPECT_DEPRECATION_WARNING(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+    pass.EndPass();
+}
+
 DAWN_INSTANTIATE_TEST(DeprecationTests,
                       D3D12Backend(),
                       MetalBackend(),

src/tests/end2end/MultisampledRenderingTests.cpp

@@ -291,7 +291,7 @@ TEST_P(MultisampledRenderingTest, ResolveInto2DTexture) {
     constexpr wgpu::Color kGreen = {0.0f, 0.8f, 0.0f, 0.8f};
 
     // storeOp should not affect the result in the resolve target.
-    for (wgpu::StoreOp storeOp : {wgpu::StoreOp::Store, wgpu::StoreOp::Clear}) {
+    for (wgpu::StoreOp storeOp : {wgpu::StoreOp::Store, wgpu::StoreOp::Discard}) {
         wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
 
         // Draw a green triangle.

src/tests/end2end/TextureZeroInitTests.cpp

@@ -600,7 +600,7 @@ TEST_P(TextureZeroInitTest, IndependentDepthStencilLoadAfterDiscard) {
         {
             utils::ComboRenderPassDescriptor renderPassDescriptor({},
                                                                   depthStencilTexture.CreateView());
-            renderPassDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Clear;
+            renderPassDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Discard;
             renderPassDescriptor.cDepthStencilAttachmentInfo.clearStencil = 2;
             renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
 
@@ -676,7 +676,8 @@ TEST_P(TextureZeroInitTest, IndependentDepthStencilLoadAfterDiscard) {
                                                                   depthStencilTexture.CreateView());
             renderPassDescriptor.cDepthStencilAttachmentInfo.clearDepth = 0.7;
             renderPassDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
-            renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Clear;
+            renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp =
+                wgpu::StoreOp::Discard;
 
             wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
             auto pass = encoder.BeginRenderPass(&renderPassDescriptor);
@@ -761,7 +762,7 @@ TEST_P(TextureZeroInitTest, IndependentDepthStencilCopyAfterDiscard) {
         utils::ComboRenderPassDescriptor renderPassDescriptor({}, depthStencilTexture.CreateView());
         renderPassDescriptor.cDepthStencilAttachmentInfo.clearDepth = 0.3;
         renderPassDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
-        renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Clear;
+        renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Discard;
 
         wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
         auto pass = encoder.BeginRenderPass(&renderPassDescriptor);
@@ -1157,7 +1158,7 @@ TEST_P(TextureZeroInitTest, RenderPassStoreOpClear) {
     utils::ComboRenderPassDescriptor renderPassDesc({renderTexture.CreateView()});
     renderPassDesc.cColorAttachments[0].clearColor = {0.0, 0.0, 0.0, 0.0};
     renderPassDesc.cColorAttachments[0].loadOp = wgpu::LoadOp::Clear;
-    renderPassDesc.cColorAttachments[0].storeOp = wgpu::StoreOp::Clear;
+    renderPassDesc.cColorAttachments[0].storeOp = wgpu::StoreOp::Discard;
     wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDesc);
     pass.SetPipeline(renderPipeline);
     pass.SetBindGroup(0, bindGroup);
@@ -1180,9 +1181,9 @@ TEST_P(TextureZeroInitTest, RenderPassStoreOpClear) {
 
 // This tests storeOp Clear on depth and stencil textures.
 // We put the depth stencil texture through 2 passes:
-// 1) LoadOp::Clear and StoreOp::Clear, fail the depth and stencil test set in the render pipeline.
+// 1) LoadOp::Clear and StoreOp::Discard, fail the depth and stencil test set in the render
-//      This means nothing is drawn and subresource is set as uninitialized.
+//      pipeline. This means nothing is drawn and subresource is set as uninitialized.
-// 2) LoadOp::Load and StoreOp::Clear, pass the depth and stencil test set in the render pipeline.
+// 2) LoadOp::Load and StoreOp::Discard, pass the depth and stencil test set in the render pipeline.
 //      Because LoadOp is Load and the subresource is uninitialized, the texture will be cleared to
 //      0's This means the depth and stencil test will pass and the red square is drawn.
 TEST_P(TextureZeroInitTest, RenderingLoadingDepthStencilStoreOpClear) {
@@ -1209,8 +1210,8 @@ TEST_P(TextureZeroInitTest, RenderingLoadingDepthStencilStoreOpClear) {
     renderPassDescriptor.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Clear;
     renderPassDescriptor.cDepthStencilAttachmentInfo.clearDepth = 1.0f;
     renderPassDescriptor.cDepthStencilAttachmentInfo.clearStencil = 1u;
-    renderPassDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Clear;
+    renderPassDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Discard;
-    renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Clear;
+    renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Discard;
     {
         wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
         wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDescriptor);
@@ -1303,7 +1304,7 @@ TEST_P(TextureZeroInitTest, PreservesInitializedMip) {
     utils::ComboRenderPassDescriptor renderPassDesc({renderTexture.CreateView()});
     renderPassDesc.cColorAttachments[0].clearColor = {0.0, 0.0, 0.0, 0.0};
     renderPassDesc.cColorAttachments[0].loadOp = wgpu::LoadOp::Clear;
-    renderPassDesc.cColorAttachments[0].storeOp = wgpu::StoreOp::Clear;
+    renderPassDesc.cColorAttachments[0].storeOp = wgpu::StoreOp::Discard;
     wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDesc);
     pass.SetPipeline(renderPipeline);
     pass.SetBindGroup(0, bindGroup);
@@ -1386,7 +1387,7 @@ TEST_P(TextureZeroInitTest, PreservesInitializedArrayLayer) {
     utils::ComboRenderPassDescriptor renderPassDesc({renderTexture.CreateView()});
     renderPassDesc.cColorAttachments[0].clearColor = {0.0, 0.0, 0.0, 0.0};
     renderPassDesc.cColorAttachments[0].loadOp = wgpu::LoadOp::Clear;
-    renderPassDesc.cColorAttachments[0].storeOp = wgpu::StoreOp::Clear;
+    renderPassDesc.cColorAttachments[0].storeOp = wgpu::StoreOp::Discard;
     wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDesc);
     pass.SetPipeline(renderPipeline);
     pass.SetBindGroup(0, bindGroup);

src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp

@@ -257,8 +257,8 @@ namespace {
         // Base case: StoreOps match so render pass is a success
         {
             utils::ComboRenderPassDescriptor renderPass({}, depthStencilView);
-            renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Clear;
+            renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Discard;
-            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Clear;
+            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Discard;
             AssertBeginRenderPassSuccess(&renderPass);
         }
 
@@ -266,7 +266,7 @@ namespace {
         {
             utils::ComboRenderPassDescriptor renderPass({}, depthStencilView);
             renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
-            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Clear;
+            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Discard;
             AssertBeginRenderPassSuccess(&renderPass);
         }
     }
@@ -871,10 +871,10 @@ namespace {
         {
             utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);
             renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
-            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Clear;
+            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Discard;
             renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
             renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
-            renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Clear;
+            renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Discard;
             renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = true;
             AssertBeginRenderPassError(&renderPass);
         }