-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate invalidation of submitted command buffers #3716
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm % standardization, one request
…he submit call succeeds https://bugs.webkit.org/show_bug.cgi?id=274244> <radar://128177642> Reviewed by NOBODY (OOPS!). Invalidate all command buffers passed to GPUQueue.submit, regardless of whether or not they get committed. Fixes the test being added in gpuweb/cts#3716 * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::invalidateCommandBuffers): (WebGPU::Queue::submit):
Rebased and split the tests as requested. Spec change was approved and landed in the last WG call, so this is ready to land now once reviewed. |
…he submit call succeeds https://bugs.webkit.org/show_bug.cgi?id=274244> <radar://128177642> Reviewed by Dan Glastonbury. Invalidate all command buffers passed to GPUQueue.submit, regardless of whether or not they get committed. Fixes the test being added in gpuweb/cts#3716 * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::invalidateCommandBuffers): (WebGPU::Queue::submit): Canonical link: https://commits.webkit.org/279084@main
|
||
// Submit should fail because on of the command buffers is invalid | ||
t.expectValidationError(() => { | ||
t.device.queue.submit([cb, cb_invalid]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm... any way we could verify that cb
doesn't execute? Maybe a new test as a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, though I am inclined to do that as a follow up.
…he submit call succeeds https://bugs.webkit.org/show_bug.cgi?id=274244> <radar://128177642> Reviewed by Dan Glastonbury. Invalidate all command buffers passed to GPUQueue.submit, regardless of whether or not they get committed. Fixes the test being added in gpuweb/cts#3716 Reland with regression fixed and test enabled. * LayoutTests/platform/mac-wk2/TestExpectations: * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::invalidateCommandBuffers): (WebGPU::Queue::submit):
…he submit call succeeds https://bugs.webkit.org/show_bug.cgi?id=274244> <radar://128177642> Reviewed by Dan Glastonbury. Invalidate all command buffers passed to GPUQueue.submit, regardless of whether or not they get committed. Fixes the test being added in gpuweb/cts#3716 Reland with regression fixed and test enabled. * LayoutTests/platform/mac-wk2/TestExpectations: * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::invalidateCommandBuffers): (WebGPU::Queue::submit):
…he submit call succeeds https://bugs.webkit.org/show_bug.cgi?id=274244> <radar://128177642> Reviewed by Dan Glastonbury. Invalidate all command buffers passed to GPUQueue.submit, regardless of whether or not they get committed. Fixes the test being added in gpuweb/cts#3716 Reland with regression fixed and test enabled. * LayoutTests/platform/mac-wk2/TestExpectations: * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::invalidateCommandBuffers): (WebGPU::Queue::submit): Canonical link: https://commits.webkit.org/279177@main
…he submit call succeeds https://bugs.webkit.org/show_bug.cgi?id=274244> <radar://128177642> Reviewed by Dan Glastonbury. Invalidate all command buffers passed to GPUQueue.submit, regardless of whether or not they get committed. Fixes the test being added in gpuweb/cts#3716 Reland with regression fixed and test enabled. * LayoutTests/platform/mac-wk2/TestExpectations: * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::invalidateCommandBuffers): (WebGPU::Queue::submit):
…he submit call succeeds https://bugs.webkit.org/show_bug.cgi?id=274244> <radar://128177642> Reviewed by Dan Glastonbury. Invalidate all command buffers passed to GPUQueue.submit, regardless of whether or not they get committed. Fixes the test being added in gpuweb/cts#3716 Reland with regression fixed and test enabled. * LayoutTests/platform/mac-wk2/TestExpectations: * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::invalidateCommandBuffers): (WebGPU::Queue::submit): Canonical link: https://commits.webkit.org/279235@main
…he submit call succeeds https://bugs.webkit.org/show_bug.cgi?id=274244> <radar://128177642> Reviewed by Dan Glastonbury. Invalidate all command buffers passed to GPUQueue.submit, regardless of whether or not they get committed. Fixes the test being added in gpuweb/cts#3716 Reland with regression fixed and test enabled. * LayoutTests/platform/mac-wk2/TestExpectations: * Source/WebGPU/WebGPU/Queue.mm: (WebGPU::invalidateCommandBuffers): (WebGPU::Queue::submit): Canonical link: https://commits.webkit.org/279243@main
Adds validation to ensure that submit invalidates command buffers. I'm surprised this didn't exist before, but I'm unable to find any tests that attempt that specifically. Also makes a small utility for creating command buffers to make these tests a bit more terse and easy to write.
Last two checks in this test assume that we change the spec for
submit()
as proposed in gpuweb/gpuweb#4608 and discussed in gpuweb/gpuweb#4599.Requirements for PR author:
.unimplemented()
./** documented */
and new helper files are found inhelper_index.txt
.Requirements for reviewer sign-off:
When landing this PR, be sure to make any necessary issue status updates.