Fix clearing sint/uint color attachments on Vulkan and OpenGL
This patch fixes a bug on the clear of color attachments with signed or
unsigned integer formats on Vulkan and OpenGL by using the correct APIs
to set the clear color for signed/unsigned integer formats.
BUG=dawn:497
Change-Id: If1bc9858875e6384e71c15bb6770fbbb10045037
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/26041
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp
index e1446ab..8329e95 100644
--- a/src/dawn_native/CommandBuffer.cpp
+++ b/src/dawn_native/CommandBuffer.cpp
@@ -175,4 +175,18 @@
return true;
}
+
+ std::array<int32_t, 4> ConvertToSignedIntegerColor(dawn_native::Color color) {
+ const std::array<int32_t, 4> outputValue = {
+ static_cast<int32_t>(color.r), static_cast<int32_t>(color.g),
+ static_cast<int32_t>(color.b), static_cast<int32_t>(color.a)};
+ return outputValue;
+ }
+
+ std::array<uint32_t, 4> ConvertToUnsignedIntegerColor(dawn_native::Color color) {
+ const std::array<uint32_t, 4> outputValue = {
+ static_cast<uint32_t>(color.r), static_cast<uint32_t>(color.g),
+ static_cast<uint32_t>(color.b), static_cast<uint32_t>(color.a)};
+ return outputValue;
+ }
} // namespace dawn_native
diff --git a/src/dawn_native/CommandBuffer.h b/src/dawn_native/CommandBuffer.h
index 375f102..11076a1 100644
--- a/src/dawn_native/CommandBuffer.h
+++ b/src/dawn_native/CommandBuffer.h
@@ -51,6 +51,9 @@
bool IsFullBufferOverwrittenInTextureToBufferCopy(const CopyTextureToBufferCmd* copy);
+ std::array<int32_t, 4> ConvertToSignedIntegerColor(dawn_native::Color color);
+ std::array<uint32_t, 4> ConvertToUnsignedIntegerColor(dawn_native::Color color);
+
} // namespace dawn_native
#endif // DAWNNATIVE_COMMANDBUFFER_H_
diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp
index 8bb099c..c15b943 100644
--- a/src/dawn_native/opengl/CommandBufferGL.cpp
+++ b/src/dawn_native/opengl/CommandBufferGL.cpp
@@ -913,13 +913,23 @@
auto* attachmentInfo = &renderPass->colorAttachments[i];
// Load op - color
- // TODO(cwallez@chromium.org): Choose the clear function depending on the
- // componentType: things work for now because the clear color is always a float, but
- // when that's fixed will lose precision on integer formats when converting to
- // float.
if (attachmentInfo->loadOp == wgpu::LoadOp::Clear) {
gl.ColorMaski(i, true, true, true, true);
- gl.ClearBufferfv(GL_COLOR, i, &attachmentInfo->clearColor.r);
+
+ const Format& attachmentFormat = attachmentInfo->view->GetFormat();
+ if (attachmentFormat.HasComponentType(Format::Type::Float)) {
+ gl.ClearBufferfv(GL_COLOR, i, &attachmentInfo->clearColor.r);
+ } else if (attachmentFormat.HasComponentType(Format::Type::Uint)) {
+ const std::array<uint32_t, 4> appliedClearColor =
+ ConvertToUnsignedIntegerColor(attachmentInfo->clearColor);
+ gl.ClearBufferuiv(GL_COLOR, i, appliedClearColor.data());
+ } else if (attachmentFormat.HasComponentType(Format::Type::Sint)) {
+ const std::array<int32_t, 4> appliedClearColor =
+ ConvertToSignedIntegerColor(attachmentInfo->clearColor);
+ gl.ClearBufferiv(GL_COLOR, i, appliedClearColor.data());
+ } else {
+ UNREACHABLE();
+ }
}
if (attachmentInfo->storeOp == wgpu::StoreOp::Clear) {
diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp
index 9e281dc..8315b1b 100644
--- a/src/dawn_native/vulkan/CommandBufferVk.cpp
+++ b/src/dawn_native/vulkan/CommandBufferVk.cpp
@@ -234,10 +234,27 @@
attachments[attachmentCount] = view->GetHandle();
- clearValues[attachmentCount].color.float32[0] = attachmentInfo.clearColor.r;
- clearValues[attachmentCount].color.float32[1] = attachmentInfo.clearColor.g;
- clearValues[attachmentCount].color.float32[2] = attachmentInfo.clearColor.b;
- clearValues[attachmentCount].color.float32[3] = attachmentInfo.clearColor.a;
+ const Format& attachmentFormat = view->GetFormat();
+ if (attachmentFormat.HasComponentType(Format::Type::Float)) {
+ clearValues[attachmentCount].color.float32[0] = attachmentInfo.clearColor.r;
+ clearValues[attachmentCount].color.float32[1] = attachmentInfo.clearColor.g;
+ clearValues[attachmentCount].color.float32[2] = attachmentInfo.clearColor.b;
+ clearValues[attachmentCount].color.float32[3] = attachmentInfo.clearColor.a;
+ } else if (attachmentFormat.HasComponentType(Format::Type::Uint)) {
+ const std::array<uint32_t, 4> appliedClearColor =
+ ConvertToUnsignedIntegerColor(attachmentInfo.clearColor);
+ for (uint32_t i = 0; i < 4; ++i) {
+ clearValues[attachmentCount].color.uint32[i] = appliedClearColor[i];
+ }
+ } else if (attachmentFormat.HasComponentType(Format::Type::Sint)) {
+ const std::array<int32_t, 4> appliedClearColor =
+ ConvertToSignedIntegerColor(attachmentInfo.clearColor);
+ for (uint32_t i = 0; i < 4; ++i) {
+ clearValues[attachmentCount].color.int32[i] = appliedClearColor[i];
+ }
+ } else {
+ UNREACHABLE();
+ }
attachmentCount++;
}
diff --git a/src/tests/end2end/RenderPassLoadOpTests.cpp b/src/tests/end2end/RenderPassLoadOpTests.cpp
index 011c6e0..42a5c79 100644
--- a/src/tests/end2end/RenderPassLoadOpTests.cpp
+++ b/src/tests/end2end/RenderPassLoadOpTests.cpp
@@ -95,6 +95,44 @@
blueQuad = DrawQuad(device, vsSource, fsSource);
}
+ template <class T>
+ void TestIntegerClearColor(wgpu::TextureFormat format,
+ const wgpu::Color& clearColor,
+ const std::array<T, 4>& expectedPixelValue) {
+ constexpr wgpu::Extent3D kTextureSize = {1, 1, 1};
+
+ wgpu::TextureDescriptor textureDescriptor;
+ textureDescriptor.dimension = wgpu::TextureDimension::e2D;
+ textureDescriptor.size = kTextureSize;
+ textureDescriptor.usage =
+ wgpu::TextureUsage::OutputAttachment | wgpu::TextureUsage::CopySrc;
+ textureDescriptor.format = format;
+ wgpu::Texture texture = device.CreateTexture(&textureDescriptor);
+
+ utils::ComboRenderPassDescriptor renderPassDescriptor({texture.CreateView()});
+ renderPassDescriptor.cColorAttachments[0].clearColor = clearColor;
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder renderPass = encoder.BeginRenderPass(&renderPassDescriptor);
+ renderPass.EndPass();
+
+ const uint64_t bufferSize = sizeof(T) * expectedPixelValue.size();
+ wgpu::BufferDescriptor bufferDescriptor;
+ bufferDescriptor.size = bufferSize;
+ bufferDescriptor.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst;
+ wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor);
+
+ wgpu::TextureCopyView textureCopyView = utils::CreateTextureCopyView(texture, 0, {0, 0, 0});
+ wgpu::BufferCopyView bufferCopyView =
+ utils::CreateBufferCopyView(buffer, 0, kTextureBytesPerRowAlignment, 0);
+ encoder.CopyTextureToBuffer(&textureCopyView, &bufferCopyView, &kTextureSize);
+
+ wgpu::CommandBuffer commandBuffer = encoder.Finish();
+ queue.Submit(1, &commandBuffer);
+
+ EXPECT_BUFFER_U32_RANGE_EQ(reinterpret_cast<const uint32_t*>(expectedPixelValue.data()),
+ buffer, 0, bufferSize / sizeof(uint32_t));
+ }
+
wgpu::Texture renderTarget;
wgpu::TextureView renderTargetView;
@@ -147,6 +185,57 @@
0, 0);
}
+// Test clearing a color attachment with signed and unsigned integer formats.
+TEST_P(RenderPassLoadOpTests, LoadOpClearOnIntegerFormats) {
+ // RGBA8Uint
+ {
+ constexpr wgpu::Color kClearColor = {2.f, 3.3f, 254.8f, 255.0f};
+ constexpr std::array<uint8_t, 4> kExpectedPixelValue = {2, 3, 254, 255};
+ TestIntegerClearColor<uint8_t>(wgpu::TextureFormat::RGBA8Uint, kClearColor,
+ kExpectedPixelValue);
+ }
+
+ // RGBA8Sint
+ {
+ constexpr wgpu::Color kClearColor = {2.f, -3.3f, 126.8f, -128.0f};
+ constexpr std::array<int8_t, 4> kExpectedPixelValue = {2, -3, 126, -128};
+ TestIntegerClearColor<int8_t>(wgpu::TextureFormat::RGBA8Sint, kClearColor,
+ kExpectedPixelValue);
+ }
+
+ // RGBA16Uint
+ {
+ constexpr wgpu::Color kClearColor = {2.f, 3.3f, 512.7f, 65535.f};
+ constexpr std::array<uint16_t, 4> kExpectedPixelValue = {2, 3, 512, 65535u};
+ TestIntegerClearColor<uint16_t>(wgpu::TextureFormat::RGBA16Uint, kClearColor,
+ kExpectedPixelValue);
+ }
+
+ // RGBA16Sint
+ {
+ constexpr wgpu::Color kClearColor = {2.f, -3.3f, 32767.8f, -32768.0f};
+ constexpr std::array<int16_t, 4> kExpectedPixelValue = {2, -3, 32767, -32768};
+ TestIntegerClearColor<int16_t>(wgpu::TextureFormat::RGBA16Sint, kClearColor,
+ kExpectedPixelValue);
+ }
+
+ // RGBA32Uint
+ {
+ constexpr wgpu::Color kClearColor = {2.f, 3.3f, 65534.8f, 65537.f};
+ constexpr std::array<uint32_t, 4> kExpectedPixelValue = {2, 3, 65534, 65537};
+ TestIntegerClearColor<uint32_t>(wgpu::TextureFormat::RGBA32Uint, kClearColor,
+ kExpectedPixelValue);
+ }
+
+ // RGBA32Sint
+ {
+ constexpr wgpu::Color kClearColor = {2.f, -3.3f, 65534.8f, -65537.f};
+ constexpr std::array<int32_t, 4> kExpectedPixelValue = {2, -3, 65534, -65537};
+ TestIntegerClearColor<int32_t>(wgpu::TextureFormat::RGBA32Sint, kClearColor,
+ kExpectedPixelValue);
+ }
+}
+
DAWN_INSTANTIATE_TEST(RenderPassLoadOpTests,
D3D12Backend(),
MetalBackend(),
