这是indexloc提供的服务，不要输入任何密码
Skip to content

GPUQueue.writeBuffer/writeTexture #509

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

GPUQueue.writeBuffer/writeTexture #509

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2bc2178
setBufferSubData and setTextureSubData
kvark Dec 3, 2019
c4d4db0
Address Corentin's comments
kvark Dec 3, 2019
bf10b1d
Address Freek's comments
kvark Dec 3, 2019
38bec47
Common GPUTextureDataLayout, address Kai's comments
kvark Dec 4, 2019
b6a597a
Re-introduce GPUBufferCopyView
kvark Dec 5, 2019
f42a6b8
Address Rafael's comments
kvark Dec 10, 2019
d7cb553
Address Justin's review comments
kvark Feb 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 66 additions & 9 deletions spec/index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2255,11 +2255,8 @@ dictionary GPUCommandEncoderDescriptor : GPUObjectDescriptorBase {
## Copy Commands ## {#copy-commands}

<script type=idl>
dictionary GPUBufferCopyView {
dictionary GPUBufferCopyView: GPUTextureDataLayout {
required GPUBuffer buffer;
GPUSize64 offset = 0;
required GPUSize32 rowPitch;
required GPUSize32 imageHeight;
};
</script>

Expand Down Expand Up @@ -2481,27 +2478,87 @@ dictionary GPURenderBundleEncoderDescriptor : GPUObjectDescriptorBase {
Queues {#queues}
================

<script type=idl>
dictionary GPUTextureDataLayout {
GPUSize64 offset = 0;
required GPUSize32 rowPitch;
required GPUSize32 imageHeight;
};
</script>

<script type=idl>
interface GPUQueue {
void submit(sequence<GPUCommandBuffer> commandBuffers);

GPUFence createFence(optional GPUFenceDescriptor descriptor = {});
void signal(GPUFence fence, GPUFenceValue signalValue);

void writeBuffer(
ArrayBuffer sourceData,
GPUSize64 sourceOffset,
GPUBuffer destination,
GPUSize64 destinationOffset,
GPUSize64 size);

void writeTexture(
ArrayBuffer sourceData,
GPUTextureDataLayout sourceLayout,
Copy link
Contributor

@Kangz Kangz Dec 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this have the same 256 limitation as for copies between buffers and textures?

Copy link
Contributor Author

@kvark kvark Dec 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question!

I don't think it should have the limitation, since the data is coming from the CPU side, via a shortcut method, so we have both a room and a permit to manage the alignment automatically.

This could also be one more thing towards the list of why an API like this is useful. cc @jdashg

Copy link
Contributor

@kainino0x kainino0x Dec 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this too, I think it should not have the limitation.

Copy link
Contributor

@kdashg kdashg Dec 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it doesn't have the limitation, it must copy bloat when this happens, which chips away at the "good enough performance" claims.

Copy link
Contributor

@litherum litherum Feb 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume when @Kangz says "256 limitation" he's referring to buffer copies having to have strides that are a multiple of 256? (I ⌘f-ed for "256" in the spec and didn't find anything...)

If the source data just happens to have stride that is a multiple of 256, just by coincidence, then no copy bloat, right? Just like how, if writeBuffer() is called when the buffer just happens to be ready, just by coincidence, then no copy bloat either, right? It doesn't make a lot of sense to mandate 256 here when we aren't mandating an internal copy.

Perhaps this can be alleviated with additional spec text describing a set of steps which, if applications take, they are guaranteed to be on the fast path. These steps would say "use fences to know when your buffer is ready" and "make the stride a multiple of 256" (possibly among other things).

Copy link
Contributor

@Kangz Kangz Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comment wasn't super clear: the 256 limitation is around the rowPitchInBytes in GPUTextureDataLayout. copyBufferToTexture and copyTextureToBuffer require it to be a multiple of 256 and I was wondering if the same applied for writeTexture.

Just like how, if writeBuffer() is called when the buffer just happens to be ready, just by coincidence, then no copy bloat either, right?

That's an implementation detail, and at least in Chromium/Dawn we'd like to avoid doing it if we can (it requires knowing for each resource when's the last time they were used by the GPU, which we'd like to avoid tracking).

GPUTextureCopyView destination,
GPUExtent3D size);

void copyImageBitmapToTexture(
GPUImageBitmapCopyView source,
GPUTextureCopyView destination,
GPUExtent3D copySize);
GPUExtent3D size);
};
GPUQueue includes GPUObjectBase;
</script>

- {{GPUQueue/copyImageBitmapToTexture()}}:
- For now, `copySize.z` must be `1`.
<dl dfn-type="method" dfn-for="GPUQueue">
: <dfn>writeBuffer(sourceData, sourceOffset, destination, destinationOffset, size)</dfn>
::
Takes the `sourceData` contents starting from the offset `sourceOffset` and
schedules a write operation of these contents to the `destination` buffer in the queue.
Any subsequent modifications to sourceData do not affect what is written
at the time that the scheduled operation runs.
The operation does nothing and produces an error if any of the following is true:

- `destination` buffer wasn't created with {{GPUBufferUsage/COPY_DST}} flag in {{GPUBufferDescriptor/usage}}.
- `destination` buffer is destroyed
- `destinationOffset + size` exceeds the {{GPUBufferDescriptor/size}} of the `destination` buffer.
- `destination` buffer isn't in the `"unmapped"` [=buffer state=].
- `destinationOffset` is not a multiple of 4.
- `size` is not a positive multiple of 4.
- `sourceOffset + size` exceeds `sourceData.byteLength`.

: <dfn>writeTexture(sourceData, sourceLayout, destination, size)</dfn>
::
Takes the `sourceData` contents and schedules a write operation of these contents to the
`destination` texture copy view in the queue.
Any subsequent modifications to sourceData do not affect what is written
at the time that the scheduled operation runs.
The operation does nothing and produces an error if any of the following is true:

- `destination.texture` texture wasn't created with {{GPUTextureUsage/COPY_DST}} flag in {{GPUTextureDescriptor/usage}}.
- `destination.texture` is destroyed
- `destination.origin + size` exceeds the size of texture dimensions of the mipmap `destination.mipLevel`.
Copy link
Contributor

@RafaelCintron RafaelCintron Dec 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I presume that size is specified to be in bytes and origin in pixels? If so, then the operation destination.origin + size is using mismatched units and will not work, especially if there are padding pixels at the end of each scanline.

Copy link
Contributor

@kainino0x kainino0x Dec 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Size here is a GPUExtent3D in pixels. Unfortunately the types are not locally visible (you have to check the IDL)...

- any of the `size` extents are zero
- `sourceOffset + requiredSize` exceeds `sourceData.byteLength`, where `requiredSize` can be computed as
`(sourceLayout.imageHeight * (size.depth - 1) + size.height - 1) * sourceLayout.rowPitch + size.width * bpp`.

: <dfn>copyImageBitmapToTexture(source, destination, size)</dfn>
::
Schedules a copy operation of the contents of an image bitmap into the destination texture.

{{GPUQueue/submit(commandBuffers)}} does nothing and produces an error if any of the following is true:
- For now, `size.z` must be `1`.

- Any {{GPUBuffer}} referenced in any element of `commandBuffers` isn't in the `"unmapped"` [=buffer state=].
: <dfn>submit(commandBuffers)</dfn>
::
Schedules the execution of the command buffers by the GPU on this queue.
Does nothing and produces an error if any of the following is true:

- Any {{GPUBuffer}} referenced in any element of `commandBuffers` isn't in the `"unmapped"` [=buffer state=].
</dl>

## GPUFence ## {#fence}

Expand Down