From c303bdf70523040c0425ab3166cb8fac6df5dcc3 Mon Sep 17 00:00:00 2001 From: "Yan,Shaobo" Date: Fri, 28 Oct 2022 15:35:28 +0000 Subject: [PATCH] Fix HandleAssertionFailure in ValidateCopyTextureForBrowser The assert failure is caused by the validation orders. Current validation order cannot ensure "destination" is valid when it is passed to "ValidateTextureToTextureCopyCommonRestrictions". This CL seperate "ValidateCopyForBrowserCommonRestrictions" to "ValidateCopyForBrowserDestination" and "ValidateCopyForBrowserOptions". Correcting the order and adding more comments. Bug: chromium:1379001 Change-Id: I9bdbd773659827d0056cd7c37e78ac02ce22451c Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/107560 Reviewed-by: Corentin Wallez Reviewed-by: Austin Eng Kokoro: Kokoro Commit-Queue: Austin Eng --- .../native/CopyTextureForBrowserHelper.cpp | 53 +++++++++++-------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/src/dawn/native/CopyTextureForBrowserHelper.cpp b/src/dawn/native/CopyTextureForBrowserHelper.cpp index fb585f9e85..5d63725a95 100644 --- a/src/dawn/native/CopyTextureForBrowserHelper.cpp +++ b/src/dawn/native/CopyTextureForBrowserHelper.cpp @@ -613,10 +613,10 @@ MaybeError DoCopyForBrowser(DeviceBase* device, return {}; } -MaybeError ValidateCopyForBrowserCommonRestrictions(DeviceBase* device, - const ImageCopyTexture& destination, - const Extent3D& copySize, - const CopyTextureForBrowserOptions& options) { +MaybeError ValidateCopyForBrowserDestination(DeviceBase* device, + const ImageCopyTexture& destination, + const Extent3D& copySize, + const CopyTextureForBrowserOptions& options) { DAWN_TRY(device->ValidateObject(destination.texture)); DAWN_INVALID_IF(destination.texture->GetTextureState() == TextureBase::TextureState::Destroyed, "Destination texture %s is destroyed.", destination.texture); @@ -630,9 +630,6 @@ MaybeError ValidateCopyForBrowserCommonRestrictions(DeviceBase* device, DAWN_TRY(ValidateCanUseAs(destination.texture, wgpu::TextureUsage::CopyDst, mode)); DAWN_TRY(ValidateCanUseAs(destination.texture, wgpu::TextureUsage::RenderAttachment, mode)); - DAWN_INVALID_IF(copySize.depthOrArrayLayers > 1, "Copy is for more than one array layer (%u)", - copySize.depthOrArrayLayers); - DAWN_INVALID_IF(destination.texture->GetSampleCount() > 1, "The destination texture sample count (%u) is not 1.", destination.texture->GetSampleCount()); @@ -645,6 +642,10 @@ MaybeError ValidateCopyForBrowserCommonRestrictions(DeviceBase* device, "Destination %s aspect (%s) doesn't select all the aspects of the destination format.", destination.texture, destination.aspect); + return {}; +} + +MaybeError ValidateCopyForBrowserOptions(const CopyTextureForBrowserOptions& options) { DAWN_INVALID_IF(options.nextInChain != nullptr, "nextInChain must be nullptr"); DAWN_TRY(ValidateAlphaMode(options.srcAlphaMode)); @@ -667,25 +668,18 @@ MaybeError ValidateCopyTextureForBrowser(DeviceBase* device, const ImageCopyTexture* destination, const Extent3D* copySize, const CopyTextureForBrowserOptions* options) { + // Validate source DAWN_TRY(device->ValidateObject(source->texture)); - DAWN_INVALID_IF(source->texture->GetTextureState() == TextureBase::TextureState::Destroyed, "Source texture %s is destroyed.", source->texture); - DAWN_TRY_CONTEXT(ValidateImageCopyTexture(device, *source, *copySize), "validating the ImageCopyTexture for the source"); - DAWN_TRY_CONTEXT(ValidateTextureCopyRange(device, *source, *copySize), "validating that the copy fits in the source"); - - DAWN_TRY(ValidateTextureToTextureCopyCommonRestrictions(*source, *destination, *copySize)); - DAWN_INVALID_IF(source->origin.z > 0, "Source has a non-zero z origin (%u).", source->origin.z); - DAWN_INVALID_IF(source->texture->GetSampleCount() > 1, "The source texture sample count (%u) is not 1. ", source->texture->GetSampleCount()); - DAWN_INVALID_IF( options->internalUsage && !device->HasFeature(Feature::DawnInternalUsages), "The internalUsage is true while the dawn-internal-usages feature is not enabled."); @@ -693,10 +687,19 @@ MaybeError ValidateCopyTextureForBrowser(DeviceBase* device, options->internalUsage ? UsageValidationMode::Internal : UsageValidationMode::Default; DAWN_TRY(ValidateCanUseAs(source->texture, wgpu::TextureUsage::CopySrc, mode)); DAWN_TRY(ValidateCanUseAs(source->texture, wgpu::TextureUsage::TextureBinding, mode)); - DAWN_TRY(ValidateCopyTextureSourceFormat(source->texture->GetFormat().format)); - DAWN_TRY(ValidateCopyForBrowserCommonRestrictions(device, *destination, *copySize, *options)); + // Validate destination + DAWN_TRY(ValidateCopyForBrowserDestination(device, *destination, *copySize, *options)); + + // Validate copy common rules and copySize. + DAWN_INVALID_IF(copySize->depthOrArrayLayers > 1, "Copy is for more than one array layer (%u)", + copySize->depthOrArrayLayers); + DAWN_TRY(ValidateTextureToTextureCopyCommonRestrictions(*source, *destination, *copySize)); + + // Validate options + DAWN_TRY(ValidateCopyForBrowserOptions(*options)); + return {}; } @@ -705,12 +708,10 @@ MaybeError ValidateCopyExternalTextureForBrowser(DeviceBase* device, const ImageCopyTexture* destination, const Extent3D* copySize, const CopyTextureForBrowserOptions* options) { + // Validate source DAWN_TRY(device->ValidateObject(source->externalTexture)); - DAWN_TRY(source->externalTexture->ValidateCanUseInSubmitNow()); - const Extent2D& sourceVisibleRect = source->externalTexture->GetVisibleRect(); - // All texture dimensions are in uint32_t so by doing checks in uint64_t we avoid // overflows. DAWN_INVALID_IF( @@ -721,14 +722,20 @@ MaybeError ValidateCopyExternalTextureForBrowser(DeviceBase* device, static_cast(source->origin.z) > 0, "Texture copy range (origin: %s, copySize: %s) touches outside of %s visible size (%s).", &source->origin, copySize, source->externalTexture, &sourceVisibleRect); - DAWN_INVALID_IF(source->origin.z > 0, "Source has a non-zero z origin (%u).", source->origin.z); - DAWN_INVALID_IF( options->internalUsage && !device->HasFeature(Feature::DawnInternalUsages), "The internalUsage is true while the dawn-internal-usages feature is not enabled."); - DAWN_TRY(ValidateCopyForBrowserCommonRestrictions(device, *destination, *copySize, *options)); + // Validate destination + DAWN_TRY(ValidateCopyForBrowserDestination(device, *destination, *copySize, *options)); + + // Validate copySize + DAWN_INVALID_IF(copySize->depthOrArrayLayers > 1, "Copy is for more than one array layer (%u)", + copySize->depthOrArrayLayers); + + // Validate options + DAWN_TRY(ValidateCopyForBrowserOptions(*options)); return {}; }