D3D11: add clear_color_with_draw toggle
D3D11 ClearRenderTargetView() could be buggy with some old driver
or GPUs. So add this toggle to force dawn using draw to clear
color attachments of a render pass.
This CL also fixes some issues in MaybeApplyClearWithDraw()
1. the draw quad's vertex coordinates are wrong.
2. the pipeline sample count doesn't match the render pass
3. the pipeline depth and stencil doesn't match the
render pass.
Bug: chromium:329702368
Change-Id: I66c473b5870378c85f72ebbba6ea2bb382ccc8f0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/179982
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>
diff --git a/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp b/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp
index 59a4f65..db0deff 100644
--- a/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp
+++ b/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp
@@ -28,6 +28,7 @@
#include "dawn/native/ApplyClearColorValueWithDrawHelper.h"
#include <limits>
+#include <optional>
#include <string>
#include <utility>
#include <vector>
@@ -53,10 +54,10 @@
@vertex
fn main(@builtin(vertex_index) VertexIndex : u32) -> @builtin(position) vec4f {
var pos = array(
- vec2f( 0.0, -1.0),
+ vec2f(-1.0, -1.0),
vec2f( 1.0, -1.0),
- vec2f( 0.0, 1.0),
- vec2f( 0.0, 1.0),
+ vec2f(-1.0, 1.0),
+ vec2f(-1.0, 1.0),
vec2f( 1.0, -1.0),
vec2f( 1.0, 1.0));
return vec4f(pos[VertexIndex], 0.0, 1.0);
@@ -72,7 +73,7 @@
case TextureComponentType::Uint:
return "u32";
case TextureComponentType::Float:
- break;
+ return "f32";
}
DAWN_UNREACHABLE();
}
@@ -117,7 +118,6 @@
<< R"(
return outputColor;
})";
-
return fragmentShaderStream.str();
}
@@ -169,7 +169,12 @@
renderPipelineDesc.vertex = vertex;
renderPipelineDesc.fragment = &fragment;
- renderPipelineDesc.primitive.topology = wgpu::PrimitiveTopology::TriangleList;
+ renderPipelineDesc.multisample.count = key.sampleCount;
+ DepthStencilState depthStencilState = {};
+ if (key.depthStencilFormat != wgpu::TextureFormat::Undefined) {
+ depthStencilState.format = key.depthStencilFormat;
+ renderPipelineDesc.depthStencil = &depthStencilState;
+ }
fragment.targetCount = key.colorAttachmentCount;
fragment.targets = colorTargets.data();
@@ -239,17 +244,7 @@
return std::move(outputBuffer);
}
-// Helper functions for applying big integer clear values with draw
-bool ShouldApplyClearBigIntegerColorValueWithDraw(
- const RenderPassColorAttachment& colorAttachmentInfo) {
- if (colorAttachmentInfo.view == nullptr) {
- return false;
- }
-
- if (colorAttachmentInfo.loadOp != wgpu::LoadOp::Clear) {
- return false;
- }
-
+bool NeedsBigIntClear(const RenderPassColorAttachment& colorAttachmentInfo) {
// We should only apply this workaround on 32-bit signed and unsigned integer formats.
const Format& format = colorAttachmentInfo.view->GetFormat();
switch (format.format) {
@@ -300,8 +295,17 @@
return true;
}
-KeyOfApplyClearColorValueWithDrawPipelines GetKeyOfApplyClearColorValueWithDrawPipelines(
- const RenderPassDescriptor* renderPassDescriptor) {
+std::optional<KeyOfApplyClearColorValueWithDrawPipelines>
+GetKeyOfApplyClearColorValueWithDrawPipelines(const DeviceBase* device,
+ const RenderPassDescriptor* renderPassDescriptor) {
+ bool clearWithDraw = device->IsToggleEnabled(Toggle::ClearColorWithDraw);
+ bool clearWithDrawForBigInt =
+ device->IsToggleEnabled(Toggle::ApplyClearBigIntegerColorValueWithDraw);
+
+ if (!clearWithDraw && !clearWithDrawForBigInt) {
+ return std::nullopt;
+ }
+
KeyOfApplyClearColorValueWithDrawPipelines key;
key.colorAttachmentCount = renderPassDescriptor->colorAttachmentCount;
@@ -310,14 +314,36 @@
key.colorTargetFormats.fill(wgpu::TextureFormat::Undefined);
for (auto [i, attachment] : Enumerate(colorAttachments)) {
- if (attachment.view != nullptr) {
- key.colorTargetFormats[i] = attachment.view->GetFormat().format;
+ if (attachment.view == nullptr) {
+ continue;
}
- if (ShouldApplyClearBigIntegerColorValueWithDraw(attachment)) {
+ key.colorTargetFormats[i] = attachment.view->GetFormat().format;
+ if (key.sampleCount == 0) {
+ key.sampleCount = attachment.view->GetTexture()->GetSampleCount();
+ } else {
+ DAWN_ASSERT(key.sampleCount == attachment.view->GetTexture()->GetSampleCount());
+ }
+
+ if (attachment.loadOp != wgpu::LoadOp::Clear) {
+ continue;
+ }
+
+ if (clearWithDraw || (clearWithDrawForBigInt && NeedsBigIntClear(attachment))) {
key.colorTargetsToApplyClearColorValue.set(i);
}
}
+
+ if (renderPassDescriptor->depthStencilAttachment &&
+ renderPassDescriptor->depthStencilAttachment->view != nullptr) {
+ key.depthStencilFormat =
+ renderPassDescriptor->depthStencilAttachment->view->GetFormat().format;
+ }
+
+ if (key.colorTargetsToApplyClearColorValue.none()) {
+ return std::nullopt;
+ }
+
return key;
}
@@ -335,6 +361,10 @@
HashCombine(&hash, format);
}
+ HashCombine(&hash, key.sampleCount);
+
+ HashCombine(&hash, key.depthStencilFormat);
+
return hash;
}
@@ -354,43 +384,29 @@
return false;
}
}
- return true;
+
+ return key1.sampleCount == key2.sampleCount &&
+ key1.depthStencilFormat == key2.depthStencilFormat;
}
-bool ShouldApplyClearBigIntegerColorValueWithDraw(
- const DeviceBase* device,
- const RenderPassDescriptor* renderPassDescriptor) {
- if (!device->IsToggleEnabled(Toggle::ApplyClearBigIntegerColorValueWithDraw)) {
- return false;
- }
-
- for (uint32_t i = 0; i < renderPassDescriptor->colorAttachmentCount; ++i) {
- if (ShouldApplyClearBigIntegerColorValueWithDraw(
- renderPassDescriptor->colorAttachments[i])) {
- return true;
- }
- }
-
- return false;
-}
-
-MaybeError ApplyClearBigIntegerColorValueWithDraw(
- RenderPassEncoder* renderPassEncoder,
- const RenderPassDescriptor* renderPassDescriptor) {
+MaybeError ApplyClearWithDraw(RenderPassEncoder* renderPassEncoder,
+ const RenderPassDescriptor* renderPassDescriptor) {
DeviceBase* device = renderPassEncoder->GetDevice();
-
- KeyOfApplyClearColorValueWithDrawPipelines key =
- GetKeyOfApplyClearColorValueWithDrawPipelines(renderPassDescriptor);
+ std::optional<KeyOfApplyClearColorValueWithDrawPipelines> key =
+ GetKeyOfApplyClearColorValueWithDrawPipelines(device, renderPassDescriptor);
+ if (!key.has_value()) {
+ return {};
+ }
RenderPipelineBase* pipeline = nullptr;
- DAWN_TRY_ASSIGN(pipeline, GetOrCreateApplyClearValueWithDrawPipeline(device, key));
+ DAWN_TRY_ASSIGN(pipeline, GetOrCreateApplyClearValueWithDrawPipeline(device, key.value()));
Ref<BindGroupLayoutBase> layout;
DAWN_TRY_ASSIGN(layout, pipeline->GetBindGroupLayout(0));
Ref<BufferBase> uniformBufferWithClearColorValues;
DAWN_TRY_ASSIGN(uniformBufferWithClearColorValues,
- CreateUniformBufferWithClearValues(device, renderPassDescriptor, key));
+ CreateUniformBufferWithClearValues(device, renderPassDescriptor, key.value()));
Ref<BindGroupBase> bindGroup;
DAWN_TRY_ASSIGN(bindGroup,
diff --git a/src/dawn/native/ApplyClearColorValueWithDrawHelper.h b/src/dawn/native/ApplyClearColorValueWithDrawHelper.h
index b1a2a05..102de12 100644
--- a/src/dawn/native/ApplyClearColorValueWithDrawHelper.h
+++ b/src/dawn/native/ApplyClearColorValueWithDrawHelper.h
@@ -42,9 +42,11 @@
struct RenderPassDescriptor;
struct KeyOfApplyClearColorValueWithDrawPipelines {
- uint8_t colorAttachmentCount;
+ uint8_t colorAttachmentCount = 0;
PerColorAttachment<wgpu::TextureFormat> colorTargetFormats;
ColorAttachmentMask colorTargetsToApplyClearColorValue;
+ uint32_t sampleCount = 0;
+ wgpu::TextureFormat depthStencilFormat = wgpu::TextureFormat::Undefined;
};
struct KeyOfApplyClearColorValueWithDrawPipelinesHashFunc {
@@ -60,11 +62,8 @@
KeyOfApplyClearColorValueWithDrawPipelinesHashFunc,
KeyOfApplyClearColorValueWithDrawPipelinesEqualityFunc>;
-bool ShouldApplyClearBigIntegerColorValueWithDraw(const DeviceBase* device,
- const RenderPassDescriptor* renderPassDescriptor);
-
-MaybeError ApplyClearBigIntegerColorValueWithDraw(RenderPassEncoder* renderPassEncoder,
- const RenderPassDescriptor* renderPassDescriptor);
+MaybeError ApplyClearWithDraw(RenderPassEncoder* renderPassEncoder,
+ const RenderPassDescriptor* renderPassDescriptor);
} // namespace dawn::native
diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp
index e370789..cd33fac 100644
--- a/src/dawn/native/CommandEncoder.cpp
+++ b/src/dawn/native/CommandEncoder.cpp
@@ -1362,15 +1362,20 @@
mEncodingContext.EnterPass(passEncoder.Get());
- MaybeError error;
- if (validationState.GetImplicitSampleCount() > 1) {
- error = ApplyMSAARenderToSingleSampledLoadOp(device, passEncoder.Get(), *descriptor,
- validationState.GetImplicitSampleCount());
- } else if (ShouldApplyClearBigIntegerColorValueWithDraw(device, *descriptor)) {
- // This is skipped if implicitSampleCount > 1. Because implicitSampleCount > 1 is only
- // supported for non-integer textures.
- error = ApplyClearBigIntegerColorValueWithDraw(passEncoder.Get(), *descriptor);
- }
+ auto error = [&]() -> MaybeError {
+ if (validationState.GetImplicitSampleCount() > 1) {
+ DAWN_TRY(
+ ApplyMSAARenderToSingleSampledLoadOp(device, passEncoder.Get(), *descriptor,
+ validationState.GetImplicitSampleCount()));
+ }
+ // ApplyClearWithDraw() applies clear with draw if clear_color_with_draw or
+ // apply_clear_big_integer_color_value_with_draw toggle is enabled, and the render pass
+ // attachments need to be cleared.
+ DAWN_TRY(ApplyClearWithDraw(passEncoder.Get(), *descriptor));
+
+ return {};
+ }();
+
if (device->ConsumedError(std::move(error))) {
return MakeError();
}
diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp
index 8f758ce..0c88d0f 100644
--- a/src/dawn/native/Toggles.cpp
+++ b/src/dawn/native/Toggles.cpp
@@ -558,6 +558,12 @@
"Using D3D12_BLEND_DEST_ALPHA as source blend factor for both color and alpha blending "
"doesn't work correctly on the D3D12 backend using Intel Gen9 or Gen9.5 GPUs.",
"https://crbug.com/dawn/1579", ToggleStage::Device}},
+ {Toggle::ClearColorWithDraw,
+ {"clear_color_with_draw",
+ "Use Draw instead of ClearRenderTargetView() to clear color attachments. On D3D11, "
+ "ClearRenderTargetView() does not always clear texture correctly.",
+ "https://crbug.com/chromium/329702368", ToggleStage::Device}},
+
// Comment to separate the }} so it is clearer what to copy-paste to add a toggle.
}};
} // anonymous namespace
diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h
index 2527ef2..35d1006 100644
--- a/src/dawn/native/Toggles.h
+++ b/src/dawn/native/Toggles.h
@@ -138,6 +138,8 @@
NoWorkaroundIndirectBaseVertexNotApplied,
NoWorkaroundDstAlphaAsSrcBlendFactorForBothColorAndAlphaDoesNotWork,
+ ClearColorWithDraw,
+
EnumCount,
InvalidEnum = EnumCount,
};
diff --git a/src/dawn/native/d3d11/CommandBufferD3D11.cpp b/src/dawn/native/d3d11/CommandBufferD3D11.cpp
index eaa105f..e337ed8 100644
--- a/src/dawn/native/d3d11/CommandBufferD3D11.cpp
+++ b/src/dawn/native/d3d11/CommandBufferD3D11.cpp
@@ -559,13 +559,14 @@
// Hold ID3D11RenderTargetView ComPtr to make attachments alive.
PerColorAttachment<ID3D11RenderTargetView*> d3d11RenderTargetViews = {};
ColorAttachmentIndex attachmentCount{};
+ bool clearWithDraw = GetDevice()->IsToggleEnabled(Toggle::ClearColorWithDraw);
// TODO(dawn:1815): Shrink the sparse attachments to accommodate more UAVs.
for (auto i : IterateBitSet(renderPass->attachmentState->GetColorAttachmentsMask())) {
TextureView* colorTextureView = ToBackend(renderPass->colorAttachments[i].view.Get());
DAWN_TRY_ASSIGN(d3d11RenderTargetViews[i],
colorTextureView->GetOrCreateD3D11RenderTargetView(
renderPass->colorAttachments[i].depthSlice));
- if (renderPass->colorAttachments[i].loadOp == wgpu::LoadOp::Clear) {
+ if (!clearWithDraw && renderPass->colorAttachments[i].loadOp == wgpu::LoadOp::Clear) {
std::array<float, 4> clearColor =
ConvertToFloatColor(renderPass->colorAttachments[i].clearColor);
d3d11DeviceContext->ClearRenderTargetView(d3d11RenderTargetViews[i], clearColor.data());
diff --git a/src/dawn/tests/end2end/RenderPassLoadOpTests.cpp b/src/dawn/tests/end2end/RenderPassLoadOpTests.cpp
index 4628f1d..eff93a6 100644
--- a/src/dawn/tests/end2end/RenderPassLoadOpTests.cpp
+++ b/src/dawn/tests/end2end/RenderPassLoadOpTests.cpp
@@ -752,6 +752,7 @@
DAWN_INSTANTIATE_TEST(RenderPassLoadOpTests,
D3D11Backend(),
+ D3D11Backend({"clear_color_with_draw"}),
D3D12Backend(),
MetalBackend(),
OpenGLBackend(),
