KhronosGroup/WebGL

Specs ambiguous about when&how UNPACK_COLORSPACE_CONVERSION_WEBGL should apply, and interaction with unpackColorSpace

Closed this issue · 8 comments

squelart commented

Looking at the latest specs at https://github.com/KhronosGroup/WebGL/blob/22e4642d40cca87e6c050119c4f6a1f897214436/specs/latest/1.0/index.html , I see:

  • Under texImage2D:

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.

  • Under additional parameters to pixelStorei:

UNPACK_COLORSPACE_CONVERSION_WEBGL of type unsigned long
If set to BROWSER_DEFAULT_WEBGL, then the browser's default colorspace conversion is applied during subsequent texImage2D and texSubImage2D calls taking HTMLImageElement. The precise conversions may be specific to both the browser and file type. If set to NONE, no colorspace conversion is applied. The initial value is BROWSER_DEFAULT_WEBGL.

In pseudo-code, I would interpret the first snippet to mean:

if !(source == HTMLImageElement && UNPACK_COLORSPACE_CONVERSION_WEBGL == NONE)
    convert data to unpackColorSpace

And the second one:

if UNPACK_COLORSPACE_CONVERSION_WEBGL == BROWSER_DEFAULT_WEBGL
    if source == HTMLImageElement
        convert data to browser's default colorspace
    else
        ??
else
    no colorspace conversions

They're quite different!
I admit I've only just noticed the difference between "unpackColorSpace" and "browser's default colorspace conversion", is the latter a totally different thing, and both conversions could happen? (In what order?)

Putting everything in a table:

 UNPACK_COLORSPACE_ |                        |
\  CONVERSION_WEBGL | BROWSER_DEFAULT_WEBGL  |          NONE
Source              |                        |
-----------------------------------------------------------------------
HTMLImageElement    | unpackColorSpace (Or?) | no unpackColorSpace
                    | browser's default conv | no colorspace conversion
-----------------------------------------------------------------------
Others              | unpackColorSpace       | unpackColorSpace  Or(?)
                    | other conversion?      | no colorspace conversion

It would be great to get some clarity around UNPACK_COLORSPACE_CONVERSION_WEBGL and unpackColorSpace and if/how they interact. Please let me know if I've missed anything already in the specs.

I'm also wondering, with language such as "The precise conversions may be specific to both the browser and file type", how there can be conformance tests that actually test affected conversions. (More about that below.)

Some more context, if that helps: I'm trying to implement unpackColorSpace in WebKit.
First, I've noticed that the test videos like red-green.mp4 are in smpte170m, which I believe is some old colorspace from NTSC times!
Now all tests using these resources expect pure colors like red (255,0,0) and green (0,255,0). However when doing the proper conversion to the default unpackColorSpace "srgb", these colors should get converted to around (248,36,0) and (63,251,0).
I think at least some tests do set UNPACK_COLORSPACE_CONVERSION_WEBGL to NONE, but as explained above, my initial reading of the specs meant that it shouldn't matter when importing a video.

Interestingly, when trying to decypher browser source code to see what they're doing, it looks like Firefox is only respecting UNPACK_COLORSPACE_CONVERSION_WEBGL==NONE for images (correct as per my reading of the specs), while Chrome applies it to images but also to videos, and Safari applies it to everything! (But I may have read wrong, of course.)

Trying a small local test just displaying red-green.mp4 as a video, along with doing a drawImage into a 2d canvas, and texImage2D into a WebGL canvas, when I sample the red colors as displayed on an sRGB monitor on macOS:

  • Current Safari: video (248,37,1), 2d (248,36,1), 3d (255,2,1)
  • Safari with unpackColorSpace WIP: video (248,36,0), 2d (248,36,1), 3d (248,36,1)
  • Chrome: video (249,37,0), 2d (255,0,0), 3d (255,0,0)
  • Firefox: video (248,36,0) [edit: (255,0,0)], 2d (255,0,0), 3d (255,0,0)

It seems that Chrome and Firefox agree in not doing any conversions in canvases.
[edit: Chrome is not doing any conversions in canvases. Firefox is not even converting the initial video.]
Safari does the conversion in 2d but not 3d, and I am (was?) working on adding the conversion in 3d.
Who is correct? (Between browsers and specs.)

kdashg commented

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.

if imageSource instanceof HTMLImageElement and UNPACK_COLORSPACE_CONVERSION_WEBGL == NONE:
  no conversion
else:
  convert imageSource's data from imageSource's colorspace to unpackColorSpace

(This could be useful for things like normal maps, or other not-really-color "images")

However, for:

If set to BROWSER_DEFAULT_WEBGL, then the browser's default colorspace conversion is applied during subsequent texImage2D and texSubImage2D calls taking HTMLImageElement. The precise conversions may be specific to both the browser and file type. If set to NONE, no colorspace conversion is applied. The initial value is BROWSER_DEFAULT_WEBGL.

This is slightly inconsistent, yeah, sorry!
This would be better I think:

If set to BROWSER_DEFAULT_WEBGL, then the browser's default colorspace conversion is applied during subsequent texImage2D and texSubImage2D calls taking HTMLImageElement. The precise conversions may be specific to both the browser and file type. For such calls, if instead set to NONE, no colorspace conversion is applied. The initial value is BROWSER_DEFAULT_WEBGL.

squelart commented

Thank you Kelsey for your reply.

I think we agree for the first paragraph, under texImage2D. 😁

For the second one:

This would be better I think:

If set to BROWSER_DEFAULT_WEBGL, then the browser's default colorspace conversion is applied during subsequent texImage2D and texSubImage2D calls taking HTMLImageElement. The precise conversions may be specific to both the browser and file type. For such calls, if instead set to NONE, no colorspace conversion is applied. The initial value is BROWSER_DEFAULT_WEBGL.

That seems closer to the first paragraph in spirit, though it's now restricted to HTMLImageElements, so it changes the tests and moves the "else: ??" elsewhere:

if imageSource instanceof HTMLImageElement:
  if UNPACK_COLORSPACE_CONVERSION_WEBGL == BROWSER_DEFAULT_WEBGL:
    browser's default colorspace conversion
  else:
    no colorspace conversion
else: // other imageSources
  ??

(If that's not your intent, maybe you could write the pseudo-code first, and then derive the English wording from it.)

Regardless of this detail, my confusion still remains around the relationship between these two paragraphs: Are they talking about the same conversions (clearly it's the unpackColorSpace conversion in the 1st one, but I'm not sure about the 2nd one), and trying to specify the same thing with different words?

And what to do with tests that I think should fail per specs, because they use a resource in strange colorspaces, like red-green.mp4 in smpte170m which should not map to sRGB pure red&green?

ccameron-chromium commented

Personally I'd like to remove the HTMLImageElement carve-out, and have it just be:

if UNPACK_COLORSPACE_CONVERSION_WEBGL == NONE:
  no conversion
else:
  convert imageSource's data from imageSource's colorspace to unpackColorSpace

This would not change the behavior for any non-Chromium browsers. For Chromium, we've already made this change for video (this patch), so the only work would be for Chromium to fix that for all remaining input types.

We should also add conformance tests for the "don't do color management" behavior. We have tests for the "do color management" behavior (I wrote them in a way that, if the unpackColorSpace is absent, they skip themselves, which was probably a mistake).

--

Pontification and motivation below:

My suspicion is that this behavior is from some time in the distant past when we only thought about color management for images. Canvases were perforce sRGB, videos were almost all Rec709-which-most-of-us-treat-as-sRGB, and "convert" generally meant "convert to sRGB", so it was a moot point for all content except for HTMLImageElement.

This "treat images differently" behavior is also present for createImageBitmap (see here). I similarly find this frustrating when importing content into WebGPU demos.

Personally I find it very useful to be able to opt-out of any color conversion for videos (e.g, for this tone mapping demo), but I suspect that many (most?) applications would just prefer that the pixels arrive in a predictable format, which is what I'd like for UNPACK_COLORSPACE_CONVERSION_WEBGL == DEFAULT in combination with unpackColorSpace to do.

kdashg commented

Let's not get into whether to change this here. We should discuss that elsewhere. I'm not necessarily against it! But it will need a larger discussion than this clarification of language here.

kdashg commented

Ok actually I think I remembered why it's like this!

Long ago, I believe that we thought that:

  • Canvases, ImageData, and ImageBitmaps will already be in the same colorspace as eachother, so UNPACK_COLORSPACE_CONVERSION_WEBGL should be a no-op.
  • Videos "always need conversion" because they're generally coming in as yuv data of some kind, and we didn't want UNPACK_COLORSPACE_CONVERSION_WEBGL to mean really-no-conversion and just give YUV data, so we didn't want UNPACK_COLORSPACE_CONVERSION_WEBGL to apply for videos.

This leaves UNPACK_COLORSPACE_CONVERSION_WEBGL as only functional for ImageElements, hence the way the spec is worded.

I see that in tests we do often use UNPACK_COLORSPACE_CONVERSION_WEBGL: NONE even for e.g. videos.
I suspect that browsers generally don't implement this, but I'll make sure I add tests for it.

As for the spec text

From my previous comment, this is the intent of the spec as I understand it (for both paragraphs):

if imageSource instanceof HTMLImageElement and UNPACK_COLORSPACE_CONVERSION_WEBGL == NONE:
  no conversion
else:
  convert imageSource's data from imageSource's colorspace to unpackColorSpace

Your pseudocode is also accurate!:

if imageSource instanceof HTMLImageElement:
  if UNPACK_COLORSPACE_CONVERSION_WEBGL == BROWSER_DEFAULT_WEBGL:
    browser's default colorspace conversion
  else:
    no colorspace conversion
else: // other imageSources
  ??

This is correct, because UNPACK_COLORSPACE_CONVERSION_WEBGL does not apply to non-ImageElement sources.
The ?? therefore does not mean unspecified, but rather that the spec of UNPACK_COLORSPACE_CONVERSION_WEBGL has no effect on non-ImageElement sources.

So maybe:

Only applies to texImage and texSubImage calls taking HTMLImageElement. If set to BROWSER_DEFAULT_WEBGL, then the browser's default colorspace conversion is applied. The precise conversions may be specific to both the browser and file type. If instead set to NONE, no colorspace conversion is applied. The initial value is BROWSER_DEFAULT_WEBGL.

kdashg commented

If there's no disagreement, I think we're all satisfied here!