Repository: google/filament
File: filament/backend/src/webgpu/WebGPUBufferBase.cpp (C++)

Code before:

// Updates the GPU buffer with data from a BufferDescriptor.
// WebGPU requires that the size of the data written to a buffer is a multiple of 4.
// This function handles cases where the buffer descriptor's size is not a multiple of 4
// by writing the bulk of the data first, and then copying the remaining bytes into a
// padded temporary chunk which is then written to the buffer.
void WebGPUBufferBase::updateGPUBuffer(BufferDescriptor const& bufferDescriptor,
        const uint32_t byteOffset, wgpu::Queue const& queue) {
    FILAMENT_CHECK_PRECONDITION(bufferDescriptor.buffer)
            << "updateGPUBuffer called with a null buffer";
    FILAMENT_CHECK_PRECONDITION(bufferDescriptor.size + byteOffset <= mBuffer.GetSize())
            << "Attempting to copy " << bufferDescriptor.size << " bytes into a buffer of size "
            << mBuffer.GetSize() << " at offset " << byteOffset;
    FILAMENT_CHECK_PRECONDITION(byteOffset % FILAMENT_WEBGPU_BUFFER_SIZE_MODULUS == 0)
            << "Byte offset must be a multiple of " << FILAMENT_WEBGPU_BUFFER_SIZE_MODULUS
            << " but is " << byteOffset;

    // TODO: All buffer objects are created with CopyDst usage.
    // This may have some performance implications. That should be investigated later.
    assert_invariant(mBuffer.GetUsage() & wgpu::BufferUsage::CopyDst);

    // create the staging buffer
    wgpu::BufferDescriptor descriptor{
        .label = "staging buffer",
        .usage =  wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc,
        .size = mBuffer.GetSize(),
        .mappedAtCreation = true };
    wgpu::Buffer stagingBuffer = mDevice.CreateBuffer(&descriptor);
    std::cout << "Run Yu: creating a new staging buffer, with size of " << descriptor.size << std::endl;

    // Calculate some alignment related sizes
    const size_t copySizeRemainder = bufferDescriptor.size % FILAMENT_WEBGPU_BUFFER_SIZE_MODULUS;
    const size_t legalCopySize = bufferDescriptor.size - copySizeRemainder;
    // WGPU requires the offset of GetMappedRange to be a multiple of 8
    const int stagingOffsetShift = -(byteOffset % FILAMENT_WEBGPU_MAPPED_RANGE_OFFSET_MODULUS);

    void *mappedRange = stagingBuffer.GetMappedRange(byteOffset + stagingOffsetShift);
    // copy the data to the staging buffer
    memcpy(mappedRange, bufferDescriptor.buffer, bufferDescriptor.size);

    stagingBuffer.Unmap();

    // Copy the staging buffer contents to the destination buffer.
    wgpu::CommandEncoderDescriptor commandEncodeDescriptor = {};
    commandEncodeDescriptor.label = "copy buffer to buffer";
    const wgpu::CommandEncoder commandEncoder = mDevice.CreateCommandEncoder(&commandEncodeDescriptor);
    commandEncoder.CopyBufferToBuffer(
        stagingBuffer,
        byteOffset + stagingOffsetShift,
        mBuffer,
        byteOffset,

Review comment (suggestion):
I think the staging buffer should be of the `bufferDescriptor.size`.  

So imagine buffer A is of size X.  Update is called with size Y and offset O, what we want is to 
``` 
update A starting at O and copy Y bytes
```
But here `mBuffer.GetSize() == X`.

Code after:

// Updates the GPU buffer with data from a BufferDescriptor.
// WebGPU requires that the size of the data copied from the staging buffer to the GPU buffer is a
// multiple of 4. This function handles cases where the buffer descriptor's size is not a multiple
// of 4 by padding with zeros.
void WebGPUBufferBase::updateGPUBuffer(BufferDescriptor const& bufferDescriptor,
        const uint32_t byteOffset, wgpu::Device const& device,
        WebGPUQueueManager* const webGPUQueueManager) {
    FILAMENT_CHECK_PRECONDITION(bufferDescriptor.buffer)
            << "updateGPUBuffer called with a null buffer";
    FILAMENT_CHECK_PRECONDITION(bufferDescriptor.size + byteOffset <= mBuffer.GetSize())
            << "Attempting to copy " << bufferDescriptor.size << " bytes into a buffer of size "
            << mBuffer.GetSize() << " at offset " << byteOffset;
    FILAMENT_CHECK_PRECONDITION(byteOffset % FILAMENT_WEBGPU_BUFFER_SIZE_MODULUS == 0)
            << "Byte offset must be a multiple of " << FILAMENT_WEBGPU_BUFFER_SIZE_MODULUS
            << " but is " << byteOffset;

    // TODO: All buffer objects are created with CopyDst usage.
    // This may have some performance implications. That should be investigated later.
    assert_invariant(mBuffer.GetUsage() & wgpu::BufferUsage::CopyDst);

    // Calculate some alignment related sizes
    const size_t remainder = bufferDescriptor.size % FILAMENT_WEBGPU_BUFFER_SIZE_MODULUS;
    const size_t mainBulk = bufferDescriptor.size - remainder;
    const size_t stagingBufferSize =
            remainder == 0 ? bufferDescriptor.size : mainBulk + FILAMENT_WEBGPU_BUFFER_SIZE_MODULUS;

    // create a staging buffer
    wgpu::BufferDescriptor descriptor{
        .label = "Filament WebGPU Staging Buffer",
        .usage = wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc,
        .size = stagingBufferSize,
        .mappedAtCreation = true };
    wgpu::Buffer stagingBuffer = device.CreateBuffer(&descriptor);

    void* mappedRange = stagingBuffer.GetMappedRange();
    memcpy(mappedRange, bufferDescriptor.buffer, bufferDescriptor.size);

    // Make sure the padded memory is set to 0 to have deterministic behaviors
    if (remainder != 0) {
        uint8_t* paddingStart = static_cast<uint8_t*>(mappedRange) + bufferDescriptor.size;
        memset(paddingStart, 0, FILAMENT_WEBGPU_BUFFER_SIZE_MODULUS - remainder);
    }

    stagingBuffer.Unmap();

    // Copy the staging buffer contents to the destination buffer.
    webGPUQueueManager->getCommandEncoder().CopyBufferToBuffer(stagingBuffer, 0, mBuffer,
            byteOffset, stagingBufferSize);
}