From ccc78f6be49827dde2968507982465fd927ae28e Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Wed, 10 May 2023 20:36:33 +0000 Subject: [PATCH] Revert "Allow internal errors for pipeline creation failure" This reverts commit e241d64d25a53d2178f75a288ac2001ffdb39485. Reason for revert: Missing the change to handle the enums in the wire Original change's description: > Allow internal errors for pipeline creation failure > > Change-Id: I6b8c109ae67e230fea3fb14511c2b3562191c0fa > Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/132300 > Kokoro: Kokoro > Commit-Queue: Austin Eng > Reviewed-by: Loko Kung TBR=enga@chromium.org,noreply+kokoro@google.com,dawn-scoped@luci-project-accounts.iam.gserviceaccount.com,lokokung@google.com Change-Id: I12309e85e888cc400729f88bb2340e8ecdf6b798 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/132440 Auto-Submit: Austin Eng Kokoro: Austin Eng Reviewed-by: Loko Kung Commit-Queue: Austin Eng --- src/dawn/native/Device.cpp | 4 +- src/dawn/native/Error.cpp | 4 +- .../unittests/native/AllowedErrorTests.cpp | 105 ------------------ 3 files changed, 5 insertions(+), 108 deletions(-) diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index 80a8ba0f2b..183919fef1 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp @@ -1111,7 +1111,7 @@ ComputePipelineBase* DeviceBase::APICreateComputePipeline( utils::GetLabelForTrace(descriptor->label)); Ref result; - if (ConsumedError(CreateComputePipeline(descriptor), &result, InternalErrorType::Internal, + if (ConsumedError(CreateComputePipeline(descriptor), &result, "calling %s.CreateComputePipeline(%s).", this, descriptor)) { return ComputePipelineBase::MakeError(this, descriptor ? descriptor->label : nullptr); } @@ -1201,7 +1201,7 @@ RenderPipelineBase* DeviceBase::APICreateRenderPipeline( utils::GetLabelForTrace(descriptor->label)); Ref result; - if (ConsumedError(CreateRenderPipeline(descriptor), &result, InternalErrorType::Internal, + if (ConsumedError(CreateRenderPipeline(descriptor), &result, "calling %s.CreateRenderPipeline(%s).", this, descriptor)) { return RenderPipelineBase::MakeError(this, descriptor ? descriptor->label : nullptr); } diff --git a/src/dawn/native/Error.cpp b/src/dawn/native/Error.cpp index d5e56f5cd1..40d9b2adb9 100644 --- a/src/dawn/native/Error.cpp +++ b/src/dawn/native/Error.cpp @@ -36,8 +36,10 @@ wgpu::ErrorType ToWGPUErrorType(InternalErrorType type) { return wgpu::ErrorType::Validation; case InternalErrorType::OutOfMemory: return wgpu::ErrorType::OutOfMemory; + + // There is no equivalent of Internal errors in the WebGPU API. Internal errors cause + // the device at the API level to be lost, so treat it like a DeviceLost error. case InternalErrorType::Internal: - return wgpu::ErrorType::Internal; case InternalErrorType::DeviceLost: return wgpu::ErrorType::DeviceLost; diff --git a/src/dawn/tests/unittests/native/AllowedErrorTests.cpp b/src/dawn/tests/unittests/native/AllowedErrorTests.cpp index f140798984..78b45d2539 100644 --- a/src/dawn/tests/unittests/native/AllowedErrorTests.cpp +++ b/src/dawn/tests/unittests/native/AllowedErrorTests.cpp @@ -42,7 +42,6 @@ using ::testing::StrictMock; using ::testing::Test; static constexpr char kOomErrorMessage[] = "Out of memory error"; -static constexpr char kInternalErrorMessage[] = "Internal error"; static constexpr std::string_view kComputeShader = R"( @compute @workgroup_size(1) fn main() {} @@ -238,56 +237,6 @@ TEST_F(AllowedErrorTests, CreateRenderPipeline) { device.CreateRenderPipeline(ToCppAPI(&desc)); } -// Internal error from synchronously initializing a compute pipeline should not result in a device -// loss. -TEST_F(AllowedErrorTests, CreateComputePipelineInternalError) { - Ref csModule = ShaderModuleMock::Create(mDeviceMock, kComputeShader.data()); - - ComputePipelineDescriptor desc = {}; - desc.compute.module = csModule.Get(); - desc.compute.entryPoint = "main"; - - Ref computePipelineMock = ComputePipelineMock::Create(mDeviceMock, &desc); - EXPECT_CALL(*computePipelineMock.Get(), Initialize) - .WillOnce(Return(ByMove(DAWN_INTERNAL_ERROR(kInternalErrorMessage)))); - EXPECT_CALL(*mDeviceMock, CreateUninitializedComputePipelineImpl) - .WillOnce(Return(ByMove(std::move(computePipelineMock)))); - - // Expect the internal error. - EXPECT_CALL(mDeviceErrorCb, - Call(WGPUErrorType_Internal, HasSubstr(kInternalErrorMessage), this)) - .Times(1); - device.CreateComputePipeline(ToCppAPI(&desc)); - - // Device lost should only happen due to destruction. - EXPECT_CALL(mDeviceLostCb, Call(WGPUDeviceLostReason_Destroyed, _, this)).Times(1); -} - -// Internal error from synchronously initializing a render pipeline should not result in a device -// loss. -TEST_F(AllowedErrorTests, CreateRenderPipelineInternalError) { - Ref vsModule = ShaderModuleMock::Create(mDeviceMock, kVertexShader.data()); - - RenderPipelineDescriptor desc = {}; - desc.vertex.module = vsModule.Get(); - desc.vertex.entryPoint = "main"; - - Ref renderPipelineMock = RenderPipelineMock::Create(mDeviceMock, &desc); - EXPECT_CALL(*renderPipelineMock.Get(), Initialize) - .WillOnce(Return(ByMove(DAWN_INTERNAL_ERROR(kInternalErrorMessage)))); - EXPECT_CALL(*mDeviceMock, CreateUninitializedRenderPipelineImpl) - .WillOnce(Return(ByMove(std::move(renderPipelineMock)))); - - // Expect the internal error. - EXPECT_CALL(mDeviceErrorCb, - Call(WGPUErrorType_Internal, HasSubstr(kInternalErrorMessage), this)) - .Times(1); - device.CreateRenderPipeline(ToCppAPI(&desc)); - - // Device lost should only happen due to destruction. - EXPECT_CALL(mDeviceLostCb, Call(WGPUDeviceLostReason_Destroyed, _, this)).Times(1); -} - // // Exercise async APIs where OOM errors do NOT currently cause a device lost. // @@ -344,60 +293,6 @@ TEST_F(AllowedErrorTests, CreateRenderPipelineAsync) { EXPECT_CALL(mDeviceLostCb, Call(WGPUDeviceLostReason_Destroyed, _, this)).Times(1); } -// Internal error from asynchronously initializing a compute pipeline should not result in a device -// loss. -TEST_F(AllowedErrorTests, CreateComputePipelineAsyncInternalError) { - Ref csModule = ShaderModuleMock::Create(mDeviceMock, kComputeShader.data()); - - ComputePipelineDescriptor desc = {}; - desc.compute.module = csModule.Get(); - desc.compute.entryPoint = "main"; - - Ref computePipelineMock = ComputePipelineMock::Create(mDeviceMock, &desc); - EXPECT_CALL(*computePipelineMock.Get(), Initialize) - .WillOnce(Return(ByMove(DAWN_INTERNAL_ERROR(kInternalErrorMessage)))); - EXPECT_CALL(*mDeviceMock, CreateUninitializedComputePipelineImpl) - .WillOnce(Return(ByMove(std::move(computePipelineMock)))); - - MockCallback cb; - EXPECT_CALL(cb, Call(WGPUCreatePipelineAsyncStatus_InternalError, _, - HasSubstr(kInternalErrorMessage), this)) - .Times(1); - - device.CreateComputePipelineAsync(ToCppAPI(&desc), cb.Callback(), cb.MakeUserdata(this)); - device.Tick(); - - // Device lost should only happen because of destruction. - EXPECT_CALL(mDeviceLostCb, Call(WGPUDeviceLostReason_Destroyed, _, this)).Times(1); -} - -// Internal error from asynchronously initializing a render pipeline should not result in a device -// loss. -TEST_F(AllowedErrorTests, CreateRenderPipelineAsyncInternalError) { - Ref vsModule = ShaderModuleMock::Create(mDeviceMock, kVertexShader.data()); - - RenderPipelineDescriptor desc = {}; - desc.vertex.module = vsModule.Get(); - desc.vertex.entryPoint = "main"; - - Ref renderPipelineMock = RenderPipelineMock::Create(mDeviceMock, &desc); - EXPECT_CALL(*renderPipelineMock.Get(), Initialize) - .WillOnce(Return(ByMove(DAWN_INTERNAL_ERROR(kInternalErrorMessage)))); - EXPECT_CALL(*mDeviceMock, CreateUninitializedRenderPipelineImpl) - .WillOnce(Return(ByMove(std::move(renderPipelineMock)))); - - MockCallback cb; - EXPECT_CALL(cb, Call(WGPUCreatePipelineAsyncStatus_InternalError, _, - HasSubstr(kInternalErrorMessage), this)) - .Times(1); - - device.CreateRenderPipelineAsync(ToCppAPI(&desc), cb.Callback(), cb.MakeUserdata(this)); - device.Tick(); - - // Device lost should only happen because of destruction. - EXPECT_CALL(mDeviceLostCb, Call(WGPUDeviceLostReason_Destroyed, _, this)).Times(1); -} - // // Exercise APIs where OOM error are allowed and surfaced. //