KhronosGroup / WebGL

The Official Khronos WebGL Repository
Other
2.63k stars 668 forks source link

Make UNPACK_COLORSPACE_CONVERSION_WEBGL=FALSE apply to all content #3683

Open ccameron-chromium opened 1 month ago

ccameron-chromium commented 1 month ago

For texImage2D, the spec says

First, the source image data is conceptually converted to the color space specified by the unpackColorSpace attribute, except if the source image data is an HTMLImageElement, and the UNPACK_COLORSPACE_CONVERSION_WEBGL pixel storage parameter is set to NONE.

This proposal is to remove the "except if the source image data is an HTMLImageElement" part, and instead have this read:

First, the source image data is conceptually converted to the color space specified by the unpackColorSpace attribute. If any of the following conditions apply then this color space conversion is skipped, and only YUV to RGB conversion is performed (if applicable):

  • The UNPACK_COLORSPACE_CONVERSION_WEBGL pixel storage parameter is set to NONE
  • The source image data is an ImageBitmap that was created with colorSpaceConversion set to "none".

Prior to this change, ImageData, HTMLCanvasElement, HTMLVideoElement, and ImageBitmap have no way to "opt out" of color conversion. With this change, they do.

The motivation for only having HTMLImageElement be affected by UNPACK_COLORSPACE_CONVERSION_WEBGL=NONE is discussed in this comment. Those motivations are now obsolete.

We do want to be clear that any data that is a YUV format is always converted from YUV to RGB. This applies to images as well as videos (we often think of videos as being stored in YUV, but most images are, too).

kenrussell commented 1 month ago

For ImageBitmap specifically, even if colorSpaceConversion was default in the dictionary of options during createImageBitmap, it's infeasible to have UNPACK_COLORSPACE_CONVERSION_WEBGL=NONE affect the result. The colorspace conversion is baked into the ImageBitmap during its construction, and the browser can't reconstruct the ImageBitmap again from its source if the user requested a different option than before.

I think the carve-out in the Pixel Storage Parameters section of the spec should remain for all ImageBitmaps, not just those created with colorSpaceConversion: none.

ccameron-chromium commented 1 month ago

The colorspace conversion is baked into the ImageBitmap during its construction

There should be no color space conversion done during createImageBitmap. There is a Chromium-specific bug where createImageBitmap crushes everything to 8-bit sRGB, but this is a bug and not a specced behavior. I've been hoping to find the time to fix that bug for a while, but haven't. I actually will need to get to it soon, because it's causing issues for partners.

We can remove all of the ImageBitmap carve-out without breaking any backwards compatibility except for applications that are also setting unpackColorSpace. I would prefer to do that (and might even advocate for gathering usage stats to see if we can do it without breaking anything).

kdashg commented 3 weeks ago

I'll draft this spec change.

ccameron-chromium commented 2 weeks ago

Thanks!!

There is a similar issue over in createImageBitmap in the HTML spec, where it seems to have been written assuming only images can be non-sRGB. I've filed this issue and written this patch.

Also, this adds a feature to WebGL that WebGPU doesn't have, so I filed this issue for WebGPU to get a UNPACK_COLORSPACE_CONVERSION_WEBGL=FALSE parameter.

kenrussell commented 2 weeks ago

After the WebGL working group meetings of 2024-08-08 and 2024-08-22, the consensus among browser vendors was to move in this direction, as ImageBitmap's implementations have improved in recent years and there's no significant cost to potentially doing a last-mile colorspace adjustment while uploading the ImageBitmap to a WebGL texture. Thanks in advance @kdashg for writing up the change.