Provide getSubData() as a sibling to setSubData() - #213

How would this be beneficial over mapReadAsync? both return a Promise. The fact that mapReadAsync doesn't specify the range will be addressed when we get the buffer sub-views and can currently be worked around rather easily by having a smaller buffer that the user copies the data into before reading it.

Note: setSubData only makes sense because it's a pseudo-synchronous operation from the user perspective, unlike mapWriteAsync that requires them to deal with a promise.

The author doesn’t have to call unmap, and calling this function doesn’t cause future calls to map() to fail. This call doesn’t participate in the buffer lifecycle timeline; it’s a one-shot read.

I would prefer not to have any API that generates garbage like this -- why not have the user pass in an ArrayBuffer which will get filled in instead?

@magcius An implementation that has separate content and GPU processes and doesn't have a way to share CPU-visible driver-managed memory between those processes would have to make an extra copy of the data if it doesn't create the ArrayBuffer contents. If it can create the ArrayBuffer, then at least it can wrap the process-shared memory (containing the read data) in it.

FWIW WebGL has a similar getBufferSubData that writes into an existing ArrayBuffer. Avoiding the additional copy would be great, but the user could still write to the ArrayBuffer returned, so I'm not sure if it's possible to know this upfront to avoid the copy.

I don't know a great solution for @magcius's concern, but I agree that it will be important to try to avoid temporary Promises and ArrayBuffers (and any temporary views into them) from any high frequency operations (per-frame if possible). A lot of the use cases targeted by WebGPU will probably try to avoid noticeable GC events from temporaries. It looks like WebAudio had some similar concerns in WebAudio/web-audio-api#373

I'm not sure if there is any precedent for using SharedArrayBuffer-backed views (I found https://github.com/WebAudio/web-audio-api/issues/1713 for example), or some other alternative to support neutered ArrayBuffers or similar?

Let me file a separate bug about the GC concerns -- looking at the sketch.idl for the first time in a bit, it's certainly not the only offender. Sorry for the off-topic spam.

Note that this entry point isn't a subject for high-performance code: like setSubData it's only meant to be a convenience helper to get something working quickly. We shouldn't go out of our way (and sacrifice convenience) here, and instead will just recommend doing the mapping explicitly in areas that matter.

Yes, @kvark is totally right.

I assume getSubDate would require the buffer to have the TRANSFER_SRC usage given at creation?

Overall I feel there is much less a need for this compared to setSubData as a convenience function for beginners. Getting data back from the GPU is already simpler then uploading data, it is something that's less essential when you get started, and one can write a polyfill for getSubData in less than 10 lines of code:

function getSubData(device, buffer, offset, size) {
    const readbackBuffer = device.createBuffer({size, usage: TRANSFER_DST | MAP_READ});

    const commands = device.createCommandEncoder();
    commands.copyBufferToBuffer(buffer, offset, readbackBuffer, 0, size);
    device.getQueue().submit([commands.finish()]);

    return readbackBuffer.mapReadAsync();
}

Am I missing something?

@Kangz wouldn't the same argument apply to setSubData? It's also poly-fillable with the same amount of code:

function setSubData(device, buffer, offset, data) {
    const (writeBuffer, writeArray) = device.createBufferMapped({size: data.size, usage: TRANSFER_SRC | MAP_WRITE});
    writeArray.copyFrom(data); // or something like that...
    writeBuffer.unmap(); //Note: missing from your `getSubData` code, would be on the user side
    const commands = device.createCommandEncoder();
    commands.copyBufferToBuffer(writeBuffer, 0, buffer, offset, data.size);
    device.getQueue().submit([commands.finish()]);
}

It does indeed, I didn't think of the newly added createBufferMapped.

If these helper functions can be polyfilled in JavaScript without too much effort, I think it's better to remove them from the WebGPU IDL.

@grovesNL I agree. It would be more useful to have set_buffer_sub_data on a command buffer timeline, in order for some of the backends to implement this more efficiently than a polyfill.

Discussed at the 11 Mar 2019 teleconference

Discussed at 25 March teleconference

Discussed at the 29 April 2019 meeting.

We centered on not having this in the MVP, though we can always investigate things in more detail at a later point.