Update to reflect WhatWG changes #63

ccameron-chromium · 2021-06-15T22:05:56Z

In WhatWG review, a handful of changes were made to this proposal. Update to reflect these changes, to avoid confusion.

Note that WebGL/GPU parts are in separate review from Canvas2D changes
Update ImageData interface to use an attribute
Update getImageData and createImageData methods to use canvas's color space by default
Move ImageBitmap changes to be discussed under WebGL (similarly not part of the Canvas2D changes)

kenrussell · 2021-06-16T21:04:56Z

CanvasColorSpaceProposal.md

@@ -110,6 +103,8 @@ When an unsupported color space is requested, the color space will fall back to

 ### WebGL

+_NOTE: This section is not finalized and is being fleshed out and reviewed in Khronos_


Would you like to link to KhronosGroup/WebGL#3285 ?

kenrussell · 2021-06-16T21:06:11Z

CanvasColorSpaceProposal.md

@@ -125,8 +120,12 @@ In implementations in which a ``UNPACK_COLORSPACE_CONVERSION_WEBGL`` of ``BROWSE

 Note that, while convenient, this behavior is likely less efficient than specifying color conversion in ``ImageBitmapOptions``, where the color conversion may be done asynchronously and simultaneously with image decode.

+An mechanism for using ``ImageBitmapOptions`` to specify color space conversion will be added in the context of WebGL color space support, to support asynchronous and decode-time color space conversion for texture data upload.


An mechanism -> A mechanism

WebGL's spec is managed outside of the W3C, so it's infeasible for it to make changes to the ImageBitmapOptions dictionary. Those changes will need to be made in the WHATWG spec.

kenrussell · 2021-06-16T21:07:07Z

CanvasColorSpaceProposal.md

 ### WebGPU

+_NOTE: This section is not finalized and is being fleshed out and reviewed in Khronos_


Not in Khronos - in the W3C. May want to link to https://github.com/gpuweb/gpuweb . I'm not sure there's a pull request open for this yet. cc @kainino0x @Kangz

It's in progress. The section to link to is https://gpuweb.github.io/gpuweb/#color-spaces which links to the other relevant parts of the spec. There are some open github issues still, but everything is tracked inline in the spec.

For copying into WebGPU (done): https://gpuweb.github.io/gpuweb/#gpu-image-copy-texture-tagged
Importing (done except premultiplyAlpha): https://gpuweb.github.io/gpuweb/#external-texture-creation
Canvas output (not done, has issue): https://gpuweb.github.io/gpuweb/#presentation-configuration

kenrussell · 2021-06-16T21:09:52Z

CanvasColorSpaceProposal.md

@@ -151,26 +150,14 @@ Canvas contents are composited in accordance with the canvas element's style (e.
 The chromiumance of color values outside of [0, 1] is not to be clamped, and extended values may be used to display colors outside of the gamut defined by the canvas' color space's primaries.
 This is in contrast with luminance, which is to be clamped to the maximum standard dynamic range luminance, unless high dynamic range is explicitly enabled for the canvas element.

-### ImageBitmap


Per comment above, these changes can't be made in the WebGL or WebGPU specs; they need to be made in the WHATWG HTML spec. I think it's worth just leaving this here so that we know we have to make these changes in the WHATWG later.

kainino0x · 2021-06-16T22:14:31Z

CanvasColorSpaceProposal.md

 ### WebGPU

+_NOTE: This section is not finalized and is being fleshed out and reviewed in Khronos_


It's in progress. The section to link to is https://gpuweb.github.io/gpuweb/#color-spaces which links to the other relevant parts of the spec. There are some open github issues still, but everything is tracked inline in the spec.

For copying into WebGPU (done): https://gpuweb.github.io/gpuweb/#gpu-image-copy-texture-tagged
Importing (done except premultiplyAlpha): https://gpuweb.github.io/gpuweb/#external-texture-creation
Canvas output (not done, has issue): https://gpuweb.github.io/gpuweb/#presentation-configuration

kainino0x · 2021-06-16T22:20:11Z

CanvasColorSpaceProposal.md

@@ -180,15 +167,14 @@ IDL:
 <pre>
 partial interface ImageData {
  constructor(unsigned long sw, unsigned long sh, optional ImageDataSettings);


Missing an argument name.

Also, it can default to {} since that's effectively the default value. You could make the same change createImageData/getImageData too

Suggested change

constructor(unsigned long sw, unsigned long sh, optional ImageDataSettings);

constructor(unsigned long sw, unsigned long sh, optional ImageDataSettings settings = {});

kainino0x · 2021-06-16T22:20:18Z

CanvasColorSpaceProposal.md

-  constructor(ImageDataArray data, unsigned long sw, unsigned long sh, optional ImageDataSettings);
-  readonly ImageDataSettings getImageDataSettings();
-  readonly attribute ImageDataArray data;
+  constructor(Uint8ClampedArray data, unsigned long sw, unsigned long sh, optional ImageDataSettings);


Suggested change

constructor(Uint8ClampedArray data, unsigned long sw, unsigned long sh, optional ImageDataSettings);

constructor(Uint8ClampedArray data, unsigned long sw, unsigned long sh, optional ImageDataSettings settings = {});

junov · 2021-06-17T21:15:39Z

CanvasColorSpaceProposal.md

 ### ImageData

 Add the following types to be used by `ImageData`.

 IDL:
 <pre>
 dictionary ImageDataSettings {
-  PredefinedColorSpaceEnum colorSpace = "srgb";
+  PredefinedColorSpaceEnum colorSpace;


Remove "Enum"

junov · 2021-06-17T21:19:43Z

CanvasColorSpaceProposal.md

-* The constructors now take an optional `ImageDataSettings` dictionary.
-* The ImageDataSettings attribute may be queried using `getImageDataSettings`.
+The changes to this interface are that the constructors now take an optional `ImageDataSettings` dictionary.
+If this dictionary is specified and has a `colorSpace` entry, then the resulting `ImageData` will be created with the specified color space.


Suggested change

If this dictionary is specified and has a `colorSpace` entry, then the resulting `ImageData` will be created with the specified color space.

If this dictionary is specified and has a `colorSpace` member, then the resulting `ImageData` will store pixel data in the specified color space.

junov · 2021-06-17T21:20:57Z

CanvasColorSpaceProposal.md

-* The ImageDataSettings attribute may be queried using `getImageDataSettings`.
+The changes to this interface are that the constructors now take an optional `ImageDataSettings` dictionary.
+If this dictionary is specified and has a `colorSpace` entry, then the resulting `ImageData` will be created with the specified color space.
+If no dictionary is specified, or the specified dictionary does not specify a `colorSpace` entry, then the resulting `ImageData` will be created as ``"srgb"``.


Suggested change

If no dictionary is specified, or the specified dictionary does not specify a `colorSpace` entry, then the resulting `ImageData` will be created as ``"srgb"``.

If no dictionary is specified, or the specified dictionary does not specify a `colorSpace` member, then the resulting `ImageData` will store pixel data in the ``"srgb"`` color space.

junov · 2021-06-17T21:23:35Z

CanvasColorSpaceProposal.md

-* The ImageDataSettings attribute may be queried using `getImageDataSettings`.
+The changes to this interface are that the constructors now take an optional `ImageDataSettings` dictionary.
+If this dictionary is specified and has a `colorSpace` entry, then the resulting `ImageData` will be created with the specified color space.
+If no dictionary is specified, or the specified dictionary does not specify a `colorSpace` entry, then the resulting `ImageData` will be created as ``"srgb"``.

 When an ``ImageData`` is used in a canvas (e.g, in ``putImageData``), the data is converted from the ``ImageData``'s color space to the color space of the canvas.


This should be rephrased to indicate that the conversion can be in either direction (put vs. get)

junov · 2021-06-17T21:25:04Z

CanvasColorSpaceProposal.md

-The changes to this interface are the addion of the optional ``ImageDataSettings`` argument. If this argument is unspecified, then the default value of ``colorSpace="srgb"`` will be used (this default match previous behavior).
+The changes to this interface are the addition of the optional `ImageDataSettings` argument.
+If this dictionary is specified and has a `colorSpace` entry, then the resulting `ImageData` will be created with the specified color space.
+If no dictionary is specified, or the specified dictionary does not specify a `colorSpace` entry, then the resulting `ImageData` will default to using the same color space as the `CanvasRenderingContext2D` that the method is called on.


Suggested change

If no dictionary is specified, or the specified dictionary does not specify a `colorSpace` entry, then the resulting `ImageData` will default to using the same color space as the `CanvasRenderingContext2D` that the method is called on.

If no dictionary is specified, or the specified dictionary does not specify a `colorSpace` member, then the resulting `ImageData` will default to using the same color space as the `CanvasRenderingContext2D` that the method is called on.

Update to reflect WhatWG changes

abf1e7c

ccameron-chromium requested a review from junov June 15, 2021 22:06

kenrussell reviewed Jun 16, 2021

View reviewed changes

kainino0x approved these changes Jun 16, 2021

View reviewed changes

junov reviewed Jun 18, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to reflect WhatWG changes #63

Update to reflect WhatWG changes #63

ccameron-chromium commented Jun 15, 2021

kenrussell Jun 16, 2021

kenrussell Jun 16, 2021

kenrussell Jun 16, 2021

kainino0x Jun 16, 2021

kenrussell Jun 16, 2021

kainino0x Jun 16, 2021

kainino0x Jun 16, 2021

kainino0x Jun 16, 2021

junov Jun 17, 2021

junov Jun 17, 2021

junov Jun 17, 2021

junov Jun 17, 2021

junov Jun 17, 2021

		@@ -110,6 +103,8 @@ When an unsupported color space is requested, the color space will fall back to

		### WebGL

		_NOTE: This section is not finalized and is being fleshed out and reviewed in Khronos_

		@@ -125,8 +120,12 @@ In implementations in which a ``UNPACK_COLORSPACE_CONVERSION_WEBGL`` of ``BROWSE

		Note that, while convenient, this behavior is likely less efficient than specifying color conversion in ``ImageBitmapOptions``, where the color conversion may be done asynchronously and simultaneously with image decode.

		An mechanism for using ``ImageBitmapOptions`` to specify color space conversion will be added in the context of WebGL color space support, to support asynchronous and decode-time color space conversion for texture data upload.

		### WebGPU

		_NOTE: This section is not finalized and is being fleshed out and reviewed in Khronos_

	constructor(unsigned long sw, unsigned long sh, optional ImageDataSettings);
	constructor(unsigned long sw, unsigned long sh, optional ImageDataSettings settings = {});

	constructor(Uint8ClampedArray data, unsigned long sw, unsigned long sh, optional ImageDataSettings);
	constructor(Uint8ClampedArray data, unsigned long sw, unsigned long sh, optional ImageDataSettings settings = {});

	If this dictionary is specified and has a `colorSpace` entry, then the resulting `ImageData` will be created with the specified color space.
	If this dictionary is specified and has a `colorSpace` member, then the resulting `ImageData` will store pixel data in the specified color space.

	If no dictionary is specified, or the specified dictionary does not specify a `colorSpace` entry, then the resulting `ImageData` will be created as ``"srgb"``.
	If no dictionary is specified, or the specified dictionary does not specify a `colorSpace` member, then the resulting `ImageData` will store pixel data in the ``"srgb"`` color space.

Update to reflect WhatWG changes #63

Are you sure you want to change the base?

Update to reflect WhatWG changes #63

Conversation

ccameron-chromium commented Jun 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment