Skip to content

Commit

Permalink
[WebGPU] Queue::commitMTLCommandBuffer will fail if command buffer wa…
Browse files Browse the repository at this point in the history
…s already committed

https://bugs.webkit.org/show_bug.cgi?id=264339
<radar://117851924>

Reviewed by Tadeu Zagallo.

Ensure that a command buffer doesn't get submitted twice and generate
a validation error if we would attempt to do so.

* Source/WebGPU/WebGPU/CommandBuffer.h:
* Source/WebGPU/WebGPU/CommandBuffer.mm:
(WebGPU::CommandBuffer::makeInvalid):
* Source/WebGPU/WebGPU/Queue.h:
* Source/WebGPU/WebGPU/Queue.mm:
(WebGPU::Queue::validateSubmit const):
(WebGPU::Queue::commitMTLCommandBuffer):
(WebGPU::Queue::submit):
(wgpuQueueSubmit):

Canonical link: https://commits.webkit.org/270361@main
  • Loading branch information
mwyrzykowski committed Nov 8, 2023
1 parent 3d7e86f commit a965533
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 8 deletions.
3 changes: 2 additions & 1 deletion Source/WebGPU/WebGPU/CommandBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,13 @@ class CommandBuffer : public WGPUCommandBufferImpl, public RefCounted<CommandBuf
id<MTLCommandBuffer> commandBuffer() const { return m_commandBuffer; }

Device& device() const { return m_device; }
void makeInvalid();

private:
CommandBuffer(id<MTLCommandBuffer>, Device&);
CommandBuffer(Device&);

const id<MTLCommandBuffer> m_commandBuffer { nil };
id<MTLCommandBuffer> m_commandBuffer { nil };

const Ref<Device> m_device;
};
Expand Down
5 changes: 5 additions & 0 deletions Source/WebGPU/WebGPU/CommandBuffer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@
m_commandBuffer.label = label;
}

void CommandBuffer::makeInvalid()
{
m_commandBuffer = nil;
}

} // namespace WebGPU

#pragma mark WGPU Stubs
Expand Down
4 changes: 2 additions & 2 deletions Source/WebGPU/WebGPU/Queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class Queue : public WGPUQueueImpl, public ThreadSafeRefCounted<Queue> {
~Queue();

void onSubmittedWorkDone(CompletionHandler<void(WGPUQueueWorkDoneStatus)>&& callback);
void submit(Vector<std::reference_wrapper<const CommandBuffer>>&& commands);
void submit(Vector<std::reference_wrapper<CommandBuffer>>&& commands);
void writeBuffer(const Buffer&, uint64_t bufferOffset, const void* data, size_t);
void writeTexture(const WGPUImageCopyTexture& destination, const void* data, size_t dataSize, const WGPUTextureDataLayout&, const WGPUExtent3D& writeSize);
void setLabel(String&&);
Expand All @@ -78,7 +78,7 @@ class Queue : public WGPUQueueImpl, public ThreadSafeRefCounted<Queue> {
Queue(id<MTLCommandQueue>, Device&);
Queue(Device&);

bool validateSubmit(const Vector<std::reference_wrapper<const CommandBuffer>>&) const;
bool validateSubmit(const Vector<std::reference_wrapper<CommandBuffer>>&) const;
bool validateWriteBuffer(const Buffer&, uint64_t bufferOffset, size_t) const;

void ensureBlitCommandEncoder();
Expand Down
25 changes: 20 additions & 5 deletions Source/WebGPU/WebGPU/Queue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
callbacks.append(WTFMove(completionHandler));
}

bool Queue::validateSubmit(const Vector<std::reference_wrapper<const CommandBuffer>>& commands) const
bool Queue::validateSubmit(const Vector<std::reference_wrapper<CommandBuffer>>& commands) const
{
for (auto command : commands) {
if (!isValidToUseWith(command.get(), *this))
Expand All @@ -140,6 +140,9 @@

void Queue::commitMTLCommandBuffer(id<MTLCommandBuffer> commandBuffer)
{
if (!commandBuffer || commandBuffer.status >= MTLCommandBufferStatusCommitted)
return;

ASSERT(commandBuffer.commandQueue == m_commandQueue);
[commandBuffer addScheduledHandler:[protectedThis = Ref { *this }](id<MTLCommandBuffer>) {
protectedThis->scheduleWork(CompletionHandler<void(void)>([protectedThis = protectedThis.copyRef()]() {
Expand All @@ -162,7 +165,7 @@
++m_submittedCommandBufferCount;
}

void Queue::submit(Vector<std::reference_wrapper<const CommandBuffer>>&& commands)
void Queue::submit(Vector<std::reference_wrapper<CommandBuffer>>&& commands)
{
// https://gpuweb.github.io/gpuweb/#dom-gpuqueue-submit

Expand All @@ -173,8 +176,20 @@

finalizeBlitCommandEncoder();

for (auto commandBuffer : commands)
commitMTLCommandBuffer(commandBuffer.get().commandBuffer());
NSMutableArray<id<MTLCommandBuffer>> *commandBuffersToSubmit = [NSMutableArray arrayWithCapacity:commands.size()];
for (auto commandBuffer : commands) {
auto& command = commandBuffer.get();
if (id<MTLCommandBuffer> mtlBuffer = command.commandBuffer())
[commandBuffersToSubmit addObject:mtlBuffer];
else {
m_device.generateAValidationError("Command buffer appears twice."_s);
return;
}
command.makeInvalid();
}

for (id<MTLCommandBuffer> commandBuffer in commandBuffersToSubmit)
commitMTLCommandBuffer(commandBuffer);

if ([MTLCaptureManager sharedCaptureManager].isCapturing)
[[MTLCaptureManager sharedCaptureManager] stopCapture];
Expand Down Expand Up @@ -616,7 +631,7 @@ void wgpuQueueOnSubmittedWorkDoneWithBlock(WGPUQueue queue, WGPUQueueWorkDoneBlo

void wgpuQueueSubmit(WGPUQueue queue, size_t commandCount, const WGPUCommandBuffer* commands)
{
Vector<std::reference_wrapper<const WebGPU::CommandBuffer>> commandsToForward;
Vector<std::reference_wrapper<WebGPU::CommandBuffer>> commandsToForward;
for (uint32_t i = 0; i < commandCount; ++i)
commandsToForward.append(WebGPU::fromAPI(commands[i]));
WebGPU::fromAPI(queue).submit(WTFMove(commandsToForward));
Expand Down

0 comments on commit a965533

Please sign in to comment.