Intent to Remove: ImageEncodeOptions colorSpace and pixelFormat

58 views
Skip to first unread message

Christopher Cameron

unread,
Nov 15, 2022, 11:21:54 AM11/15/22
to blink-dev, Jeremy Roman, Justin Novosad
Contact email:

Explainer: None
Specification: None

Summary:
These fields were accidentally exposed to the web in crrev.com/562990 by way of crrev.com/427153. They are not part of any specification.

These fields allow OffscreenCanvas::convertToBlob to specify a target color space (srgb, display-p3, and rec2020) and pixel format (uint8 and uint16). This sets a default behavior of setting the color space sRGB and pixel format of 8 bits per pixel.

When these parameters are removed, the target space for convertToBlob will default to the color space of the canvas (srgb or display-p3), and the pixel format will default to the pixel format of the canvas (uint8 is the only supported pixel format).

Motivation:
The behavior caused by these fields at odds with the HTML specification, section "Serializing bitmaps to a file", which says that

For image types that support color profiles, the serialized image must include a color profile indicating the color space of the underlying bitmap.

For example, for a Display P3 canvas, calling convertToBlob with no arguments should result in a blob that specifies a Display P3 color profile.

These fields can only degrade the quality of the resulting image, compared with the behavior from the HTML specification.

TAG review: N/A

Risks:
When this feature is removed, the images resulting from convertToBlob are guaranteed to be an equal or higher fidelity representation as compared with behavior before this feature was removed, because all OffscreenCanvas elements are 8-bit sRGB or Display P3.

Interoperability and Compatibility
Gecko: Never implemented
WebKit: Never implemented
Web developers: No signals

Debuggability: N/A

Is this feature fully tested by web-platform-tests?
No, there exists one Blink layout test for this feature.

Tracking bug: crbug.com/1384538

Link to related intents: 
This was discovered when writing tests for Canvas Floating Point Color Types, which is a prerequisite for High Dynamic Range Support for HTMLCanvasElement.


Mike Taylor

unread,
Nov 15, 2022, 1:23:44 PM11/15/22
to Christopher Cameron, blink-dev, Jeremy Roman, Justin Novosad
On 11/15/22 11:21 AM, 'Christopher Cameron' via blink-dev wrote:
Contact email:

Explainer: None
Specification: None

Summary:
These fields were accidentally exposed to the web in crrev.com/562990 by way of crrev.com/427153. They are not part of any specification.
These CLs landed in 2018 and 2016... do we have any UseCounter or usage data?

These fields allow OffscreenCanvas::convertToBlob to specify a target color space (srgb, display-p3, and rec2020) and pixel format (uint8 and uint16). This sets a default behavior of setting the color space sRGB and pixel format of 8 bits per pixel.

When these parameters are removed, the target space for convertToBlob will default to the color space of the canvas (srgb or display-p3), and the pixel format will default to the pixel format of the canvas (uint8 is the only supported pixel format).

Motivation:
The behavior caused by these fields at odds with the HTML specification, section "Serializing bitmaps to a file", which says that

For image types that support color profiles, the serialized image must include a color profile indicating the color space of the underlying bitmap.

For example, for a Display P3 canvas, calling convertToBlob with no arguments should result in a blob that specifies a Display P3 color profile.

These fields can only degrade the quality of the resulting image, compared with the behavior from the HTML specification.

TAG review: N/A

Risks:
When this feature is removed, the images resulting from convertToBlob are guaranteed to be an equal or higher fidelity representation as compared with behavior before this feature was removed, because all OffscreenCanvas elements are 8-bit sRGB or Display P3.

Interoperability and Compatibility
Gecko: Never implemented
WebKit: Never implemented
Web developers: No signals

Debuggability: N/A

Is this feature fully tested by web-platform-tests?
No, there exists one Blink layout test for this feature.

Tracking bug: crbug.com/1384538

Link to related intents: 
This was discovered when writing tests for Canvas Floating Point Color Types, which is a prerequisite for High Dynamic Range Support for HTMLCanvasElement.


--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGnfxj-oe0G%3DgaD8aMA%3DWNwG210m%2BJ%3DdM0XrpDxqa-02pNE3AQ%40mail.gmail.com.


Christopher Cameron

unread,
Nov 15, 2022, 3:18:56 PM11/15/22
to Mike Taylor, blink-dev, Jeremy Roman, Justin Novosad
On Tue, Nov 15, 2022 at 7:23 PM Mike Taylor <mike...@chromium.org> wrote:
On 11/15/22 11:21 AM, 'Christopher Cameron' via blink-dev wrote:
Contact email:

Explainer: None
Specification: None

Summary:
These fields were accidentally exposed to the web in crrev.com/562990 by way of crrev.com/427153. They are not part of any specification.
These CLs landed in 2018 and 2016... do we have any UseCounter or usage data?

We don't have usage counters for these members of ImageEncodeOptions.

We do have stats for the relevant APIs though. The proposal (removing the parameters) will bring OffscreenCanvasConvertToBlob in line with the related CanvasToBlob and CanvasToDataURL, both of which are far more popular.
Of note is that:
  • There is no documentation of colorSpace and pixelFormat -- the only way to know they exist is to read our IDL files
  • The only effect that those ImageEncodeOptions can have is to degrade the quality of the resulting image

Rick Byers

unread,
Nov 16, 2022, 11:30:19 AM11/16/22
to Christopher Cameron, Mike Taylor, blink-dev, Jeremy Roman, Justin Novosad
On Tue, Nov 15, 2022 at 3:18 PM 'Christopher Cameron' via blink-dev <blin...@chromium.org> wrote:
On Tue, Nov 15, 2022 at 7:23 PM Mike Taylor <mike...@chromium.org> wrote:
On 11/15/22 11:21 AM, 'Christopher Cameron' via blink-dev wrote:
Contact email:

Explainer: None
Specification: None

Summary:
These fields were accidentally exposed to the web in crrev.com/562990 by way of crrev.com/427153. They are not part of any specification.
These CLs landed in 2018 and 2016... do we have any UseCounter or usage data?

We don't have usage counters for these members of ImageEncodeOptions.

We do have stats for the relevant APIs though. The proposal (removing the parameters) will bring OffscreenCanvasConvertToBlob in line with the related CanvasToBlob and CanvasToDataURL, both of which are far more popular.
That's helpful as an upper bar to impact. 
Of note is that:
  • There is no documentation of colorSpace and pixelFormat -- the only way to know they exist is to read our IDL files
That and lack of support in any other engine certainly suggests this is likely to be just a bug fix that nobody notices. But it's always scary to do this blind - the web is weird.
  • The only effect that those ImageEncodeOptions can have is to degrade the quality of the resulting image
If that's really true, then I'd be supportive of removing it without further data. But I don't understand why it wouldn't be possible for eg. some image blob processing code to be written assuming uint8 pixels and then break horribly in scenarios where it starts getting uint16 pixels. Am I missing something?

If we're not 100% confident in the safety of removal then I'd suggest landing a simple UseCounter now, and removing in the next milestone. You could probably even get the UseCounter merged into M109 if you want, then land removal for M110 now. But I'm open to an argument that this is definitely unnecessary extra work.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

Mike Taylor

unread,
Nov 16, 2022, 2:57:24 PM11/16/22
to Rick Byers, Christopher Cameron, blink-dev, Jeremy Roman, Justin Novosad
On Wed, Nov 16, 2022, 7:30 PM Rick Byers <rby...@chromium.org> wrote:
On Tue, Nov 15, 2022 at 3:18 PM 'Christopher Cameron' via blink-dev <blin...@chromium.org> wrote:
On Tue, Nov 15, 2022 at 7:23 PM Mike Taylor <mike...@chromium.org> wrote:
On 11/15/22 11:21 AM, 'Christopher Cameron' via blink-dev wrote:
Contact email:

Explainer: None
Specification: None

Summary:
These fields were accidentally exposed to the web in crrev.com/562990 by way of crrev.com/427153. They are not part of any specification.
These CLs landed in 2018 and 2016... do we have any UseCounter or usage data?

We don't have usage counters for these members of ImageEncodeOptions.

We do have stats for the relevant APIs though. The proposal (removing the parameters) will bring OffscreenCanvasConvertToBlob in line with the related CanvasToBlob and CanvasToDataURL, both of which are far more popular.
That's helpful as an upper bar to impact. 
Of note is that:
  • There is no documentation of colorSpace and pixelFormat -- the only way to know they exist is to read our IDL files
I see - thanks.

That and lack of support in any other engine certainly suggests this is likely to be just a bug fix that nobody notices. But it's always scary to do this blind - the web is weird.
  • The only effect that those ImageEncodeOptions can have is to degrade the quality of the resulting image
If that's really true, then I'd be supportive of removing it without further data. But I don't understand why it wouldn't be possible for eg. some image blob processing code to be written assuming uint8 pixels and then break horribly in scenarios where it starts getting uint16 pixels. Am I missing something?

I also have this question, for pixelFormat.

As a workaround to specify the color-space (except for display-p3?), do I understand correctly that that sites could do something like like the following, e.g., 

f = new OffscreenCanvas(10,10);
f.getContext("2d", {colorSpace: "srgb"}) //or "display-p3", but not rec2020?
f.convertToBlob().then(b => doSomething(b))

Give your previous answer, the side effect for this is the image might look different (worse? degraded?), I think.

If that's true, it seems safer to remove colorSpace than pixelFormat. If there is some risk to removing pixelFormat, a UseCounter seems prudent (or at least some HTTPArchive analysis to see how people are processing blobs where it's used).

Christopher Cameron

unread,
Nov 17, 2022, 5:12:13 AM11/17/22
to Mike Taylor, Rick Byers, blink-dev, Jeremy Roman, Justin Novosad
On Wed, Nov 16, 2022 at 8:57 PM Mike Taylor <mike...@chromium.org> wrote:
If we're not 100% confident in the safety of removal then I'd suggest landing a simple UseCounter now, and removing in the next milestone. You could probably even get the UseCounter merged into M109 if you want, then land removal for M110 now. But I'm open to an argument that this is definitely unnecessary extra work.

I was about to write up a case-by-case discussion, but I wanted to make sure that I was accurately representing things. So I started writing tests for this, and found that it's actually all dead code. Removing these parameters from the public API is a no-op.

The only function that takes ImageEncodeOptions is OffscreenCanvas::convertToBlob. But, we only pay attention to ImageEncodeOptions if the calling function identifies itself as kHTMLCanvasConvertToBlobPromise (that is, not that one function). When we trace this back, we see that offscreen canvases do identify themselves correctly.

And, in fact, we have at least one WPT test that verifies that we ignore these parameters.

Yoav Weiss

unread,
Nov 17, 2022, 6:42:44 AM11/17/22
to Christopher Cameron, Mike Taylor, Rick Byers, blink-dev, Jeremy Roman, Justin Novosad
If this is indeed dead code, then this change is not web exposed, and hence no LGTMs are needed

Can you outline how this test is testing that we're ignoring these parameters? I'm not sure I get it.. Also, Safari seems to be failing that test..
May be nice to add dedicated tests for both of these parameters to ensure they are ignored before deleting them.


--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

Christopher Cameron

unread,
Nov 17, 2022, 7:19:05 AM11/17/22
to Yoav Weiss, Mike Taylor, Rick Byers, blink-dev, Jeremy Roman, Justin Novosad
On Thu, Nov 17, 2022 at 12:42 PM Yoav Weiss <yoav...@chromium.org> wrote:
If this is indeed dead code, then this change is not web exposed, and hence no LGTMs are needed

SGTM.
 
Can you outline how this test is testing that we're ignoring these parameters? I'm not sure I get it.. Also, Safari seems to be failing that test..
May be nice to add dedicated tests for both of these parameters to ensure they are ignored before deleting them.

The parameter has a default color space of sRGB. If we were to respect that, then, in the canvas-to-blob-to-image-to-canvas round-trip, all colors would be clamped to sRGB. To look a bit more carefully, this test only uses within-sRGB colors, so no clamping would be detected. I'll update it when removing the parameters.

Rick Byers

unread,
Nov 17, 2022, 10:03:09 AM11/17/22
to Christopher Cameron, Yoav Weiss, Mike Taylor, blink-dev, Jeremy Roman, Justin Novosad
Hah, awesome! These "we accidentally web exposed this for years" threads always scare me a little, glad this turned out not to be one after all!

Rick
Reply all
Reply to author
Forward
0 new messages