copyExternalImageToTexture for ImageBitmap and canvases #1581
Conversation
|
|
kainino0x
force-pushed
the
copyToTexture
branch
4 times, most recently
from
9d8d43f to
b439eeb
Compare
kainino0x
force-pushed
the
copyToTexture
branch
from
b439eeb to
95258e2
Compare
|
Lots of open issues, but PTAL |
kainino0x
added
for webgpu editors meeting
and removed
for webgpu editors meeting
labels
kainino0x
changed the title
|
Renamed to copyExternalImageToTexture to make this more specific. |
kenrussell
left a comment
kenrussell
left a comment
There was a problem hiding this comment.
Looks good to me! Will be good to move forward with this so that conformance tests and implementations can follow. A few small comments.
| ImageBitmap, canvas, and VideoElement are all color-managed: they encode colors. | ||
| GPUTexture is not color-managed: it encodes raw numbers. Producing raw numbers requires knowing | ||
| the target encoding. Probably there should be a particular default value (e.g. the default profile | ||
| used by the browser for unmanaged content?), but eventually we'll want to add knobs. |
There was a problem hiding this comment.
Per recent updates to:
https://github.com/WICG/canvas-color-space/blob/main/CanvasColorSpaceProposal.md
specifically:
WICG/canvas-color-space#54
which was WebGL-specific, it might be possible to define this behavior for WebGPU from the colorSpace configured on the GPUSwapChain. Specifically:
- The destination color space for uploads of HTMLCanvasElement and HTMLVideoElement is the
colorSpaceconfigured on the GPUSwapChain. - No conversion is performed for ImageBitmap, since those are intended to be configured properly at their creation time via ImageBitmapOptions.
There was a problem hiding this comment.
- The destination color space for uploads of HTMLCanvasElement and HTMLVideoElement is the
colorSpaceconfigured on the GPUSwapChain.
Unfortunately we can't pick a default from the swap chain because there could be 0 or >1 swap chains for a GPUDevice.
There was a problem hiding this comment.
I see.
In that case it seems to me that we might not need to have any controls in this copying path.
- For HTMLVideoElement it's always desired to do the browser's default color space conversion.
- For canvases and offscreen canvases, the source canvas's context provides enough control over the color space. The copy to a GPUTexture can be a passthrough with no color space conversion.
- For ImageBitmap, the creation-time options provide the controls needed - and these are being enhanced in the CanvasColorSpaceProposal.
kainino0x
force-pushed
the
copyToTexture
branch
from
ebc4f61 to
31e6518
Compare
kainino0x
changed the title
kainino0x
changed the title
|
Per the editors' discussion, I have removed the video overload from this PR. I've split it into the (dependent) PR #1647 |
| - |copySize|.[=Extent3D/depthOrArrayLayers=] must be `1`. | ||
| - |destination|.{{GPUImageCopyTexture/texture}}.{{GPUTexture/[[textureUsage]]}} | ||
| must include both {{GPUTextureUsage/RENDER_ATTACHMENT}} and | ||
| {{GPUTextureUsage/COPY_DST}}. |
There was a problem hiding this comment.
Note: We originally added RENDER_ATTACHMENT with video in mind, but I think it's still necessary without, because there is still a possible format conversion in this call (e.g. from rgba8unorm to rg16float, or from rgba16float to rgb10a2unorm).
kainino0x
force-pushed
the
copyToTexture
branch
from
8e4ae4b to
4d60238
Compare
kainino0x
force-pushed
the
copyToTexture
branch
from
d580abe to
023b579
Compare
|
Editors meeting: decided to land this as it's the direction we agreed upon in the VF2F, and there are no surprises here, only details (like that it requires COPY_DST and RENDER_ATTACHMENT). |
5 participants
Realized I already had this sketched out on a local branch, should have pushed it up earlier.
This does not solve #1483 yet.
With bonus minor copyediting, because I can't help myself. Needs a bit more work later though.
Fixes #1154
Preview | Diff