gpuweb icon indicating copy to clipboard operation
gpuweb copied to clipboard

Add HTMLImageElement and ImageData to copyExternalImageToTexture

Open kainino0x opened this issue 2 years ago • 3 comments

HTMLImageElement requested by @kdashg , ImageData added to complete symmetry with TexImageSource WebGL (except for VideoFrame).

kainino0x avatar Sep 09 '22 23:09 kainino0x

Previews, as seen when this build job started (8fdb28a62789bb802366a693c6651605d3911098): WebGPU webgpu.idl | Explainer | Correspondence Reference WGSL grammar.js | wgsl.lalr.txt

github-actions[bot] avatar Sep 09 '22 23:09 github-actions[bot]

Some notes:

  • I think adding HTMLVideoElement support (#3410) was an easy sell - HTMLVideoElement is the only representation of a video, I think (except VideoFrame), so adding that strictly removed a middleperson (ImageBitmap). HTMLImageElement is not so straightforward: the often-recommended path for getting an image into an ImageBitmap is actually through createImageBitmap(Blob), not createImageBitmap(HTMLImageElement). HTMLImageElement.decode() is not semantically the same as createImageBitmap(), even though both are async:

    • HTMLImageElement in general, and decode() in particular, imply the the image should be prepared for the DOM: "resolves when the image is decoded and it is safe to append the image to the DOM".
    • ImageBitmap is used as a copy source (or bitmaprenderer frame) and therefore createImageBitmap() implies it should be prepared for copy. In browsers, this may or may not be the same thing.

    For this reason, the Fetch-Blob-ImageBitmap-WebGPU path is arguably more direct than an HTMLImageElement(-decode)-WebGPU path. Of course, a Fetch-Blob-WebGPU path would be even more direct....

  • ImageData is a relatively simple overload, though not trivial: It looks like it can be easily implemented on top of writeTexture, but it actually can't because ImageData is color-managed and writeTexture is not.

kainino0x avatar Sep 10 '22 00:09 kainino0x

I propose punting these decisions to post-v1.

kainino0x avatar Sep 10 '22 00:09 kainino0x

This is probably tolerable for images, for which uploads should be cold code.

kdashg avatar Nov 16 '22 08:11 kdashg

GPU Web meeting 2022-11-16 non-APAC-timed
  • KG: probably tolerable for image uploads to be post-V1. OK for them to be slow. Main concern - having to go through ImageBitmaps is harder to optimize for Firefox. But we don't care about this for images as much. So probably fine.
  • KN: there are things I'd like to do eventually, but not for V1.
  • KR: agree this isn't a performance-critical code path.

kdashg avatar Nov 29 '22 02:11 kdashg

meeting: Tentative agreement for now, but google wants to think a bit more about the ImageData overload

kainino0x avatar Jul 12 '23 23:07 kainino0x

Also tagging as likely polish-post-v1 (i.e. we can land this and then implementations will go ahead with it whenever they're ready)

kainino0x avatar Jul 12 '23 23:07 kainino0x

I want to verify my understanding.

In WebGL I can set gl.pixelStorei(gl.UNPACK_COLORSPACE_CONVERSION_WEBGL, gl.NONE) and pass an HTMLImageElement to texImage2D and get the raw data from a .PNG image

In WebGPU, if we add support for HTMLImageElement, there is no such path. If I want to make sure to get the raw data from an image in WebGPU I have to first convert the image to an ImageBitmap

const ib = await createImageBitmap(someImageElement, { colorSpaceConversion: 'none' });

That is the only path that guarantees raw .PNG data doesn't have any color space applied in WebGPU. Correct?

greggman avatar Jul 21 '23 21:07 greggman

@greggman correct. The preferred case would be that your PNG is either untagged or tagged as sRGB, then the color conversion into sRGB will be a no-op.

Maybe there's a need for the NONE mode though?

kainino0x avatar Jul 25 '23 04:07 kainino0x

A PNG with srgb or none might have already been converted to display-p3 by the browser on a display-p3 display? I guess the spec could say it requires raw data, but you'd only be able to test on a device where you thought the browser might be converting color spaces for images it usually displays? I wonder how many WebGL apps out there are using .PNGs that would suddenly start providing different data if used in WebGPU if there was no flag

greggman avatar Jul 25 '23 15:07 greggman

A PNG with srgb or none might have already been converted to display-p3 by the browser on a display-p3 display?

We should be clear that a browser that does that is in error. (And is also leaking tons of fingerprinting bits). The color conversion to apply must be "the color space specified in the image (if any, sRGB if none)" to "the color space specified to copyExternalImageToTexture").

Of note is that colorSpaceConversion: 'none' was removed from WebCodecs shortly after it shipped. A better name for colorSpaceConversion: 'none' is ignoreImageICCProfiles: 'yes-please'.

ccameron-chromium avatar Jul 26 '23 19:07 ccameron-chromium

A better name for colorSpaceConversion: 'none' is ignoreImageICCProfiles: 'yes-please'.

That sounds very specific. There are many other forms of color profile / color space metadata. It seems better to have a flag that says to ignore all of them, rather than a flag to ignore just one of them. The intent is "give me the raw uncompressed data from the file" most likely because the data is not colors.

greggman avatar Jul 27 '23 16:07 greggman

Of note is that colorSpaceConversion: 'none' was removed from WebCodecs shortly after it shipped.

Sorry, this is incorrect. I was thinking of premultiply alpha.

ccameron-chromium avatar Jul 31 '23 15:07 ccameron-chromium

A better name for colorSpaceConversion: 'none' is ignoreImageICCProfiles: 'yes-please'.

That sounds very specific. There are many other forms of color profile / color space metadata. It seems better to have a flag that says to ignore all of them, rather than a flag to ignore just one of them. The intent is "give me the raw uncompressed data from the file" most likely because the data is not colors.

Yes. There are ICC profiles and CICP chunks.

For ImageBitmap, the HTML spec says:

When the steps above require that the user agent crop bitmap data to the source rectangle with formatting, the user agent must run the following steps: [...] 9. If image is an img element or a Blob object, let val be the value of the colorSpaceConversion member of options, and then run these substeps:

  • If val is "default", the color space conversion behavior is implementation-specific, and should be chosen according to the default color space that the implementation uses for drawing images onto the canvas.
  • If val is "none", output must be decoded without performing any color space conversions. This means that the image decoding algorithm must ignore color profile metadata embedded in the source data as well as the display device color profile.

This only applies to HTMLImageElement and Blob (so it doesn't apply to videos, canvases, etc). The part about this only applying when a crop is required is terrifying, and I'm not sure what to do with it.

So a more realistic name would be ignoreImageColorProfileMetadata:true.

For reference, I added a test page (comparing 2D, WebGL, and WebGPU) here.

Over in another thread, junov@ suggested that this was added to compensate for a bug where Chrome would automatically convert images to the display's color space (and this was a flag to suppress that behavior). Chrome never does that anymore (it's a fingerprinting vector, if nothing else), so maybe this just isn't needed anymore (in any of its various incarnations).

ccameron-chromium avatar Jul 31 '23 15:07 ccameron-chromium