From f00c68a216d6366ae3cdd6adb3e2bbf7af819be6 Mon Sep 17 00:00:00 2001 From: Juanmi Date: Thu, 5 Aug 2021 22:55:09 +0000 Subject: [PATCH] Adding APICopyTextureToTextureInternal This CL adds an internal method to copy textures, that will disregard the need of having CopySrc usage. This CL adds an intermediate helped method to be used by both APIContextTextureToTexture and APIContextTextureToTextureInternal. Fixed: dawn:1052 Change-Id: Icd28e0ef58ecfaa412eefe8c95e41cd2298a9acc Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/58440 Reviewed-by: Kai Ninomiya Commit-Queue: Juanmi Huertas --- dawn.json | 8 +++ src/dawn_native/CommandEncoder.cpp | 26 +++++++- src/dawn_native/CommandEncoder.h | 11 ++++ src/dawn_native/CommandValidation.cpp | 9 +++ src/dawn_native/CommandValidation.h | 2 + src/tests/end2end/CopyTests.cpp | 60 ++++++++++++++----- .../InternalUsageValidationTests.cpp | 24 ++++++++ 7 files changed, 123 insertions(+), 17 deletions(-) diff --git a/dawn.json b/dawn.json index 8d5b24dadf..60f0f7d09f 100644 --- a/dawn.json +++ b/dawn.json @@ -426,6 +426,14 @@ {"name": "copy size", "type": "extent 3D", "annotation": "const*"} ] }, + { + "name": "copy texture to texture internal", + "args": [ + {"name": "source", "type": "image copy texture", "annotation": "const*"}, + {"name": "destination", "type": "image copy texture", "annotation": "const*"}, + {"name": "copy size", "type": "extent 3D", "annotation": "const*"} + ] + }, { "name": "inject validation error", "args": [ diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index 58c4a6dce3..431c7c2f9d 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -778,6 +778,19 @@ namespace dawn_native { void CommandEncoder::APICopyTextureToTexture(const ImageCopyTexture* source, const ImageCopyTexture* destination, const Extent3D* copySize) { + APICopyTextureToTextureHelper(source, destination, copySize); + } + + void CommandEncoder::APICopyTextureToTextureInternal(const ImageCopyTexture* source, + const ImageCopyTexture* destination, + const Extent3D* copySize) { + APICopyTextureToTextureHelper(source, destination, copySize); + } + + template + void CommandEncoder::APICopyTextureToTextureHelper(const ImageCopyTexture* source, + const ImageCopyTexture* destination, + const Extent3D* copySize) { mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { if (GetDevice()->IsValidationEnabled()) { DAWN_TRY(GetDevice()->ValidateObject(source->texture)); @@ -792,8 +805,17 @@ namespace dawn_native { DAWN_TRY(ValidateTextureCopyRange(GetDevice(), *source, *copySize)); DAWN_TRY(ValidateTextureCopyRange(GetDevice(), *destination, *copySize)); - DAWN_TRY(ValidateCanUseAs(source->texture, wgpu::TextureUsage::CopySrc)); - DAWN_TRY(ValidateCanUseAs(destination->texture, wgpu::TextureUsage::CopyDst)); + // For internal usages (CopyToCopyInternal) we don't care if the user has added + // CopySrc as a usage for this texture, but we will always add it internally. + if (Internal) { + DAWN_TRY( + ValidateInternalCanUseAs(source->texture, wgpu::TextureUsage::CopySrc)); + DAWN_TRY(ValidateInternalCanUseAs(destination->texture, + wgpu::TextureUsage::CopyDst)); + } else { + DAWN_TRY(ValidateCanUseAs(source->texture, wgpu::TextureUsage::CopySrc)); + DAWN_TRY(ValidateCanUseAs(destination->texture, wgpu::TextureUsage::CopyDst)); + } mTopLevelTextures.insert(source->texture); mTopLevelTextures.insert(destination->texture); diff --git a/src/dawn_native/CommandEncoder.h b/src/dawn_native/CommandEncoder.h index 13678f17ce..776e1d2831 100644 --- a/src/dawn_native/CommandEncoder.h +++ b/src/dawn_native/CommandEncoder.h @@ -54,6 +54,9 @@ namespace dawn_native { void APICopyTextureToTexture(const ImageCopyTexture* source, const ImageCopyTexture* destination, const Extent3D* copySize); + void APICopyTextureToTextureInternal(const ImageCopyTexture* source, + const ImageCopyTexture* destination, + const Extent3D* copySize); void APIInjectValidationError(const char* message); void APIInsertDebugMarker(const char* groupLabel); @@ -73,6 +76,14 @@ namespace dawn_native { ResultOrError> FinishInternal( const CommandBufferDescriptor* descriptor); + // Helper to be able to implement both APICopyTextureToTexture and + // APICopyTextureToTextureInternal. The only difference between both + // copies, is that the Internal one will also check internal usage. + template + void APICopyTextureToTextureHelper(const ImageCopyTexture* source, + const ImageCopyTexture* destination, + const Extent3D* copySize); + MaybeError ValidateFinish() const; EncodingContext mEncodingContext; diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index 9654b2a5ea..d61a61976f 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -421,6 +421,15 @@ namespace dawn_native { return {}; } + MaybeError ValidateInternalCanUseAs(const TextureBase* texture, wgpu::TextureUsage usage) { + ASSERT(wgpu::HasZeroOrOneBits(usage)); + if (!(texture->GetInternalUsage() & usage)) { + return DAWN_VALIDATION_ERROR("texture doesn't have the required usage."); + } + + return {}; + } + MaybeError ValidateCanUseAs(const BufferBase* buffer, wgpu::BufferUsage usage) { ASSERT(wgpu::HasZeroOrOneBits(usage)); if (!(buffer->GetUsage() & usage)) { diff --git a/src/dawn_native/CommandValidation.h b/src/dawn_native/CommandValidation.h index a822d056c3..a87e956cbb 100644 --- a/src/dawn_native/CommandValidation.h +++ b/src/dawn_native/CommandValidation.h @@ -71,6 +71,8 @@ namespace dawn_native { MaybeError ValidateCanUseAs(const TextureBase* texture, wgpu::TextureUsage usage); + MaybeError ValidateInternalCanUseAs(const TextureBase* texture, wgpu::TextureUsage usage); + MaybeError ValidateCanUseAs(const BufferBase* buffer, wgpu::BufferUsage usage); } // namespace dawn_native diff --git a/src/tests/end2end/CopyTests.cpp b/src/tests/end2end/CopyTests.cpp index 096f5df1b2..43bc5caea2 100644 --- a/src/tests/end2end/CopyTests.cpp +++ b/src/tests/end2end/CopyTests.cpp @@ -26,7 +26,7 @@ constexpr uint32_t kStrideComputeDefault = 0xFFFF'FFFEul; constexpr wgpu::TextureFormat kDefaultFormat = wgpu::TextureFormat::RGBA8Unorm; -class CopyTests : public DawnTest { +class CopyTests { protected: struct TextureSpec { wgpu::TextureFormat format = kDefaultFormat; @@ -128,7 +128,7 @@ class CopyTests : public DawnTest { } }; -class CopyTests_T2B : public CopyTests { +class CopyTests_T2B : public CopyTests, public DawnTest { protected: void DoTest(const TextureSpec& textureSpec, const BufferSpec& bufferSpec, @@ -234,7 +234,7 @@ class CopyTests_T2B : public CopyTests { } }; -class CopyTests_B2T : public CopyTests { +class CopyTests_B2T : public CopyTests, public DawnTest { protected: static void FillBufferData(RGBA8* data, size_t count) { for (size_t i = 0; i < count; ++i) { @@ -322,8 +322,19 @@ class CopyTests_B2T : public CopyTests { } }; -class CopyTests_T2T : public CopyTests { +namespace { + // The CopyTests Texture to Texture in this class will validate both CopyTextureToTexture and + // CopyTextureToTextureInternal. + using UsageCopySrc = bool; + DAWN_TEST_PARAM_STRUCT(CopyTestsParams, UsageCopySrc) +} // namespace + +class CopyTests_T2T : public CopyTests, public DawnTestWithParams { protected: + std::vector GetRequiredExtensions() override { + return {"dawn-internal-usages"}; + } + void DoTest(const TextureSpec& srcSpec, const TextureSpec& dstSpec, const wgpu::Extent3D& copySize, @@ -338,6 +349,12 @@ class CopyTests_T2T : public CopyTests { wgpu::TextureDimension srcDimension, wgpu::TextureDimension dstDimension, bool copyWithinSameTexture = false) { + const bool usageCopySrc = GetParam().mUsageCopySrc; + // If we do this test with a CopyWithinSameTexture, it will need to have usageCopySrc in the + // public usage of the texture as it will later use a CopyTextureToBuffer, that needs the + // public usage of it. + DAWN_TEST_UNSUPPORTED_IF(!usageCopySrc && copyWithinSameTexture); + ASSERT_EQ(srcSpec.format, dstSpec.format); const wgpu::TextureFormat format = srcSpec.format; @@ -347,7 +364,17 @@ class CopyTests_T2T : public CopyTests { srcDescriptor.sampleCount = 1; srcDescriptor.format = format; srcDescriptor.mipLevelCount = srcSpec.levelCount; - srcDescriptor.usage = wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst; + srcDescriptor.usage = wgpu::TextureUsage::CopyDst; + // This test will have two versions, one where we check the normal CopyToCopy, and for that + // we will add the CopySrc usage, and the one where we test the CopyToCopyInternal, and then + // we have to add the CopySrc to the Internal usage. + wgpu::DawnTextureInternalUsageDescriptor internalDesc = {}; + srcDescriptor.nextInChain = &internalDesc; + if (usageCopySrc) { + srcDescriptor.usage |= wgpu::TextureUsage::CopySrc; + } else { + internalDesc.internalUsage = wgpu::TextureUsage::CopySrc; + } wgpu::Texture srcTexture = device.CreateTexture(&srcDescriptor); wgpu::Texture dstTexture; @@ -394,7 +421,13 @@ class CopyTests_T2T : public CopyTests { utils::CreateImageCopyTexture(srcTexture, srcSpec.copyLevel, srcSpec.copyOrigin); wgpu::ImageCopyTexture dstImageCopyTexture = utils::CreateImageCopyTexture(dstTexture, dstSpec.copyLevel, dstSpec.copyOrigin); - encoder.CopyTextureToTexture(&srcImageCopyTexture, &dstImageCopyTexture, ©Size); + + if (usageCopySrc) { + encoder.CopyTextureToTexture(&srcImageCopyTexture, &dstImageCopyTexture, ©Size); + } else { + encoder.CopyTextureToTextureInternal(&srcImageCopyTexture, &dstImageCopyTexture, + ©Size); + } // Create an output buffer and use it to completely populate the subresources of the dst // texture that will be copied to at the given mip level. @@ -2324,15 +2357,12 @@ TEST_P(CopyTests_T2T, Texture3DMipUnaligned) { } } -DAWN_INSTANTIATE_TEST( - CopyTests_T2T, - D3D12Backend(), - D3D12Backend( - {"use_temp_buffer_in_small_format_texture_to_texture_copy_from_greater_to_less_mip_level"}), - MetalBackend(), - OpenGLBackend(), - OpenGLESBackend(), - VulkanBackend()); +DAWN_INSTANTIATE_TEST_P(CopyTests_T2T, + {D3D12Backend(), + D3D12Backend({"use_temp_buffer_in_small_format_texture_to_texture_copy_" + "from_greater_to_less_mip_level"}), + MetalBackend(), OpenGLBackend(), OpenGLESBackend(), VulkanBackend()}, + {true, false}); static constexpr uint64_t kSmallBufferSize = 4; static constexpr uint64_t kLargeBufferSize = 1 << 16; diff --git a/src/tests/unittests/validation/InternalUsageValidationTests.cpp b/src/tests/unittests/validation/InternalUsageValidationTests.cpp index a05358551e..e640e633f7 100644 --- a/src/tests/unittests/validation/InternalUsageValidationTests.cpp +++ b/src/tests/unittests/validation/InternalUsageValidationTests.cpp @@ -115,6 +115,7 @@ TEST_F(TextureInternalUsageValidationTest, UsageValidation) { // Test that internal usage does not add to the validated usage // for command encoding +// This test also test the internal copy TEST_F(TextureInternalUsageValidationTest, CommandValidation) { wgpu::TextureDescriptor textureDesc = {}; textureDesc.size = {1, 1}; @@ -156,4 +157,27 @@ TEST_F(TextureInternalUsageValidationTest, CommandValidation) { encoder.CopyTextureToTexture(&srcImageCopyTexture, &dstImageCopyTexture, &extent3D); ASSERT_DEVICE_ERROR(encoder.Finish()); } + + // Control with internal copy: src -> dst + { + wgpu::ImageCopyTexture srcImageCopyTexture = utils::CreateImageCopyTexture(src, 0, {0, 0}); + wgpu::ImageCopyTexture dstImageCopyTexture = utils::CreateImageCopyTexture(dst, 0, {0, 0}); + wgpu::Extent3D extent3D = {1, 1}; + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.CopyTextureToTextureInternal(&srcImageCopyTexture, &dstImageCopyTexture, &extent3D); + encoder.Finish(); + } + + // Valid with internal copy: src internal -> dst + { + wgpu::ImageCopyTexture srcImageCopyTexture = + utils::CreateImageCopyTexture(srcInternal, 0, {0, 0}); + wgpu::ImageCopyTexture dstImageCopyTexture = utils::CreateImageCopyTexture(dst, 0, {0, 0}); + wgpu::Extent3D extent3D = {1, 1}; + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.CopyTextureToTextureInternal(&srcImageCopyTexture, &dstImageCopyTexture, &extent3D); + encoder.Finish(); + } }