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>
diff --git a/dawn.json b/dawn.json
index 9486ef3..0fd7be8 100644
--- a/dawn.json
+++ b/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": {
diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp
index f02a20f..19b8010 100644
--- a/src/dawn_native/CommandBuffer.cpp
+++ b/src/dawn_native/CommandBuffer.cpp
@@ -125,6 +125,7 @@
view->GetTexture()->SetIsSubresourceContentInitialized(true, range);
break;
+ case wgpu::StoreOp::Discard:
case wgpu::StoreOp::Clear:
view->GetTexture()->SetIsSubresourceContentInitialized(false, range);
break;
diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp
index 346bf5c..507c3b1 100644
--- a/src/dawn_native/CommandEncoder.cpp
+++ b/src/dawn_native/CommandEncoder.cpp
@@ -41,6 +41,14 @@
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 @@
}
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 @@
DAWN_TRY(ValidateLoadOp(depthStencilAttachment->depthLoadOp));
DAWN_TRY(ValidateLoadOp(depthStencilAttachment->stencilLoadOp));
- DAWN_TRY(ValidateStoreOp(depthStencilAttachment->depthStoreOp));
- DAWN_TRY(ValidateStoreOp(depthStencilAttachment->stencilStoreOp));
+ DAWN_TRY(ValidateDeprecatedStoreOp(device, depthStencilAttachment->depthStoreOp));
+ DAWN_TRY(ValidateDeprecatedStoreOp(device, depthStencilAttachment->stencilStoreOp));
if (attachment->GetAspects() == (Aspect::Depth | Aspect::Stencil) &&
depthStencilAttachment->depthReadOnly != depthStencilAttachment->stencilReadOnly) {
diff --git a/src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp b/src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp
index f24a39f..1475311 100644
--- a/src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp
+++ b/src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp
@@ -35,6 +35,7 @@
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 @@
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;
diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm
index b390184..4b389bf 100644
--- a/src/dawn_native/metal/CommandBufferMTL.mm
+++ b/src/dawn_native/metal/CommandBufferMTL.mm
@@ -98,6 +98,7 @@
descriptor.colorAttachments[i].storeAction =
kMTLStoreActionStoreAndMultisampleResolve;
break;
+ case wgpu::StoreOp::Discard:
case wgpu::StoreOp::Clear:
descriptor.colorAttachments[i].storeAction =
MTLStoreActionMultisampleResolve;
@@ -108,6 +109,7 @@
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 @@
descriptor.depthAttachment.storeAction = MTLStoreActionStore;
break;
+ case wgpu::StoreOp::Discard:
case wgpu::StoreOp::Clear:
descriptor.depthAttachment.storeAction = MTLStoreActionDontCare;
break;
@@ -159,6 +162,7 @@
descriptor.stencilAttachment.storeAction = MTLStoreActionStore;
break;
+ case wgpu::StoreOp::Discard:
case wgpu::StoreOp::Clear:
descriptor.stencilAttachment.storeAction = MTLStoreActionDontCare;
break;
diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp
index e97ffb6..ee8504b 100644
--- a/src/dawn_native/opengl/CommandBufferGL.cpp
+++ b/src/dawn_native/opengl/CommandBufferGL.cpp
@@ -1054,7 +1054,8 @@
}
}
- 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
}
}
diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp
index 645214b..4ab9ec1 100644
--- a/src/tests/end2end/DeprecatedAPITests.cpp
+++ b/src/tests/end2end/DeprecatedAPITests.cpp
@@ -97,6 +97,46 @@
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(),
diff --git a/src/tests/end2end/MultisampledRenderingTests.cpp b/src/tests/end2end/MultisampledRenderingTests.cpp
index d99d36b..8d16b6c 100644
--- a/src/tests/end2end/MultisampledRenderingTests.cpp
+++ b/src/tests/end2end/MultisampledRenderingTests.cpp
@@ -291,7 +291,7 @@
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.
diff --git a/src/tests/end2end/TextureZeroInitTests.cpp b/src/tests/end2end/TextureZeroInitTests.cpp
index 8c657ea..78cbc43 100644
--- a/src/tests/end2end/TextureZeroInitTests.cpp
+++ b/src/tests/end2end/TextureZeroInitTests.cpp
@@ -600,7 +600,7 @@
{
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 @@
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 @@
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 @@
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 @@
// 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.
-// 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.
+// 1) LoadOp::Clear and StoreOp::Discard, fail the depth and stencil test set in the render
+// pipeline. This means nothing is drawn and subresource is set as uninitialized.
+// 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 @@
renderPassDescriptor.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Clear;
renderPassDescriptor.cDepthStencilAttachmentInfo.clearDepth = 1.0f;
renderPassDescriptor.cDepthStencilAttachmentInfo.clearStencil = 1u;
- renderPassDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Clear;
- renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Clear;
+ renderPassDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Discard;
+ renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Discard;
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDescriptor);
@@ -1303,7 +1304,7 @@
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 @@
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);
diff --git a/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
index f82a6cb..249325a 100644
--- a/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
+++ b/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
@@ -257,8 +257,8 @@
// Base case: StoreOps match so render pass is a success
{
utils::ComboRenderPassDescriptor renderPass({}, depthStencilView);
- renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Clear;
- renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Clear;
+ renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Discard;
+ renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Discard;
AssertBeginRenderPassSuccess(&renderPass);
}
@@ -266,7 +266,7 @@
{
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 @@
{
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);
}