Title: [291688] trunk/Source/WebGPU
Revision
291688
Author
mmaxfi...@apple.com
Date
2022-03-22 12:45:56 -0700 (Tue, 22 Mar 2022)

Log Message

[WebGPU] Implement Queue::writeBuffer()
https://bugs.webkit.org/show_bug.cgi?id=238194

Reviewed by Kimmo Kinnunen.

The semantics of writeBuffer() are that the data should be written, but specifically
at the current point in the queue. So, we can't just naively memcpy(), for 2 reasons:
1. If the buffer is in-use, memcpy() will clobber its current contents, leading to
        corruption. Instead, it's supposed to populate the buffer at the current point
        in the queue, as-if it was a queue item.
2. If the buffer is a private buffer, we can't memcpy() to it anyway

So, the implementation of this function first tries to see if we can memcpy(): if the
buffer is shared or managed, and if the queue is idle. If it can, then it does so, but
if it can't, it creates a temporary buffer and enqueues a queue command to copy the
contents of the temporary buffer into the destination buffer.

That copy command is part of a lazily created command buffer.

Test: api/validation/queue/writeBuffer.spec.ts

* WebGPU/Buffer.h:
(WebGPU::Buffer::state const):
* WebGPU/Device.h:
(WebGPU::Device::device const):
* WebGPU/Queue.h:
(WebGPU::Queue::isIdle const):
* WebGPU/Queue.mm:
(WebGPU::Queue::~Queue):
(WebGPU::Queue::ensureBlitCommandEncoder):
(WebGPU::Queue::finalizeBlitCommandEncoder):
(WebGPU::Queue::onSubmittedWorkDone):
(WebGPU::Queue::commitMTLCommandBuffer):
(WebGPU::Queue::submit):
(WebGPU::validateWriteBufferInitial):
(WebGPU::Queue::validateWriteBuffer const):
(WebGPU::Queue::writeBuffer):

Modified Paths

Diff

Modified: trunk/Source/WebGPU/ChangeLog (291687 => 291688)


--- trunk/Source/WebGPU/ChangeLog	2022-03-22 19:04:20 UTC (rev 291687)
+++ trunk/Source/WebGPU/ChangeLog	2022-03-22 19:45:56 UTC (rev 291688)
@@ -1,5 +1,45 @@
 2022-03-22  Myles C. Maxfield  <mmaxfi...@apple.com>
 
+        [WebGPU] Implement Queue::writeBuffer()
+        https://bugs.webkit.org/show_bug.cgi?id=238194
+
+        Reviewed by Kimmo Kinnunen.
+
+        The semantics of writeBuffer() are that the data should be written, but specifically
+        at the current point in the queue. So, we can't just naively memcpy(), for 2 reasons:
+        1. If the buffer is in-use, memcpy() will clobber its current contents, leading to
+                corruption. Instead, it's supposed to populate the buffer at the current point
+                in the queue, as-if it was a queue item.
+        2. If the buffer is a private buffer, we can't memcpy() to it anyway
+
+        So, the implementation of this function first tries to see if we can memcpy(): if the
+        buffer is shared or managed, and if the queue is idle. If it can, then it does so, but
+        if it can't, it creates a temporary buffer and enqueues a queue command to copy the
+        contents of the temporary buffer into the destination buffer.
+
+        That copy command is part of a lazily created command buffer.
+
+        Test: api/validation/queue/writeBuffer.spec.ts
+
+        * WebGPU/Buffer.h:
+        (WebGPU::Buffer::state const):
+        * WebGPU/Device.h:
+        (WebGPU::Device::device const):
+        * WebGPU/Queue.h:
+        (WebGPU::Queue::isIdle const):
+        * WebGPU/Queue.mm:
+        (WebGPU::Queue::~Queue):
+        (WebGPU::Queue::ensureBlitCommandEncoder):
+        (WebGPU::Queue::finalizeBlitCommandEncoder):
+        (WebGPU::Queue::onSubmittedWorkDone):
+        (WebGPU::Queue::commitMTLCommandBuffer):
+        (WebGPU::Queue::submit):
+        (WebGPU::validateWriteBufferInitial):
+        (WebGPU::Queue::validateWriteBuffer const):
+        (WebGPU::Queue::writeBuffer):
+
+2022-03-22  Myles C. Maxfield  <mmaxfi...@apple.com>
+
         [WebGPU] Remove the double pointer indirection in front of all objects
         https://bugs.webkit.org/show_bug.cgi?id=238001
 

Modified: trunk/Source/WebGPU/WebGPU/Buffer.h (291687 => 291688)


--- trunk/Source/WebGPU/WebGPU/Buffer.h	2022-03-22 19:04:20 UTC (rev 291687)
+++ trunk/Source/WebGPU/WebGPU/Buffer.h	2022-03-22 19:45:56 UTC (rev 291688)
@@ -76,6 +76,7 @@
     id<MTLBuffer> buffer() const { return m_buffer; }
     uint64_t size() const { return m_size; }
     WGPUBufferUsageFlags usage() const { return m_usage; }
+    State state() const { return m_state; }
 
 private:
     Buffer(id<MTLBuffer>, uint64_t size, WGPUBufferUsageFlags, State initialState, MappingRange initialMappingRange, Device&);

Modified: trunk/Source/WebGPU/WebGPU/Device.h (291687 => 291688)


--- trunk/Source/WebGPU/WebGPU/Device.h	2022-03-22 19:04:20 UTC (rev 291687)
+++ trunk/Source/WebGPU/WebGPU/Device.h	2022-03-22 19:45:56 UTC (rev 291688)
@@ -89,6 +89,8 @@
     void setUncapturedErrorCallback(Function<void(WGPUErrorType, String&&)>&&);
     void setLabel(String&&);
 
+    id<MTLDevice> device() const { return m_device; }
+
     void generateAValidationError(String&& message);
 
     Instance& instance() const { return m_instance; }

Modified: trunk/Source/WebGPU/WebGPU/Queue.h (291687 => 291688)


--- trunk/Source/WebGPU/WebGPU/Queue.h	2022-03-22 19:04:20 UTC (rev 291687)
+++ trunk/Source/WebGPU/WebGPU/Queue.h	2022-03-22 19:45:56 UTC (rev 291688)
@@ -64,11 +64,20 @@
     Queue(id<MTLCommandQueue>, Device&);
 
     bool validateSubmit() const;
+    bool validateWriteBuffer(const Buffer&, uint64_t bufferOffset, size_t) const;
 
+    void ensureBlitCommandEncoder();
+    void finalizeBlitCommandEncoder();
+
+    void commitMTLCommandBuffer(id<MTLCommandBuffer>);
+    bool isIdle() const { return m_submittedCommandBufferCount == m_completedCommandBufferCount; }
+
     // This can be called on a background thread.
     void scheduleWork(Instance::WorkItem&&);
 
     const id<MTLCommandQueue> m_commandQueue { nil };
+    id<MTLCommandBuffer> m_commandBuffer { nil };
+    id<MTLBlitCommandEncoder> m_blitCommandEncoder { nil };
     Device& m_device; // The only kind of queues that exist right now are default queues, which are owned by Devices.
 
     uint64_t m_submittedCommandBufferCount { 0 };

Modified: trunk/Source/WebGPU/WebGPU/Queue.mm (291687 => 291688)


--- trunk/Source/WebGPU/WebGPU/Queue.mm	2022-03-22 19:04:20 UTC (rev 291687)
+++ trunk/Source/WebGPU/WebGPU/Queue.mm	2022-03-22 19:45:56 UTC (rev 291688)
@@ -39,8 +39,43 @@
 {
 }
 
-Queue::~Queue() = default;
+Queue::~Queue()
+{
+    // If we're not idle, then there's a pending completed handler to be run,
+    // but the completed handler should have retained us,
+    // which means we shouldn't be being destroyed.
+    // So we must be idle.
+    ASSERT(isIdle());
 
+    // We can't actually call finalizeBlitCommandEncoder() here because, if there are pending copies,
+    // that would cause them to be committed, which ends up retaining this in the completed handler.
+    // It's actually fine, though, because we can just drop any pending copies on the floor.
+    // If the queue is being destroyed, this is unobservable.
+    if (m_blitCommandEncoder)
+        [m_blitCommandEncoder endEncoding];
+}
+
+void Queue::ensureBlitCommandEncoder()
+{
+    if (m_blitCommandEncoder)
+        return;
+
+    auto *commandBufferDescriptor = [MTLCommandBufferDescriptor new];
+    commandBufferDescriptor.errorOptions = MTLCommandBufferErrorOptionEncoderExecutionStatus;
+    m_commandBuffer = [m_commandQueue commandBufferWithDescriptor:commandBufferDescriptor];
+    m_blitCommandEncoder = [m_commandBuffer blitCommandEncoder];
+}
+
+void Queue::finalizeBlitCommandEncoder()
+{
+    if (m_blitCommandEncoder) {
+        [m_blitCommandEncoder endEncoding];
+        commitMTLCommandBuffer(m_commandBuffer);
+        m_blitCommandEncoder = nil;
+        m_commandBuffer = nil;
+    }
+}
+
 void Queue::onSubmittedWorkDone(uint64_t, CompletionHandler<void(WGPUQueueWorkDoneStatus)>&& callback)
 {
     // https://gpuweb.github.io/gpuweb/#dom-gpuqueue-onsubmittedworkdone
@@ -47,7 +82,9 @@
 
     ASSERT(m_submittedCommandBufferCount >= m_completedCommandBufferCount);
 
-    if (m_submittedCommandBufferCount == m_completedCommandBufferCount) {
+    finalizeBlitCommandEncoder();
+
+    if (isIdle()) {
         scheduleWork([callback = WTFMove(callback)]() mutable {
             callback(WGPUQueueWorkDoneStatus_Success);
         });
@@ -72,6 +109,21 @@
     return true;
 }
 
+void Queue::commitMTLCommandBuffer(id<MTLCommandBuffer> commandBuffer)
+{
+    ASSERT(commandBuffer.commandQueue == m_commandQueue);
+    [commandBuffer addCompletedHandler:[protectedThis = Ref { *this }](id<MTLCommandBuffer>) {
+        protectedThis->scheduleWork(CompletionHandler<void(void)>([protectedThis = protectedThis.copyRef()]() {
+            ++(protectedThis->m_completedCommandBufferCount);
+            for (auto& callback : protectedThis->m_onSubmittedWorkDoneCallbacks.take(protectedThis->m_completedCommandBufferCount))
+                callback(WGPUQueueWorkDoneStatus_Success);
+        }, CompletionHandlerCallThread::MainThread));
+    }];
+
+    [commandBuffer commit];
+    ++m_submittedCommandBufferCount;
+}
+
 void Queue::submit(Vector<std::reference_wrapper<const CommandBuffer>>&& commands)
 {
     // https://gpuweb.github.io/gpuweb/#dom-gpuqueue-submit
@@ -83,30 +135,111 @@
         return;
     }
 
+    finalizeBlitCommandEncoder();
+
     // "For each commandBuffer in commandBuffers:"
     for (auto commandBuffer : commands) {
-        ASSERT(commandBuffer.get().commandBuffer().commandQueue == m_commandQueue); //
-        [commandBuffer.get().commandBuffer() addCompletedHandler:[protectedThis = Ref { *this }](id<MTLCommandBuffer>) {
-            protectedThis->scheduleWork(CompletionHandler<void(void)>([protectedThis = protectedThis.copyRef()]() {
-                ++(protectedThis->m_completedCommandBufferCount);
-                for (auto& callback : protectedThis->m_onSubmittedWorkDoneCallbacks.take(protectedThis->m_completedCommandBufferCount))
-                    callback(WGPUQueueWorkDoneStatus_Success);
-            }, CompletionHandlerCallThread::MainThread));
-        }];
-
         // "Execute each command in commandBuffer.[[command_list]]."
-        [commandBuffer.get().commandBuffer() commit];
+        commitMTLCommandBuffer(commandBuffer.get().commandBuffer());
     }
+}
 
-    m_submittedCommandBufferCount += commands.size();
+static bool validateWriteBufferInitial(size_t size)
+{
+    // "contentsSize ≥ 0."
+
+    // "dataOffset + contentsSize ≤ dataSize."
+
+    // "contentsSize, converted to bytes, is a multiple of 4 bytes."
+    if (size % 4)
+        return false;
+
+    return true;
 }
 
+bool Queue::validateWriteBuffer(const Buffer& buffer, uint64_t bufferOffset, size_t size) const
+{
+    // FIXME: "buffer is valid to use with this."
+
+    // "buffer.[[state]] is unmapped."
+    if (buffer.state() != Buffer::State::Unmapped)
+        return false;
+
+    // "buffer.[[usage]] includes COPY_DST."
+    if (!(buffer.usage() & WGPUBufferUsage_CopyDst))
+        return false;
+
+    // "bufferOffset, converted to bytes, is a multiple of 4 bytes."
+    if (bufferOffset % 4)
+        return false;
+
+    // "bufferOffset + contentsSize, converted to bytes, ≤ buffer.[[size]] bytes."
+    // FIXME: Use checked arithmetic
+    if (bufferOffset + size > buffer.size())
+        return false;
+
+    return true;
+}
+
 void Queue::writeBuffer(const Buffer& buffer, uint64_t bufferOffset, const void* data, size_t size)
 {
-    UNUSED_PARAM(buffer);
-    UNUSED_PARAM(bufferOffset);
-    UNUSED_PARAM(data);
-    UNUSED_PARAM(size);
+    // https://gpuweb.github.io/gpuweb/#dom-gpuqueue-writebuffer
+
+    // "If data is an ArrayBuffer or DataView, let the element type be "byte". Otherwise, data is a TypedArray; let the element type be the type of the TypedArray."
+
+    // "Let dataSize be the size of data, in elements."
+
+    // "If size is missing, let contentsSize be dataSize − dataOffset. Otherwise, let contentsSize be size."
+
+    // "If any of the following conditions are unsatisfied"
+    if (!validateWriteBufferInitial(size)) {
+        // FIXME: "throw OperationError and stop."
+        return;
+    }
+
+    // "Let dataContents be a copy of the bytes held by the buffer source."
+
+    // "Let contents be the contentsSize elements of dataContents starting at an offset of dataOffset elements."
+
+    // "If any of the following conditions are unsatisfied"
+    if (!validateWriteBuffer(buffer, bufferOffset, size)) {
+        // "generate a validation error and stop."
+        m_device.generateAValidationError("Validation failure."_s);
+        return;
+    }
+
+    // "Write contents into buffer starting at bufferOffset."
+    if (!size)
+        return;
+
+    // FIXME(PERFORMANCE): Instead of checking whether or not the whole queue is idle,
+    // we could detect whether this specific resource is idle, if we tracked every resource.
+    if (isIdle()) {
+        switch (buffer.buffer().storageMode) {
+        case MTLStorageModeShared:
+            memcpy(static_cast<char*>(buffer.buffer().contents) + bufferOffset, data, size);
+            return;
+#if PLATFORM(MAC) || PLATFORM(MACCATALYST)
+        case MTLStorageModeManaged:
+            memcpy(static_cast<char*>(buffer.buffer().contents) + bufferOffset, data, size);
+            [buffer.buffer() didModifyRange:NSMakeRange(bufferOffset, size)];
+            return;
+#endif
+        case MTLStorageModePrivate:
+            // The only way to get data into a private resource is to tell the GPU to copy it in.
+            break;
+        default:
+            ASSERT_NOT_REACHED();
+            return;
+        }
+    }
+
+    ensureBlitCommandEncoder();
+    // FIXME(PERFORMANCE): Suballocate, so the common case doesn't need to hit the kernel.
+    id<MTLBuffer> temporaryBuffer = [m_device.device() newBufferWithBytes:data length:static_cast<NSUInteger>(size) options:MTLResourceStorageModeShared];
+    if (!temporaryBuffer)
+        return;
+    [m_blitCommandEncoder copyFromBuffer:temporaryBuffer sourceOffset:0 toBuffer:buffer.buffer() destinationOffset:static_cast<NSUInteger>(bufferOffset) size:static_cast<NSUInteger>(size)];
 }
 
 void Queue::writeTexture(const WGPUImageCopyTexture& destination, const void* data, size_t dataSize, const WGPUTextureDataLayout& dataLayout, const WGPUExtent3D& writeSize)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to