0

WebGPU: fix viewFormats validation in GPUContextCanvas.

If the viewFormats passed to GPUCanvasContext.configure() are invalid,
the spec requires that a validation error occur. Subsequent calls to
GPUCanvasContext.getCurrentTexture() must also generate a validation
error.

The SwiftShader path in Chrome uses
SharedImageRepresentationAndAccessSkiaFallback for WebGPU canvas
contents. When GPUCanvasContext.getCurrentTexture() causes
AssociateMailbox() to be called on an invalid context, this causes the
GPUTexture allocation in the SIRAASF constructor to fail. When the
canvas is later garbage collected, DissociateMailbox() calls
SharedImageRepresentationAndAccessSkiaFallback::UploadContentsToSkia(),
which fails on that invalid texture in
GPUCommandEncoder.copyTextureToBuffer(). This can cause validation
errors in unrelated code (leading to the flaky failed tests in the bug).

The fix is to move the texture creation to SIRAASF::Create() and detect
the texture creation failure with a a push/popErrorScope(). If it fails,
return nullptr, causing
WebGPUDecoderImpl::HandleAssociateMailboxImmediate() to create a
ErrorSharedImageRepresentationAndAccess instead, as the other backends
do. This will avoid doing anything with an invalid texture.

However, it turns out that the validation error from CreateTexture is
actually load-bearing, and is the only reason that getCurrentTexture()
generates a validation error at all! Removing it causes the viewFormats
CTS tests to fail, since they don't get a validation error they're
expecting. The fix for that is to explicitly validate the texture
descriptor each time GPUCanvasContext.getCurrentTexture() is called.

Bug: 40853211
Change-Id: I445b0aff0f05efac01e0d95966ade9945876f9fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6435571
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1444466}
This commit is contained in:
Stephen White
2025-04-08 17:07:47 -07:00
committed by Chromium LUCI CQ
parent fcf3757806
commit 6a73a13bd2
3 changed files with 96 additions and 30 deletions
gpu/command_buffer
third_party/blink/renderer/modules/webgpu

@ -492,11 +492,55 @@ class WebGPUDecoderImpl final : public WebGPUDecoder {
}
const bool is_initialized = representation->IsCleared();
// Create a wgpu::Texture to hold the image contents.
// It must be internally copyable as this class itself uses the texture as
// the dest and source of copies for transfer back and forth between Skia
// and Dawn.
wgpu::DawnTextureInternalUsageDescriptor internal_usage_desc;
internal_usage_desc.internalUsage = internal_usage |
wgpu::TextureUsage::CopyDst |
wgpu::TextureUsage::CopySrc;
wgpu::TextureDescriptor texture_desc = {
.nextInChain = &internal_usage_desc,
.usage = usage,
.dimension = wgpu::TextureDimension::e2D,
.size = {static_cast<uint32_t>(representation->size().width()),
static_cast<uint32_t>(representation->size().height()), 1},
.format = ToDawnFormat(representation->format()),
.mipLevelCount = 1,
.sampleCount = 1,
.viewFormatCount = view_formats.size(),
.viewFormats =
reinterpret_cast<wgpu::TextureFormat*>(view_formats.data()),
};
// CreateTexture() may cause a validation error on an invalid texture
// descriptor, but is suppressed here. It will be caught in by the
// ValidateTextureDescriptor() call in
// GPUCanvasContext::getCurrentTexture() instead.
device.PushErrorScope(wgpu::ErrorFilter::Validation);
auto texture = device.CreateTexture(&texture_desc);
bool error = false;
device.PopErrorScope(
wgpu::CallbackMode::AllowSpontaneous,
[&error](wgpu::PopErrorScopeStatus status, wgpu::ErrorType type,
wgpu::StringView message) {
if (type == wgpu::ErrorType::Validation) {
error = true;
}
});
auto status = instance.WaitAny(0, nullptr, 0);
DCHECK(status == wgpu::WaitStatus::Success);
if (error) {
// If the CreateTexture() call failed, fail this function so that an
// ErrorSharedImageRepresentationAndAccess is created instead.
return nullptr;
}
auto result =
base::WrapUnique(new SharedImageRepresentationAndAccessSkiaFallback(
std::move(shared_context_state), std::move(representation),
std::move(instance), std::move(device), usage, internal_usage,
std::move(view_formats)));
std::move(instance), std::move(device), std::move(texture), usage,
internal_usage));
if (is_initialized && !result->PopulateFromSkia()) {
return nullptr;
}
@ -532,40 +576,16 @@ class WebGPUDecoderImpl final : public WebGPUDecoder {
std::unique_ptr<SkiaImageRepresentation> representation,
wgpu::Instance instance,
wgpu::Device device,
wgpu::Texture texture,
wgpu::TextureUsage usage,
wgpu::TextureUsage internal_usage,
std::vector<wgpu::TextureFormat> view_formats)
wgpu::TextureUsage internal_usage)
: shared_context_state_(std::move(shared_context_state)),
representation_(std::move(representation)),
instance_(std::move(instance)),
device_(std::move(device)),
texture_(std::move(texture)),
usage_(usage),
internal_usage_(internal_usage) {
// Create a wgpu::Texture to hold the image contents.
// It must be internally copyable as this class itself uses the texture as
// the dest and source of copies for transfer back and forth between Skia
// and Dawn.
wgpu::DawnTextureInternalUsageDescriptor internal_usage_desc;
internal_usage_desc.internalUsage = internal_usage |
wgpu::TextureUsage::CopyDst |
wgpu::TextureUsage::CopySrc;
wgpu::TextureDescriptor texture_desc = {
.nextInChain = &internal_usage_desc,
.usage = usage,
.dimension = wgpu::TextureDimension::e2D,
.size = {static_cast<uint32_t>(representation_->size().width()),
static_cast<uint32_t>(representation_->size().height()), 1},
.format = ToDawnFormat(representation_->format()),
.mipLevelCount = 1,
.sampleCount = 1,
.viewFormatCount = view_formats.size(),
.viewFormats =
reinterpret_cast<wgpu::TextureFormat*>(view_formats.data()),
};
texture_ = device_.CreateTexture(&texture_desc);
DCHECK(texture_);
}
internal_usage_(internal_usage) {}
bool ComputeStagingBufferParams(viz::SharedImageFormat format,
const gfx::Size& size,

@ -1416,6 +1416,47 @@ TEST_P(WebGPUMailboxTextureTest, ReflectionOfDescriptor) {
webgpu::WEBGPU_MAILBOX_NONE, shared_image2->mailbox());
}
// Test that passing a texture with invalid view formats to AssociateMailbox
// does not cause WebGPU validation errors on a later call to DissociateMailbox.
TEST_P(WebGPUMailboxTextureTest, AssociateInvalidViewFormats) {
wgpu::TextureDescriptor desc = {};
desc.size = {1, 1, 1};
desc.format = ToDawnFormat(GetParam().format);
desc.usage = wgpu::TextureUsage::RenderAttachment;
desc.dimension = wgpu::TextureDimension::e2D;
desc.sampleCount = 1;
desc.mipLevelCount = 1;
gpu::webgpu::ReservedTexture reservation = webgpu()->ReserveTexture(
device_.Get(), reinterpret_cast<const WGPUTextureDescriptor*>(&desc));
wgpu::Texture texture = wgpu::Texture::Acquire(reservation.texture);
SharedImageInterface* sii = GetSharedImageInterface();
scoped_refptr<gpu::ClientSharedImage> shared_image =
sii->CreateSharedImage({GetParam().format,
{1, 1},
gfx::ColorSpace::CreateSRGB(),
GetSharedImageUsage(AccessType::ReadWrite),
"TestLabel"},
kNullSurfaceHandle);
WGPUTextureFormat view_formats = {
WGPUTextureFormat_R8Unorm,
};
// AssociateMailbox may cause validation errors, given the invalid
// viewFormats, so wrap it in an error scope.
device_.PushErrorScope(wgpu::ErrorFilter::Validation);
webgpu()->AssociateMailbox(
reservation.deviceId, reservation.deviceGeneration, reservation.id,
reservation.generation, static_cast<WGPUTextureUsage>(desc.usage),
&view_formats, 1, webgpu::WEBGPU_MAILBOX_NONE, shared_image->mailbox());
device_.PopErrorScope(
wgpu::CallbackMode::AllowSpontaneous,
[](wgpu::PopErrorScopeStatus, wgpu::ErrorType, wgpu::StringView) {});
// DissociateMailbox should NOT cause validation errors.
webgpu()->DissociateMailbox(reservation.id, reservation.generation);
}
// Test that if some other GL context is current when
// Associate/DissociateMailbox occurs, the operations do not fail. Some WebGPU
// shared image backings rely on GL and need to be responsible for making the

@ -622,6 +622,11 @@ GPUTexture* GPUCanvasContext::getCurrentTexture(
}
DCHECK(device_);
// Validate the texture descriptor as required by the spec for
// GPUCanvasContext.getCurrentTexture(). This is required on each call,
// so it must appear before the cached texture early-out below.
device_->GetHandle().ValidateTextureDescriptor(&texture_descriptor_);
// Calling getCurrentTexture returns a texture that is valid until the
// animation frame it gets presented. If getCurrentTexture is called multiple
// time, the same texture should be returned. |texture_| is set to