Skip to content
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.

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

Make UNPACK_COLORSPACE_CONVERSION_WEBGL=FALSE apply to all content #3683

Closed
ccameron-chromium opened this issue Aug 13, 2024 · 5 comments · Fixed by #3689
Closed

Make UNPACK_COLORSPACE_CONVERSION_WEBGL=FALSE apply to all content #3683

ccameron-chromium opened this issue Aug 13, 2024 · 5 comments · Fixed by #3689
Assignees

Comments

@ccameron-chromium
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

ccameron-chromium commented Aug 13, 2024

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
Copy link
Contributor

kdashg commented Aug 22, 2024

I'll draft this spec change.

@ccameron-chromium
Copy link
Contributor Author

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
Copy link
Member

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.

kdashg added a commit to kdashg/WebGL that referenced this issue Sep 5, 2024
kenrussell pushed a commit that referenced this issue Sep 19, 2024
…t images. (#3689)

UNPACK_COLORSPACE_CONVERSION_WEBGL now applies to all inputs, not just images, but is
restricted to TexImageSource. (excludes e.g. ArrayBufferViews)

Fixes #3683.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants