WebGPU: Add uncapturedErrorCallbackInfo to requestDevice descriptor #22044
Conversation
beaufortfrancois
force-pushed
the
uncaptured
branch
from
25fcf04 to
72207ea
Compare
|
@kainino0x This PR is ready for review now. |
src/library_webgpu.js
Outdated
| assert(typeof GPUValidationError != 'undefined'); | ||
| assert(typeof GPUOutOfMemoryError != 'undefined'); | ||
| #endif | ||
| if (ev.error instanceof GPUValidationError) type = Validation; |
There was a problem hiding this comment.
old code, but let's update it:
Use the compile-time constants like {{{ gpu.ErrorType.Validation }}} instead
There was a problem hiding this comment.
I've updated this part.
src/library_webgpu.js
Outdated
| // WGPUErrorType type, const char* message, void* userdata | ||
| var Validation = 0x00000001; | ||
| var OutOfMemory = 0x00000002; | ||
| var type; |
There was a problem hiding this comment.
Default to {{{ gpu.ErrorType.Unknown }}}
| @@ -54,7 +54,11 @@ void GetAdapter(void (*callback)(wgpu::Adapter)) { | |||
| } | |||
|
|
|||
| void GetDevice(void (*callback)(wgpu::Device)) { | |||
| adapter.RequestDevice(nullptr, [](WGPURequestDeviceStatus status, WGPUDevice cDevice, const char* message, void* userdata) { | |||
| wgpu::DeviceDescriptor desc{}; | |||
| desc.uncapturedErrorCallbackInfo = {nullptr, [](WGPUErrorType type, char const* message, void*) { | |||
There was a problem hiding this comment.
I totally forgot this but this reminded me - the C++ bindings still don't use the correct types (C++ types instead of C types) and we still have a breaking change planned to fix that. Loko is going to do that soon but we don't have it yet.
In the interest of stability of the Emscripten bindings I suppose we should delay this change for now, since it's just going to have to change again?
(As I think I've mentioned we're going to try to push breaking changes to a single big release down the road as much as possible.)
There was a problem hiding this comment.
Let's wait until @lokokung's 🪄 then ;)
There was a problem hiding this comment.
I believe https://dawn-review.googlesource.com/c/dawn/+/192741 should help
There was a problem hiding this comment.
@lokokung @kainino0x Out of curiosity, what's the current status?
There was a problem hiding this comment.
For Emscripten changes, we recently pulled a fork of Emscripten into Dawn and are planning to make all changes there for a while and eventually upstream all the changes once we are closer to a stable header. As to when/where to land this change though, I'm not entirely sure and will let @kainino0x comment.
There was a problem hiding this comment.
I think we can add it to upstream emscripten, if we want, as soon as we are pretty sure the API shape for DeviceDescriptor is stable in Dawn (in both C and C++). But I'm not sure it's really worth it to do incrementally now instead of just waiting until we're doing other changes. In particular, adding fields to the descriptor can break dependent code, if they're not zero-initializing the whole struct. That should be relatively rare and they can just add zero-init but it does add some extra friction to do it now instead of later (IMO).
There was a problem hiding this comment.
@lokokung This seems related to beaufortfrancois/webgpu-cross-platform-app#8 (comment) as well.
There was a problem hiding this comment.
Similarly to #21072 (comment), I believe we can close this issue as next time we upstream changes in Emscripten, uncapturedErrorCallbackInfo will be implemented properly according to #21552 (comment)
@kainino0x What do you think?
Following up on https://github.com/beaufortfrancois/webgpu-cross-platform-app/pull/7/files#r1602327969, I'm adding
uncapturedErrorCallbackInfoto requestDevice descriptor.