Skip to content

Commit

Permalink
WebGPU: Add ErrorSharedImageRepresentationAndAccess
Browse files Browse the repository at this point in the history 
ErrorSharedImageRepresentationAndAccess is an implementation of
SharedImageRepresentationAndAccess that yields an error texture.
Using this allows AssociateMailbox to fail gracefully if the
device is lost or destroyed.

Bug: 1359106
Change-Id: Ibc5688cb680ec7782871068e4e656f7067109267
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3908270
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052233}
  • Loading branch information
@austinEng
austinEng authored and Chromium LUCI CQ committed Sep 28, 2022
1 parent b00a150 commit 7409fec
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 32 deletions.
44 changes: 41 additions & 3 deletions gpu/command_buffer/service/webgpu_decoder_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,6 @@ class WebGPUDecoderImpl final : public WebGPUDecoder {
}

const bool is_initialized = representation->IsCleared();
auto ignore_validation_errors =
IgnoreValidationErrorsScope(procs, device);
auto result =
base::WrapUnique(new SharedImageRepresentationAndAccessSkiaFallback(
std::move(shared_context_state), std::move(representation), procs,
Expand Down Expand Up @@ -876,6 +874,40 @@ class WebGPUDecoderImpl final : public WebGPUDecoder {
WGPUTextureUsage usage_;
};

// Implementation of SharedImageRepresentationAndAccess that yields an error
// texture.
class ErrorSharedImageRepresentationAndAccess
: public SharedImageRepresentationAndAccess {
public:
ErrorSharedImageRepresentationAndAccess(const DawnProcTable& procs,
WGPUDevice device,
WGPUTextureUsage usage)
: procs_(procs) {
// Note: the texture descriptor matters little since this texture won't be
// used for reflection, and all validation check the error state of the
// texture before the texture attributes.
WGPUTextureDescriptor texture_desc = {
.usage = static_cast<WGPUTextureUsageFlags>(usage),
.dimension = WGPUTextureDimension_2D,
.size = {1, 1, 1},
.format = WGPUTextureFormat_RGBA8Unorm,
.mipLevelCount = 1,
.sampleCount = 1,
};
texture_ = procs_.deviceCreateErrorTexture(device, &texture_desc);
}

~ErrorSharedImageRepresentationAndAccess() override {
procs_.textureRelease(texture_);
}

WGPUTexture texture() const override { return texture_; }

private:
const DawnProcTable& procs_;
WGPUTexture texture_;
};

// Map from the <ID, generation> pair for a wire texture to the shared image
// representation and access for it.
base::flat_map<std::tuple<uint32_t, uint32_t>,
Expand Down Expand Up @@ -1678,7 +1710,13 @@ error::Error WebGPUDecoderImpl::HandleAssociateMailboxImmediate(
}

if (!representation_and_access) {
return error::kInvalidArguments;
// According to the WebGPU specification, failing to create a WGPUTexture
// which wraps a shared image (like the canvas drawing buffer) should yield
// an error WGPUTexture. Use an implementation of
// SharedImageRepresentationAndAccess which always provides an error.
representation_and_access =
std::make_unique<ErrorSharedImageRepresentationAndAccess>(
dawn::native::GetProcs(), device, usage);
}

// Inject the texture in the dawn::wire::Server and remember which shared
Expand Down
128 changes: 101 additions & 27 deletions gpu/command_buffer/tests/webgpu_mailbox_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,19 +265,6 @@ TEST_P(WebGPUMailboxTest, AssociateMailboxCmd) {
GetGpuServiceHolder()->ScheduleGpuTask(base::BindOnce(
[](webgpu::WebGPUDecoder* decoder, webgpu::ReservedTexture reservation,
gpu::Mailbox mailbox) {
// Error case: invalid mailbox
{
gpu::Mailbox bad_mailbox;
AssociateMailboxCmdStorage cmd;
cmd.cmd.Init(reservation.deviceId, reservation.deviceGeneration,
reservation.id, reservation.generation,
WGPUTextureUsage_TextureBinding,
webgpu::WEBGPU_MAILBOX_NONE, bad_mailbox.name);
EXPECT_EQ(
error::kInvalidArguments,
ExecuteImmediateCmd(decoder, cmd.cmd, sizeof(bad_mailbox.name)));
}

// Error case: device client id doesn't exist.
{
AssociateMailboxCmdStorage cmd;
Expand Down Expand Up @@ -364,6 +351,41 @@ TEST_P(WebGPUMailboxTest, AssociateMailboxCmd) {
GetGpuServiceHolder()->gpu_thread_task_runner()->RunsTasksInCurrentSequence();
}

// Test that AssociateMailbox with a bad mailbox produces an error texture.
TEST_P(WebGPUMailboxTest, AssociateMailboxCmdBadMailboxMakesErrorTexture) {
// Create the shared image
SharedImageInterface* sii = GetSharedImageInterface();
Mailbox mailbox = sii->CreateSharedImage(
GetParam().format, {1, 1}, gfx::ColorSpace::CreateSRGB(),
kTopLeft_GrSurfaceOrigin, kPremul_SkAlphaType, SHARED_IMAGE_USAGE_WEBGPU,
kNullSurfaceHandle);

webgpu::ReservedTexture reservation = webgpu()->ReserveTexture(device_.Get());

GetGpuServiceHolder()->ScheduleGpuTask(base::BindOnce(
[](webgpu::WebGPUDecoder* decoder, webgpu::ReservedTexture reservation,
gpu::Mailbox mailbox) {
// Error case: invalid mailbox
{
gpu::Mailbox bad_mailbox;
AssociateMailboxCmdStorage cmd;
cmd.cmd.Init(reservation.deviceId, reservation.deviceGeneration,
reservation.id, reservation.generation,
WGPUTextureUsage_TextureBinding,
webgpu::WEBGPU_MAILBOX_NONE, bad_mailbox.name);
EXPECT_EQ(
error::kNoError,
ExecuteImmediateCmd(decoder, cmd.cmd, sizeof(bad_mailbox.name)));
}
},
GetDecoder(), reservation, mailbox));

wgpu::Texture texture = wgpu::Texture::Acquire(reservation.texture);

// Expect an error when creating a view since the texture is an error.
EXPECT_WEBGPU_ERROR(device_, WGPUErrorType_Validation, texture.CreateView());
}

TEST_P(WebGPUMailboxTest, DissociateMailboxCmd) {
// Create the shared image
SharedImageInterface* sii = GetSharedImageInterface();
Expand Down Expand Up @@ -424,7 +446,8 @@ TEST_P(WebGPUMailboxTest, DissociateMailboxCmd) {
// Test that Associate and Dissociate mailbox may be used after the device is
// destroyed. The test should not crash or produce unexpected validation errors.
TEST_P(WebGPUMailboxTest, AssociateDissociateMailboxAfterDeviceDestroy) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac")) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac") &&
!GPUTestBotConfig::CurrentConfigMatches("Win10")) {
GTEST_SKIP() << "Test skipped due to crbug.com/1359106.";
}

Expand All @@ -440,19 +463,62 @@ TEST_P(WebGPUMailboxTest, AssociateDissociateMailboxAfterDeviceDestroy) {

device_.Destroy();

webgpu()->AssociateMailbox(
reservation.deviceId, reservation.deviceGeneration, reservation.id,
reservation.generation, WGPUTextureUsage_RenderAttachment,
webgpu::WEBGPU_MAILBOX_NONE, reinterpret_cast<const GLbyte*>(&mailbox));
EXPECT_WEBGPU_ERROR(
device_, WGPUErrorType_Validation,
webgpu()->AssociateMailbox(
reservation.deviceId, reservation.deviceGeneration, reservation.id,
reservation.generation, WGPUTextureUsage_RenderAttachment,
webgpu::WEBGPU_MAILBOX_NONE,
reinterpret_cast<const GLbyte*>(&mailbox)));

wgpu::Texture texture = wgpu::Texture::Acquire(reservation.texture);
EXPECT_WEBGPU_ERROR(device_, WGPUErrorType_Validation, texture.CreateView());
webgpu()->DissociateMailbox(reservation.id, reservation.generation);
webgpu()->FlushCommands();
}

// Test that Associate mailbox and DissociateForPresent may be used after the
// device is destroyed. The test should not crash or produce unexpected
// validation errors.
TEST_P(WebGPUMailboxTest,
AssociateDissociateMailboxForPresentAfterDeviceDestroy) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac") &&
!GPUTestBotConfig::CurrentConfigMatches("Win10")) {
GTEST_SKIP() << "Test skipped due to crbug.com/1359106.";
}

SharedImageInterface* sii = GetSharedImageInterface();
Mailbox mailbox = sii->CreateSharedImage(
GetParam().format, {1, 1}, gfx::ColorSpace::CreateSRGB(),
kTopLeft_GrSurfaceOrigin, kPremul_SkAlphaType, SHARED_IMAGE_USAGE_WEBGPU,
kNullSurfaceHandle);
SyncToken mailbox_produced_token = sii->GenVerifiedSyncToken();
webgpu()->WaitSyncTokenCHROMIUM(mailbox_produced_token.GetConstData());

webgpu::ReservedTexture reservation = webgpu()->ReserveTexture(device_.Get());

device_.Destroy();

EXPECT_WEBGPU_ERROR(
device_, WGPUErrorType_Validation,
webgpu()->AssociateMailbox(
reservation.deviceId, reservation.deviceGeneration, reservation.id,
reservation.generation, WGPUTextureUsage_RenderAttachment,
webgpu::WEBGPU_MAILBOX_NONE,
reinterpret_cast<const GLbyte*>(&mailbox)));
wgpu::Texture texture = wgpu::Texture::Acquire(reservation.texture);
EXPECT_WEBGPU_ERROR(device_, WGPUErrorType_Validation, texture.CreateView());
webgpu()->DissociateMailboxForPresent(reservation.deviceId,
reservation.deviceGeneration,
reservation.id, reservation.generation);
webgpu()->FlushCommands();
}

// Test that ReserveTexture may be used after the device is destroyed.
// The test should not crash or produce unexpected validation errors.
TEST_P(WebGPUMailboxTest, ReserveTextureAfterDeviceDestroy) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac")) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac") &&
!GPUTestBotConfig::CurrentConfigMatches("Win10")) {
GTEST_SKIP() << "Test skipped due to crbug.com/1359106.";
}

Expand All @@ -468,19 +534,24 @@ TEST_P(WebGPUMailboxTest, ReserveTextureAfterDeviceDestroy) {

webgpu::ReservedTexture reservation = webgpu()->ReserveTexture(device_.Get());

webgpu()->AssociateMailbox(
reservation.deviceId, reservation.deviceGeneration, reservation.id,
reservation.generation, WGPUTextureUsage_RenderAttachment,
webgpu::WEBGPU_MAILBOX_NONE, reinterpret_cast<const GLbyte*>(&mailbox));
EXPECT_WEBGPU_ERROR(
device_, WGPUErrorType_Validation,
webgpu()->AssociateMailbox(
reservation.deviceId, reservation.deviceGeneration, reservation.id,
reservation.generation, WGPUTextureUsage_RenderAttachment,
webgpu::WEBGPU_MAILBOX_NONE,
reinterpret_cast<const GLbyte*>(&mailbox)));
wgpu::Texture texture = wgpu::Texture::Acquire(reservation.texture);
EXPECT_WEBGPU_ERROR(device_, WGPUErrorType_Validation, texture.CreateView());
webgpu()->DissociateMailbox(reservation.id, reservation.generation);
webgpu()->FlushCommands();
}

// Test that DissociateMailbox may be used after the device is destroyed.
// The test should not crash or produce unexpected validation errors.
TEST_P(WebGPUMailboxTest, DissociateMailboxAfterDeviceDestroy) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac")) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac") &&
!GPUTestBotConfig::CurrentConfigMatches("Win10")) {
GTEST_SKIP() << "Test skipped due to crbug.com/1359106.";
}

Expand Down Expand Up @@ -509,7 +580,8 @@ TEST_P(WebGPUMailboxTest, DissociateMailboxAfterDeviceDestroy) {
// device is destroyed. The test should not crash or produce unexpected
// validation errors.
TEST_P(WebGPUMailboxTest, DissociateMailboxForPresentAfterDeviceDestroy) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac")) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac") &&
!GPUTestBotConfig::CurrentConfigMatches("Win10")) {
GTEST_SKIP() << "Test skipped due to crbug.com/1359106.";
}

Expand Down Expand Up @@ -539,7 +611,8 @@ TEST_P(WebGPUMailboxTest, DissociateMailboxForPresentAfterDeviceDestroy) {
// Test that DissociateMailbox may be used after the texture is destroyed.
// The test should not crash or produce unexpected validation errors.
TEST_P(WebGPUMailboxTest, DissociateMailboxAfterTextureDestroy) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac")) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac") &&
!GPUTestBotConfig::CurrentConfigMatches("Win10")) {
GTEST_SKIP() << "Test skipped due to crbug.com/1359106.";
}

Expand Down Expand Up @@ -567,7 +640,8 @@ TEST_P(WebGPUMailboxTest, DissociateMailboxAfterTextureDestroy) {
// Test that DissociateMailboxForPresent may be used after the texture is
// destroyed. The test should not crash or produce unexpected validation errors.
TEST_P(WebGPUMailboxTest, DissociateMailboxForPresentAfterTextureDestroy) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac")) {
if (!GPUTestBotConfig::CurrentConfigMatches("Mac") &&
!GPUTestBotConfig::CurrentConfigMatches("Win10")) {
GTEST_SKIP() << "Test skipped due to crbug.com/1359106.";
}

Expand Down
18 changes: 16 additions & 2 deletions gpu/command_buffer/tests/webgpu_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ void CountCallback(int* count) {

WebGPUTest::Options::Options() = default;

std::map<std::pair<WGPUDevice, WGPUErrorType>, /* matched */ bool>
WebGPUTest::s_expected_errors = {};

WebGPUTest::WebGPUTest() = default;
WebGPUTest::~WebGPUTest() = default;

Expand Down Expand Up @@ -119,6 +122,9 @@ void WebGPUTest::Initialize(const Options& options) {
cmd_helper_ = std::make_unique<webgpu::WebGPUCmdHelper>(
context_->GetCommandBufferForTest());

webgpu()->SetLostContextCallback(base::BindLambdaForTesting(
[]() { GTEST_FAIL() << "Context lost unexpectedly."; }));

DawnProcTable procs = webgpu()->GetAPIChannel()->GetProcs();
dawnProcSetProcs(&procs);
instance_ = wgpu::Instance(webgpu()->GetAPIChannel()->GetWGPUInstance());
Expand Down Expand Up @@ -250,10 +256,16 @@ wgpu::Device WebGPUTest::GetNewDevice() {
},
nullptr);
device.SetUncapturedErrorCallback(
[](WGPUErrorType type, const char* message, void*) {
[](WGPUErrorType type, const char* message, void* userdata) {
auto it = s_expected_errors.find(
std::make_pair(static_cast<WGPUDevice>(userdata), type));
if (it != s_expected_errors.end() && !it->second) {
it->second = true;
return;
}
GTEST_FAIL() << "Unexpected error (" << type << "): " << message;
},
nullptr);
device.Get());
return device;
}

Expand Down Expand Up @@ -296,6 +308,7 @@ TEST_F(WebGPUTest, ReportLossReentrant) {
TEST_F(WebGPUTest, RequestAdapterAfterContextLost) {
Initialize(WebGPUTest::Options());

webgpu()->SetLostContextCallback(base::DoNothing());
webgpu()->OnGpuControlLostContext();

bool called = false;
Expand All @@ -316,6 +329,7 @@ TEST_F(WebGPUTest, RequestAdapterAfterContextLost) {
TEST_F(WebGPUTest, RequestDeviceAfterContextLost) {
Initialize(WebGPUTest::Options());

webgpu()->SetLostContextCallback(base::DoNothing());
webgpu()->OnGpuControlLostContext();

bool called = false;
Expand Down
17 changes: 17 additions & 0 deletions gpu/command_buffer/tests/webgpu_test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ class WebGPUTest : public testing::Test {
return gpu_service_holder_.get();
}

static std::map<std::pair<WGPUDevice, WGPUErrorType>, /* matched */ bool>
s_expected_errors;

wgpu::Instance instance_ = nullptr;
wgpu::Adapter adapter_ = nullptr;

Expand All @@ -86,6 +89,20 @@ class WebGPUTest : public testing::Test {
#endif
};

#define EXPECT_WEBGPU_ERROR(device, type, statement) \
do { \
PollUntilIdle(); \
auto it = \
s_expected_errors.insert({std::make_pair(device.Get(), type), false}); \
EXPECT_TRUE(it.second) \
<< "Only one expectation per-device-per-type supported."; \
statement; \
PollUntilIdle(); \
EXPECT_TRUE(it.first->second) \
<< "Expected error (" << type << ") in `" #statement "`"; \
s_expected_errors.erase(it.first); \
} while (0)

} // namespace gpu

#endif // GPU_COMMAND_BUFFER_TESTS_WEBGPU_TEST_H_

0 comments on commit 7409fec

Please sign in to comment.