From 831ff0e39dfde12644b8d4ae95ff97a03a75b163 Mon Sep 17 00:00:00 2001 From: Loko Kung Date: Tue, 7 Feb 2023 23:51:11 +0000 Subject: [PATCH] Deprecates clearColor, clearDepth, and clearStencil values. - Removes remaining usages of the values in Dawn and removes tests. - Note that the values will be removed from the JSON entirely in follow up CL after Chromium side changes. Change-Id: I30ccb3c412cd97047065ad515f6a5ff4de642420 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/117593 Kokoro: Kokoro Reviewed-by: Austin Eng Commit-Queue: Loko Kung --- .../ApplyClearColorValueWithDrawHelper.cpp | 2 +- src/dawn/native/CommandEncoder.cpp | 58 +++---------------- src/dawn/samples/CHelloTriangle.cpp | 1 - .../validation/DeprecatedAPITests.cpp | 48 --------------- 4 files changed, 8 insertions(+), 101 deletions(-) diff --git a/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp b/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp index 47563f7d45..72d62600f7 100644 --- a/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp +++ b/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp @@ -167,7 +167,7 @@ ResultOrError GetOrCreateApplyClearValueWithDrawPipeline( } Color GetClearColorValue(const RenderPassColorAttachment& attachment) { - return HasDeprecatedColor(attachment) ? attachment.clearColor : attachment.clearValue; + return attachment.clearValue; } ResultOrError> CreateUniformBufferWithClearValues( diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp index 08d8858b33..07a851187e 100644 --- a/src/dawn/native/CommandEncoder.cpp +++ b/src/dawn/native/CommandEncoder.cpp @@ -239,16 +239,7 @@ MaybeError ValidateRenderPassColorAttachment(DeviceBase* device, DAWN_INVALID_IF(colorAttachment.loadOp == wgpu::LoadOp::Undefined, "loadOp must be set."); DAWN_INVALID_IF(colorAttachment.storeOp == wgpu::StoreOp::Undefined, "storeOp must be set."); - // TODO(dawn:1269): Remove after the deprecation period. - bool useClearColor = HasDeprecatedColor(colorAttachment); - const dawn::native::Color& clearValue = - useClearColor ? colorAttachment.clearColor : colorAttachment.clearValue; - - if (useClearColor) { - DAWN_TRY(DAWN_MAKE_DEPRECATION_ERROR( - device, "clearColor is deprecated, prefer using clearValue instead.")); - } - + const dawn::native::Color& clearValue = colorAttachment.clearValue; if (colorAttachment.loadOp == wgpu::LoadOp::Clear) { DAWN_INVALID_IF(std::isnan(clearValue.r) || std::isnan(clearValue.g) || std::isnan(clearValue.b) || std::isnan(clearValue.a), @@ -380,14 +371,7 @@ MaybeError ValidateRenderPassDepthStencilAttachment( depthStencilAttachment->stencilReadOnly); } - if (!std::isnan(depthStencilAttachment->clearDepth)) { - DAWN_TRY(DAWN_MAKE_DEPRECATION_ERROR( - device, "clearDepth is deprecated, prefer depthClearValue instead.")); - DAWN_INVALID_IF( - depthStencilAttachment->clearDepth < 0.0f || depthStencilAttachment->clearDepth > 1.0f, - "clearDepth is not between 0.0 and 1.0"); - - } else if (depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Clear) { + if (depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Clear) { DAWN_INVALID_IF(std::isnan(depthStencilAttachment->depthClearValue), "depthClearValue is NaN."); DAWN_INVALID_IF(depthStencilAttachment->depthClearValue < 0.0f || @@ -395,12 +379,6 @@ MaybeError ValidateRenderPassDepthStencilAttachment( "depthClearValue is not between 0.0 and 1.0"); } - if (depthStencilAttachment->stencilClearValue == 0 && - depthStencilAttachment->clearStencil != 0) { - DAWN_TRY(DAWN_MAKE_DEPRECATION_ERROR( - device, "clearStencil is deprecated, prefer stencilClearValue instead.")); - } - // *sampleCount == 0 must only happen when there is no color attachment. In that case we // do not need to validate the sample count of the depth stencil attachment. const uint32_t depthStencilSampleCount = attachment->GetTexture()->GetSampleCount(); @@ -671,11 +649,6 @@ struct TemporaryResolveAttachment { } // namespace -bool HasDeprecatedColor(const RenderPassColorAttachment& attachment) { - return !std::isnan(attachment.clearColor.r) || !std::isnan(attachment.clearColor.g) || - !std::isnan(attachment.clearColor.b) || !std::isnan(attachment.clearColor.a); -} - Color ClampClearColorValueToLegalRange(const Color& originalColor, const Format& format) { const AspectInfo& aspectInfo = format.GetAspectInfo(Aspect::Color); double minValue = 0; @@ -894,10 +867,7 @@ Ref CommandEncoder::BeginRenderPass(const RenderPassDescripto cmd->colorAttachments[index].loadOp = descriptor->colorAttachments[i].loadOp; cmd->colorAttachments[index].storeOp = descriptor->colorAttachments[i].storeOp; - Color color = HasDeprecatedColor(descriptor->colorAttachments[i]) - ? descriptor->colorAttachments[i].clearColor - : descriptor->colorAttachments[i].clearValue; - + Color color = descriptor->colorAttachments[i].clearValue; cmd->colorAttachments[index].clearColor = ClampClearColorValueToLegalRange(color, view->GetFormat()); @@ -914,24 +884,10 @@ Ref CommandEncoder::BeginRenderPass(const RenderPassDescripto cmd->depthStencilAttachment.view = view; - if (!std::isnan(descriptor->depthStencilAttachment->clearDepth)) { - // TODO(dawn:1269): Remove this branch after the deprecation period. - cmd->depthStencilAttachment.clearDepth = - descriptor->depthStencilAttachment->clearDepth; - } else { - cmd->depthStencilAttachment.clearDepth = - descriptor->depthStencilAttachment->depthClearValue; - } - - if (descriptor->depthStencilAttachment->stencilClearValue == 0 && - descriptor->depthStencilAttachment->clearStencil != 0) { - // TODO(dawn:1269): Remove this branch after the deprecation period. - cmd->depthStencilAttachment.clearStencil = - descriptor->depthStencilAttachment->clearStencil; - } else { - cmd->depthStencilAttachment.clearStencil = - descriptor->depthStencilAttachment->stencilClearValue; - } + cmd->depthStencilAttachment.clearDepth = + descriptor->depthStencilAttachment->depthClearValue; + cmd->depthStencilAttachment.clearStencil = + descriptor->depthStencilAttachment->stencilClearValue; // Copy parameters for the depth, reyifing the values when it is not present or // readonly. diff --git a/src/dawn/samples/CHelloTriangle.cpp b/src/dawn/samples/CHelloTriangle.cpp index e4bb623f59..68ec723d84 100644 --- a/src/dawn/samples/CHelloTriangle.cpp +++ b/src/dawn/samples/CHelloTriangle.cpp @@ -107,7 +107,6 @@ void frame() { colorAttachment.view = backbufferView; colorAttachment.resolveTarget = nullptr; colorAttachment.clearValue = {0.0f, 0.0f, 0.0f, 0.0f}; - colorAttachment.clearColor = {NAN, NAN, NAN, NAN}; colorAttachment.loadOp = WGPULoadOp_Clear; colorAttachment.storeOp = WGPUStoreOp_Store; renderpassInfo.colorAttachmentCount = 1; diff --git a/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp b/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp index d66058e045..e3848a975d 100644 --- a/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp +++ b/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp @@ -82,54 +82,6 @@ TEST_P(DeprecationTests, ReadOnlyDepthStencilStoreLoadOpsAttachment) { } } -// Test that setting the clearColor, clearDepth, or clearStencil values for render pass attachments -// is deprecated. (dawn:1269) -TEST_P(DeprecationTests, AttachmentClearColor) { - utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 1, 1); - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::RenderPassEncoder pass; - - // Check that setting load/store ops with read only depth/stencil attachments gives a 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::RenderPassDepthStencilAttachment* depthAttachment = - &renderPass.renderPassInfo.cDepthStencilAttachmentInfo; - renderPass.renderPassInfo.depthStencilAttachment = depthAttachment; - depthAttachment->view = depthStencil.CreateView(); - depthAttachment->depthLoadOp = wgpu::LoadOp::Clear; - depthAttachment->stencilLoadOp = wgpu::LoadOp::Clear; - - // A pass that uses none of the deprecated value should be fine. - pass = encoder.BeginRenderPass(&renderPass.renderPassInfo); - pass.End(); - - depthAttachment->clearStencil = 1; - - EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo)); - pass.End(); - - depthAttachment->clearStencil = 0; - depthAttachment->depthClearValue = 0.0f; - depthAttachment->clearDepth = 1.0f; - - EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo)); - pass.End(); - - renderPass.renderPassInfo.depthStencilAttachment = nullptr; - renderPass.renderPassInfo.cColorAttachments[0].clearColor = {1.0, 2.0, 3.0, 4.0}; - renderPass.renderPassInfo.cColorAttachments[0].clearValue = {5.0, 4.0, 3.0, 2.0}; - - EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo)); - pass.End(); -} - // Test that endPass() is deprecated for both render and compute passes. TEST_P(DeprecationTests, EndPass) { wgpu::CommandEncoder encoder = device.CreateCommandEncoder();