`GPUError` hierarchy in WebGPU is not up to spec.
Categories
(Core :: Graphics: WebGPU, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr102 | --- | unaffected |
firefox-esr115 | --- | unaffected |
firefox114 | --- | unaffected |
firefox115 | --- | unaffected |
firefox116 | --- | disabled |
firefox117 | --- | fixed |
People
(Reporter: ErichDonGubler, Assigned: ErichDonGubler)
References
(Regression)
Details
(Keywords: regression)
Attachments
(3 files)
Bug 1837557 actually got WebGPU logic far enough that we are actually trying to return errors into error scopes! ππ» Unfortunatelyβ¦some of that is broken. In particular, we're out-of-date for the GPUError
hierarchy. We currently represent GPUError
as a union of types (permalink):
typedef (GPUOutOfMemoryError or GPUValidationError) GPUError; # do not want :(
β¦but GPUError
has changed to be a parent interface of these, further adding the "internal"
/GPUInternalError
variant to the GPUError
API (permalink; see also the official spec's sections on GPUError
):
# This should be the shape of new code:
interface GPUError {
readonly attribute DOMString message;
};
interface GPUValidationError
: GPUError { # N.B.: now extends `GPUError`
constructor(DOMString message);
};
interface GPUOutOfMemoryError
: GPUError { # N.B.: now extends `GPUError`
constructor(DOMString message);
};
interface GPUInternalError # new
: GPUError {
constructor(DOMString message);
};
enum GPUErrorFilter {
"validation",
"out-of-memory",
"internal", # new
};
This bug's scope is to bring this API back up to spec.
Comment 1•1 year ago
|
||
Set release status flags based on info from the regressing bug 1837557
:jgilbert, since you are the author of the regressor, bug 1837557, could you take a look? Also, could you set the severity field?
For more information, please visit BugBot documentation.
Assignee | ||
Comment 2•1 year ago
|
||
One motivating issue for this bug being done now is that bug 1831263 is running into an issue that starts at dom/webgpu/Device.cpp:378
where the out-of-memory
variant is not being initialized properly; see this Try push for an example of failures. D'oh!
EDIT: :jgilbert filed bug 1838703 so we have more compiler assistance with avoiding this in the future. EDIT 2: Ah, the motivating issue will be handled there instead of here. π Noice!
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Updated•1 year ago
|
Comment 3•1 year ago
|
||
This bug is true, but the root cause for the regression was not this heirarchy, but rather bug 1838739.
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Comment 4•1 year ago
|
||
Looks like this will make some CTS tests pass because the internal
error variant will finally be available. For example, this test is currently failing because of it: https://gpuweb.github.io/cts/standalone/?q=webgpu:api,validation,encoding,createRenderBundleEncoder:attachment_state,limits,maxColorAttachments:*
CC :jimb, who wrote the test_too_many_color_attachments.ini
file that I'm blood-sworn to slay bent on deleting once this test passes. π
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Comment 5•1 year ago
|
||
From https://bugzilla.mozilla.org/show_bug.cgi?id=1838694#c3 , marking the regressor as bug 1838739. Please feel free to remove if that is not the case.
Updated•1 year ago
|
Assignee | ||
Comment 6•1 year ago
|
||
Assignee | ||
Comment 7•1 year ago
|
||
TODO: Validate that the previous commit actually unblocks the
maxColorAttachments
test in WPT.
Depends on D181690
Updated•1 year ago
|
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Comment 9•1 year ago
|
||
Comment 10•1 year ago
|
||
As a reminder soft code freeze for 116 starts tomorrow in case this still needed to land for 116
Comment 11•1 year ago
|
||
Pushed by egubler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/109ebd10d0c3 fix(webgpu): impl. correct `GPUError` types r=jgilbert,webgpu-reviewers,webidl,peterv https://hg.mozilla.org/integration/autoland/rev/f6477420370e fix(webgpu): only set `PopErrorScopeResult::resultType` in `Device::PopErrorScope` if an error was actually found r=webgpu-reviewers,jgilbert https://hg.mozilla.org/integration/autoland/rev/892fed91da4e test(webgpu): remove `test_too_many_color_attachments` r=jimb,webgpu-reviewers
Comment 12•1 year ago
|
||
Backed out for causing mochitests failures in test_interfaces_secureContext.html.
- Backout link
- Push with failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | dom/tests/mochitest/general/test_interfaces_secureContext.html | If this is failing: DANGER, are you sure you want to expose the new interface GPUInternalError to all webpages as a property on 'window'? Do not make a change to this file without a review from a DOM peer for that specific change!!! (or a JS peer for changes to ecmaGlobals)
Assignee | ||
Comment 13•1 year ago
|
||
I have investigated, and pushed up a fix for the errors governing the decision to back out. I had added new interfaces accessible from secure contexts; this test's interface snapshot in dom/tests/mochitest/general/test_interface.js
doesn't contain GPUError
or GPUInternalError
yet. CC :peterv, who I'll also tag in the relevant D181690.
Comment 14•1 year ago
|
||
Pushed by egubler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/83d3204888fd fix(webgpu): impl. correct `GPUError` types r=jgilbert,webgpu-reviewers,webidl,peterv https://hg.mozilla.org/integration/autoland/rev/7fdcca116760 fix(webgpu): only set `PopErrorScopeResult::resultType` in `Device::PopErrorScope` if an error was actually found r=webgpu-reviewers,jgilbert https://hg.mozilla.org/integration/autoland/rev/8f7835cd7d0c test(webgpu): remove `test_too_many_color_attachments` r=jimb,webgpu-reviewers
Comment 15•1 year ago
|
||
Set release status flags based on info from the regressing bug 1838739
Comment 16•1 year ago
|
||
Backed out for causing webgpu failures.
This seems to affect only Windows 11 x64/x86.
Comment 18•1 year ago
|
||
Pushed by egubler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f697f6c07e36 fix(webgpu): impl. correct `GPUError` types r=jgilbert,webgpu-reviewers,webidl,peterv https://hg.mozilla.org/integration/autoland/rev/f08b314b5972 fix(webgpu): only set `PopErrorScopeResult::resultType` in `Device::PopErrorScope` if an error was actually found r=webgpu-reviewers,jgilbert https://hg.mozilla.org/integration/autoland/rev/7555a6077330 test(webgpu): remove `test_too_many_color_attachments` r=jimb,webgpu-reviewers
Comment 19•1 year ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f697f6c07e36
https://hg.mozilla.org/mozilla-central/rev/f08b314b5972
https://hg.mozilla.org/mozilla-central/rev/7555a6077330
Comment 20•1 year ago
|
||
The patch landed in nightly and beta is affected.
:ErichDonGubler, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox116
towontfix
.
For more information, please visit BugBot documentation.
Updated•1 year ago
|
Updated•1 year ago
|
Description
•