Allow CopyBufferToTexture with Depth16Unorm
Removes the validation logic that was blocking this copy from being
valid and update tests to ensure that it works properly.
Copy logic worked with this format as-is, so no updates were necessary.
Bug: dawn:1251
Change-Id: Id0ca503ed0f82e79e75f63ad6870389f7f647d22
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/81024
Reviewed-by: Shrek Shao <shrekshao@google.com>
Reviewed-by: Loko Kung <lokokung@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/native/CommandValidation.cpp b/src/dawn/native/CommandValidation.cpp
index 4fd7a60..44fbdf8 100644
--- a/src/dawn/native/CommandValidation.cpp
+++ b/src/dawn/native/CommandValidation.cpp
@@ -386,8 +386,17 @@
MaybeError ValidateLinearToDepthStencilCopyRestrictions(const ImageCopyTexture& dst) {
Aspect aspectUsed;
DAWN_TRY_ASSIGN(aspectUsed, SingleAspectUsedByImageCopyTexture(dst));
- DAWN_INVALID_IF(aspectUsed == Aspect::Depth, "Cannot copy into the depth aspect of %s.",
- dst.texture);
+
+ const Format& format = dst.texture->GetFormat();
+ switch (format.format) {
+ case wgpu::TextureFormat::Depth16Unorm:
+ return {};
+ default:
+ DAWN_INVALID_IF(aspectUsed == Aspect::Depth,
+ "Cannot copy into the depth aspect of %s with format %s.",
+ dst.texture, format.format);
+ break;
+ }
return {};
}
diff --git a/src/dawn/tests/end2end/DepthStencilCopyTests.cpp b/src/dawn/tests/end2end/DepthStencilCopyTests.cpp
index 2dfd8e5..f037555 100644
--- a/src/dawn/tests/end2end/DepthStencilCopyTests.cpp
+++ b/src/dawn/tests/end2end/DepthStencilCopyTests.cpp
@@ -31,6 +31,10 @@
wgpu::TextureFormat::Depth32Float,
wgpu::TextureFormat::Depth32FloatStencil8,
};
+
+ constexpr std::array<wgpu::TextureFormat, 1> kValidDepthCopyFromBufferFormats = {
+ wgpu::TextureFormat::Depth16Unorm,
+ };
} // namespace
class DepthStencilCopyTests : public DawnTestWithParams<DepthStencilCopyTestParams> {
@@ -240,6 +244,16 @@
return dst;
}
+ uint32_t BufferSizeForTextureCopy(
+ uint32_t width,
+ uint32_t height,
+ uint32_t depth,
+ wgpu::TextureFormat format = wgpu::TextureFormat::RGBA8Unorm) {
+ uint32_t bytesPerPixel = utils::GetTexelBlockSizeInBytes(format);
+ uint32_t bytesPerRow = Align(width * bytesPerPixel, kTextureBytesPerRowAlignment);
+ return (bytesPerRow * (height - 1) + width * bytesPerPixel) * depth;
+ }
+
wgpu::ShaderModule mVertexModule;
private:
@@ -522,6 +536,72 @@
}
}
+class DepthCopyFromBufferTests : public DepthStencilCopyTests {};
+
+// Test copying the depth-only aspect from a buffer.
+TEST_P(DepthCopyFromBufferTests, BufferToDepthAspect) {
+ // TODO(crbug.com/dawn/1237): Depth16Unorm test failed on OpenGL and OpenGLES which says
+ // Invalid format and type combination in glReadPixels
+ DAWN_TEST_UNSUPPORTED_IF(GetParam().mTextureFormat == wgpu::TextureFormat::Depth16Unorm &&
+ (IsOpenGL() || IsOpenGLES()));
+
+ constexpr uint32_t kWidth = 8;
+ constexpr uint32_t kHeight = 1;
+
+ wgpu::Texture destTexture =
+ CreateTexture(kWidth, kHeight, wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::CopySrc);
+
+ wgpu::BufferDescriptor descriptor;
+ descriptor.size = BufferSizeForTextureCopy(kWidth, kHeight, 1, GetParam().mTextureFormat);
+ descriptor.usage = wgpu::BufferUsage::CopySrc;
+ descriptor.mappedAtCreation = true;
+ wgpu::Buffer srcBuffer = device.CreateBuffer(&descriptor);
+
+ wgpu::ImageCopyBuffer imageCopyBuffer =
+ utils::CreateImageCopyBuffer(srcBuffer, 0, 256, kHeight);
+ wgpu::ImageCopyTexture imageCopyTexture =
+ utils::CreateImageCopyTexture(destTexture, 0, {0, 0, 0}, wgpu::TextureAspect::DepthOnly);
+ wgpu::Extent3D extent = {kWidth, kHeight, 1};
+
+ constexpr float kInitDepth = 0.2f;
+
+ // This expectation is the test as it performs the CopyTextureToBuffer.
+ if (GetParam().mTextureFormat == wgpu::TextureFormat::Depth16Unorm) {
+ uint16_t expected = FloatToUnorm<uint16_t>(kInitDepth);
+ std::vector<uint16_t> expectedData = {
+ 0, 0, expected, expected, 0, 0, expected, expected,
+ };
+ size_t expectedSize = expectedData.size() * sizeof(uint16_t);
+
+ memcpy(srcBuffer.GetMappedRange(0, expectedSize), expectedData.data(), expectedSize);
+ srcBuffer.Unmap();
+
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyBufferToTexture(&imageCopyBuffer, &imageCopyTexture, &extent);
+ wgpu::CommandBuffer commands = encoder.Finish();
+ queue.Submit(1, &commands);
+
+ EXPECT_TEXTURE_EQ(expectedData.data(), destTexture, {0, 0}, {kWidth, kHeight}, 0,
+ wgpu::TextureAspect::DepthOnly);
+ } else {
+ std::vector<float> expectedData = {
+ 0.0, 0.0, kInitDepth, kInitDepth, 0.0, 0.0, kInitDepth, kInitDepth,
+ };
+ size_t expectedSize = expectedData.size() * sizeof(float);
+
+ memcpy(srcBuffer.GetMappedRange(0, expectedSize), expectedData.data(), expectedSize);
+ srcBuffer.Unmap();
+
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyBufferToTexture(&imageCopyBuffer, &imageCopyTexture, &extent);
+ wgpu::CommandBuffer commands = encoder.Finish();
+ queue.Submit(1, &commands);
+
+ EXPECT_TEXTURE_EQ(expectedData.data(), destTexture, {0, 0}, {kWidth, kHeight}, 0,
+ wgpu::TextureAspect::DepthOnly);
+ }
+}
+
class StencilCopyTests : public DepthStencilCopyTests {};
// Test copying the stencil-only aspect into a buffer.
@@ -696,6 +776,12 @@
std::vector<wgpu::TextureFormat>(kValidDepthCopyTextureFormats.begin(),
kValidDepthCopyTextureFormats.end()));
+DAWN_INSTANTIATE_TEST_P(DepthCopyFromBufferTests,
+ {D3D12Backend(), MetalBackend(), OpenGLBackend(), OpenGLESBackend(),
+ VulkanBackend()},
+ std::vector<wgpu::TextureFormat>(kValidDepthCopyFromBufferFormats.begin(),
+ kValidDepthCopyFromBufferFormats.end()));
+
DAWN_INSTANTIATE_TEST_P(StencilCopyTests,
{D3D12Backend(), MetalBackend(), OpenGLBackend(), OpenGLESBackend(),
VulkanBackend()},
diff --git a/src/dawn/tests/unittests/validation/CopyCommandsValidationTests.cpp b/src/dawn/tests/unittests/validation/CopyCommandsValidationTests.cpp
index 8257171..a7d26f1 100644
--- a/src/dawn/tests/unittests/validation/CopyCommandsValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/CopyCommandsValidationTests.cpp
@@ -715,8 +715,10 @@
// Test B2T copies with incorrect buffer offset usage for depth-stencil texture
TEST_F(CopyCommandTest_B2T, IncorrectBufferOffsetForDepthStencilTexture) {
// TODO(dawn:570, dawn:666): List other valid parameters after missing texture formats
- // are implemented, e.g. Stencil8 and depth16unorm.
- std::array<std::tuple<wgpu::TextureFormat, wgpu::TextureAspect>, 3> params = {
+ // are implemented, e.g. Stencil8.
+ std::array<std::tuple<wgpu::TextureFormat, wgpu::TextureAspect>, 5> params = {
+ std::make_tuple(wgpu::TextureFormat::Depth16Unorm, wgpu::TextureAspect::DepthOnly),
+ std::make_tuple(wgpu::TextureFormat::Depth16Unorm, wgpu::TextureAspect::All),
std::make_tuple(wgpu::TextureFormat::Depth24PlusStencil8, wgpu::TextureAspect::StencilOnly),
std::make_tuple(wgpu::TextureFormat::Depth24UnormStencil8,
wgpu::TextureAspect::StencilOnly),
@@ -866,12 +868,35 @@
{0, 0, 0}, {2, 2, 1});
}
-// Test it is invalid to copy to a depth texture
+// Test whether or not it is valid to copy to a depth texture
TEST_F(CopyCommandTest_B2T, CopyToDepthAspect) {
uint64_t bufferSize = BufferSizeForTextureCopy(16, 16, 1, wgpu::TextureFormat::Depth32Float);
wgpu::Buffer source = CreateBuffer(bufferSize, wgpu::BufferUsage::CopySrc);
- for (wgpu::TextureFormat format : utils::kDepthFormats) {
+ constexpr std::array<wgpu::TextureFormat, 1> kAllowBufferToDepthCopyFormats = {
+ wgpu::TextureFormat::Depth16Unorm};
+
+ for (wgpu::TextureFormat format : kAllowBufferToDepthCopyFormats) {
+ wgpu::Texture destination =
+ Create2DTexture(16, 16, 1, 1, format, wgpu::TextureUsage::CopyDst);
+
+ // Test it is valid to copy this format from a buffer into a depth texture
+ TestB2TCopy(utils::Expectation::Success, source, 0, 256, 16, destination, 0, {0, 0, 0},
+ {16, 16, 1}, wgpu::TextureAspect::DepthOnly);
+ if (utils::IsDepthOnlyFormat(format)) {
+ // Test "all" of a depth texture which is only the depth aspect.
+ TestB2TCopy(utils::Expectation::Success, source, 0, 256, 16, destination, 0, {0, 0, 0},
+ {16, 16, 1}, wgpu::TextureAspect::All);
+ }
+ }
+
+ constexpr std::array<wgpu::TextureFormat, 5> kDisallowBufferToDepthCopyFormats = {
+ wgpu::TextureFormat::Depth32Float, wgpu::TextureFormat::Depth24Plus,
+ wgpu::TextureFormat::Depth24PlusStencil8, wgpu::TextureFormat::Depth24UnormStencil8,
+ wgpu::TextureFormat::Depth32FloatStencil8,
+ };
+
+ for (wgpu::TextureFormat format : kDisallowBufferToDepthCopyFormats) {
wgpu::Texture destination =
Create2DTexture(16, 16, 1, 1, format, wgpu::TextureUsage::CopyDst);
@@ -1325,8 +1350,10 @@
// Test T2B copies with incorrect buffer offset usage for depth-stencil texture
TEST_F(CopyCommandTest_T2B, IncorrectBufferOffsetForDepthStencilTexture) {
// TODO(dawn:570, dawn:666): List other valid parameters after missing texture formats
- // are implemented, e.g. Stencil8 and depth16unorm.
- std::array<std::tuple<wgpu::TextureFormat, wgpu::TextureAspect>, 6> params = {
+ // are implemented, e.g. Stencil8.
+ std::array<std::tuple<wgpu::TextureFormat, wgpu::TextureAspect>, 8> params = {
+ std::make_tuple(wgpu::TextureFormat::Depth16Unorm, wgpu::TextureAspect::DepthOnly),
+ std::make_tuple(wgpu::TextureFormat::Depth16Unorm, wgpu::TextureAspect::All),
std::make_tuple(wgpu::TextureFormat::Depth24PlusStencil8, wgpu::TextureAspect::StencilOnly),
std::make_tuple(wgpu::TextureFormat::Depth32Float, wgpu::TextureAspect::DepthOnly),
std::make_tuple(wgpu::TextureFormat::Depth32Float, wgpu::TextureAspect::All),
