dawn_wire: Forward client-generated errors to the server
This fixes a problem where client-generated errors weren't properly
captured in error scopes.
Bug: chromium:1004368
Change-Id: Ic0f6e5bc5e281c676ea7154dd217cfc8dd51be5c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/11642
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/dawn.json b/dawn.json
index d736b6f..47bb485 100644
--- a/dawn.json
+++ b/dawn.json
@@ -540,6 +540,14 @@
]
},
{
+ "name": "inject error",
+ "args": [
+ {"name": "type", "type": "error type"},
+ {"name": "message", "type": "char", "annotation": "const*"}
+ ],
+ "TODO": "enga@: Make this a Dawn extension"
+ },
+ {
"name": "tick"
},
{
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 1bdc792..0c76a57 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -98,6 +98,13 @@
mCurrentErrorScope->HandleError(type, message);
}
+ void DeviceBase::InjectError(dawn::ErrorType type, const char* message) {
+ if (ConsumedError(ValidateErrorType(type))) {
+ return;
+ }
+ mCurrentErrorScope->HandleError(type, message);
+ }
+
void DeviceBase::ConsumeError(ErrorData* error) {
ASSERT(error != nullptr);
HandleError(error->GetType(), error->GetMessage().c_str());
diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h
index c66826a..a17d65a 100644
--- a/src/dawn_native/Device.h
+++ b/src/dawn_native/Device.h
@@ -149,6 +149,8 @@
TextureViewBase* CreateTextureView(TextureBase* texture,
const TextureViewDescriptor* descriptor);
+ void InjectError(dawn::ErrorType type, const char* message);
+
void Tick();
void SetUncapturedErrorCallback(dawn::ErrorCallback callback, void* userdata);
diff --git a/src/dawn_wire/client/ApiProcs.cpp b/src/dawn_wire/client/ApiProcs.cpp
index df6ed53..12ecf89 100644
--- a/src/dawn_wire/client/ApiProcs.cpp
+++ b/src/dawn_wire/client/ApiProcs.cpp
@@ -285,8 +285,9 @@
void* userdata) {
Fence* fence = reinterpret_cast<Fence*>(cFence);
if (value > fence->signaledValue) {
- fence->device->HandleError(DAWN_ERROR_TYPE_VALIDATION,
- "Value greater than fence signaled value");
+ ClientDeviceInjectError(reinterpret_cast<DawnDevice>(fence->device),
+ DAWN_ERROR_TYPE_VALIDATION,
+ "Value greater than fence signaled value");
callback(DAWN_FENCE_COMPLETION_STATUS_ERROR, userdata);
return;
}
@@ -394,14 +395,15 @@
Fence* fence = reinterpret_cast<Fence*>(cFence);
Queue* queue = reinterpret_cast<Queue*>(cQueue);
if (fence->queue != queue) {
- fence->device->HandleError(
- DAWN_ERROR_TYPE_VALIDATION,
- "Fence must be signaled on the queue on which it was created.");
+ ClientDeviceInjectError(reinterpret_cast<DawnDevice>(fence->device),
+ DAWN_ERROR_TYPE_VALIDATION,
+ "Fence must be signaled on the queue on which it was created.");
return;
}
if (signalValue <= fence->signaledValue) {
- fence->device->HandleError(DAWN_ERROR_TYPE_VALIDATION,
- "Fence value less than or equal to signaled value");
+ ClientDeviceInjectError(reinterpret_cast<DawnDevice>(fence->device),
+ DAWN_ERROR_TYPE_VALIDATION,
+ "Fence value less than or equal to signaled value");
return;
}
fence->signaledValue = signalValue;
diff --git a/src/tests/end2end/FenceTests.cpp b/src/tests/end2end/FenceTests.cpp
index b88c953..4a06ea94 100644
--- a/src/tests/end2end/FenceTests.cpp
+++ b/src/tests/end2end/FenceTests.cpp
@@ -18,6 +18,8 @@
#include <array>
#include <cstring>
+using namespace testing;
+
class MockFenceOnCompletionCallback {
public:
MOCK_METHOD2(Call, void(DawnFenceCompletionStatus status, void* userdata));
@@ -27,6 +29,17 @@
static void ToMockFenceOnCompletionCallback(DawnFenceCompletionStatus status, void* userdata) {
mockFenceOnCompletionCallback->Call(status, userdata);
}
+
+class MockPopErrorScopeCallback {
+ public:
+ MOCK_METHOD3(Call, void(DawnErrorType type, const char* message, void* userdata));
+};
+
+static std::unique_ptr<MockPopErrorScopeCallback> mockPopErrorScopeCallback;
+static void ToMockPopErrorScopeCallback(DawnErrorType type, const char* message, void* userdata) {
+ mockPopErrorScopeCallback->Call(type, message, userdata);
+}
+
class FenceTests : public DawnTest {
private:
struct CallbackInfo {
@@ -50,10 +63,12 @@
void SetUp() override {
DawnTest::SetUp();
mockFenceOnCompletionCallback = std::make_unique<MockFenceOnCompletionCallback>();
+ mockPopErrorScopeCallback = std::make_unique<MockPopErrorScopeCallback>();
}
void TearDown() override {
mockFenceOnCompletionCallback = nullptr;
+ mockPopErrorScopeCallback = nullptr;
DawnTest::TearDown();
}
@@ -206,4 +221,22 @@
WaitForCompletedValue(fence, 1);
}
+// Regression test that validation errors that are tracked client-side are captured
+// in error scopes.
+TEST_P(FenceTests, ClientValidationErrorInErrorScope) {
+ dawn::FenceDescriptor descriptor;
+ descriptor.initialValue = 0u;
+ dawn::Fence fence = queue.CreateFence(&descriptor);
+
+ queue.Signal(fence, 4);
+
+ device.PushErrorScope(dawn::ErrorFilter::Validation);
+ queue.Signal(fence, 2);
+
+ EXPECT_CALL(*mockPopErrorScopeCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, this)).Times(1);
+ device.PopErrorScope(ToMockPopErrorScopeCallback, this);
+
+ WaitForCompletedValue(fence, 4);
+}
+
DAWN_INSTANTIATE_TEST(FenceTests, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend);
diff --git a/src/tests/unittests/wire/WireFenceTests.cpp b/src/tests/unittests/wire/WireFenceTests.cpp
index 7de32f2..d7d2608 100644
--- a/src/tests/unittests/wire/WireFenceTests.cpp
+++ b/src/tests/unittests/wire/WireFenceTests.cpp
@@ -19,17 +19,6 @@
namespace {
- // Mock classes to add expectations on the wire calling callbacks
- class MockDeviceErrorCallback {
- public:
- MOCK_METHOD3(Call, void(DawnErrorType type, const char* message, void* userdata));
- };
-
- std::unique_ptr<StrictMock<MockDeviceErrorCallback>> mockDeviceErrorCallback;
- void ToMockDeviceErrorCallback(DawnErrorType type, const char* message, void* userdata) {
- mockDeviceErrorCallback->Call(type, message, userdata);
- }
-
class MockFenceOnCompletionCallback {
public:
MOCK_METHOD2(Call, void(DawnFenceCompletionStatus status, void* userdata));
@@ -51,7 +40,6 @@
void SetUp() override {
WireTest::SetUp();
- mockDeviceErrorCallback = std::make_unique<StrictMock<MockDeviceErrorCallback>>();
mockFenceOnCompletionCallback =
std::make_unique<StrictMock<MockFenceOnCompletionCallback>>();
@@ -77,14 +65,12 @@
void TearDown() override {
WireTest::TearDown();
- mockDeviceErrorCallback = nullptr;
mockFenceOnCompletionCallback = nullptr;
}
void FlushServer() {
WireTest::FlushServer();
- Mock::VerifyAndClearExpectations(&mockDeviceErrorCallback);
Mock::VerifyAndClearExpectations(&mockFenceOnCompletionCallback);
}
@@ -117,37 +103,23 @@
FlushServer();
}
-// Without any flushes, it is valid to signal a value greater than the current
-// signaled value
-TEST_F(WireFenceTests, QueueSignalSynchronousValidationSuccess) {
- dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, nullptr);
- EXPECT_CALL(*mockDeviceErrorCallback, Call(_, _, _)).Times(0);
-
- dawnQueueSignal(queue, fence, 2u);
- dawnQueueSignal(queue, fence, 4u);
- dawnQueueSignal(queue, fence, 5u);
-}
-
-// Without any flushes, errors should be generated when signaling a value less
+// Errors should be generated when signaling a value less
// than or equal to the current signaled value
-TEST_F(WireFenceTests, QueueSignalSynchronousValidationError) {
- dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, nullptr);
-
- EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
+TEST_F(WireFenceTests, QueueSignalValidationError) {
dawnQueueSignal(queue, fence, 0u); // Error
- EXPECT_TRUE(Mock::VerifyAndClear(mockDeviceErrorCallback.get()));
+ EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+ FlushClient();
- EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
dawnQueueSignal(queue, fence, 1u); // Error
- EXPECT_TRUE(Mock::VerifyAndClear(mockDeviceErrorCallback.get()));
+ EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+ FlushClient();
- EXPECT_CALL(*mockDeviceErrorCallback, Call(_, _, _)).Times(0);
- dawnQueueSignal(queue, fence, 4u); // Success
- EXPECT_TRUE(Mock::VerifyAndClear(mockDeviceErrorCallback.get()));
+ DoQueueSignal(4u); // Success
+ FlushClient();
- EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
dawnQueueSignal(queue, fence, 3u); // Error
- EXPECT_TRUE(Mock::VerifyAndClear(mockDeviceErrorCallback.get()));
+ EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+ FlushClient();
}
// Check that callbacks are immediately called if the fence is already finished
@@ -214,16 +186,16 @@
.Times(3);
}
-// Without any flushes, errors should be generated when waiting on a value greater
+// Errors should be generated when waiting on a value greater
// than the last signaled value
-TEST_F(WireFenceTests, OnCompletionSynchronousValidationError) {
- dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, this + 1);
-
+TEST_F(WireFenceTests, OnCompletionValidationError) {
EXPECT_CALL(*mockFenceOnCompletionCallback, Call(DAWN_FENCE_COMPLETION_STATUS_ERROR, this + 0))
.Times(1);
- EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, this + 1)).Times(1);
dawnFenceOnCompletion(fence, 2u, ToMockFenceOnCompletionCallback, this + 0);
+
+ EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+ FlushClient();
}
// Check that the fence completed value is initialized
@@ -262,9 +234,9 @@
EXPECT_CALL(api, DeviceCreateQueue(apiDevice)).WillOnce(Return(apiQueue2));
FlushClient();
- dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, nullptr);
- EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
dawnQueueSignal(queue2, fence, 2u); // error
+ EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+ FlushClient();
}
// Test that signaling a fence on a wrong queue does not update fence signaled value
@@ -274,9 +246,9 @@
EXPECT_CALL(api, DeviceCreateQueue(apiDevice)).WillOnce(Return(apiQueue2));
FlushClient();
- dawnDeviceSetUncapturedErrorCallback(device, ToMockDeviceErrorCallback, nullptr);
- EXPECT_CALL(*mockDeviceErrorCallback, Call(DAWN_ERROR_TYPE_VALIDATION, _, _)).Times(1);
dawnQueueSignal(queue2, fence, 2u); // error
+ EXPECT_CALL(api, DeviceInjectError(apiDevice, DAWN_ERROR_TYPE_VALIDATION, _)).Times(1);
+ FlushClient();
// Fence value should be unchanged.
FlushClient();
