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>

src/dawn_native/CommandBuffer.cpp

@@ -175,4 +175,18 @@ namespace dawn_native {
 
         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

src/dawn_native/CommandBuffer.h

@@ -51,6 +51,9 @@ namespace dawn_native {
 
     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_

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -913,13 +913,23 @@ namespace dawn_native { namespace opengl {
                 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) {

src/dawn_native/vulkan/CommandBufferVk.cpp

@@ -234,10 +234,27 @@ namespace dawn_native { namespace vulkan {
 
                     attachments[attachmentCount] = view->GetHandle();
 
-                    clearValues[attachmentCount].color.float32[0] = attachmentInfo.clearColor.r;
+                    const Format& attachmentFormat = view->GetFormat();
-                    clearValues[attachmentCount].color.float32[1] = attachmentInfo.clearColor.g;
+                    if (attachmentFormat.HasComponentType(Format::Type::Float)) {
-                    clearValues[attachmentCount].color.float32[2] = attachmentInfo.clearColor.b;
+                        clearValues[attachmentCount].color.float32[0] = attachmentInfo.clearColor.r;
-                    clearValues[attachmentCount].color.float32[3] = attachmentInfo.clearColor.a;
+                        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++;
                 }

src/tests/end2end/RenderPassLoadOpTests.cpp

@@ -95,6 +95,44 @@ class RenderPassLoadOpTests : public DawnTest {
         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 @@ TEST_P(RenderPassLoadOpTests, ColorClearThenLoadAndDraw) {
                             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(),