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

Add CanvasColorType, with unorm8 (the default) and float16 support #10951

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ccameron-chromium
Copy link
Contributor

@ccameron-chromium ccameron-chromium commented Jan 27, 2025

The default behavior is unorm8, which matches the existing behavior of
all user agents.

The name unorm8 is chosen to match modern APIs which distinguish between
8-bit unsigned normalized and 8-bit unsigned integer (e.g,
VK_FORMAT_R8G8B8A8_UNORM vs VK_FORMAT_R8G8B8A8_UINT in Vulkan,
MTLPixelFormatRGBA8Unorm vs MTLPixelFormatRGBA8Uint in Metal, and
"rgba8unorm" vs "rgba8uint" in WebGPU).

The use of colorType (instead of pixelFormat) is used to avoid the
complexity of selecting channel ordering (BGRA vs RGBA) and interactions
with the alpha parameter (RGB, RGBX, or BGRX potentially being required if
alpha is false).


/canvas.html ( diff )

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can in theory be isolated from the ImageData changes, but is that useful or even observable (apart from this API itself)?

source Show resolved Hide resolved
@ccameron-chromium
Copy link
Contributor Author

I guess this can in theory be isolated from the ImageData changes, but is that useful or even observable (apart from this API itself)?

This actually hits lots of testable paths before adding ImageData support. Canvases that are float16 should be able to round-trip data from a different color space without a loss of precision (including round-tripping through PNG encoding).

This patch is the proto-WPT test that I'm working against. E.g, if you write an sRGB image to a float16-P3 canvas, then read it back as sRGB, you shouldn't lose any code points. But in our implementation you do... BUT ONLY if you read back as 8-bit (if you read back as float16, then the bug is side-stepped). The fact that there's so much to be gotten right in this area alone is part of why I chopped it into a separate commit.

@annevk annevk added addition/proposal New features or enhancements topic: canvas labels Jan 27, 2025
@annevk
Copy link
Member

annevk commented Jan 27, 2025

cc @whatwg/canvas

@ccameron-chromium
Copy link
Contributor Author

Are there any further issues with the the PR itself?

If this is all in line, then I'll file implementation and MDN bugs.

@Kaiido
Copy link
Member

Kaiido commented Feb 7, 2025

Nit: The end section about alpha-premult might need an update. I suppose the following sentence doesn't entirely hold anymore:

[...] and division and multiplication on 8-bit integers (which is how canvas's colors are currently stored) [...]

@ccameron-chromium
Copy link
Contributor Author

Nit: The end section about alpha-premult might need an update. I suppose the following sentence doesn't entirely hold anymore:

[...] and division and multiplication on 8-bit integers (which is how canvas's colors are currently stored) entails a loss of precision [...]

Thanks for finding that! I've change it to read "division and multiplication using finite precision entails a loss of accuracy". This also fixes a nit where what is lost is accuracy (not precision).

The default behavior is unorm8, which matches the existing behavior of
all user agents.

The name unorm8 is chosen to match modern APIs which distinguish between
8-bit unsigned normalized and 8-bit unsigned integer (e.g,
VK_FORMAT_R8G8B8A8_UNORM vs VK_FORMAT_R8G8B8A8_UINT in Vulkan,
MTLPixelFormatRGBA8Unorm vs MTLPixelFormatRGBA8Uint in Metal, and
"rgba8unorm" vs "rgba8uint" in WebGPU).

The use of colorType (instead of pixelFormat) is used to avoid the
complexity of selecting channel ordering (BGRA vs RGBA) and interactions
with the alpha parameter (RGB, RGBX, or BGRX potentially being required if
alpha is false).
Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done editorial review and this all looks to be in good order. I understand there's some additional bits to come, but this chunk looks testable and more importantly usable+useful.


<p class="example">For example, when serializing a 2D context that has
<span data-x="concept-canvas-color-type">color type</span> of
<span data-x="dom-CanvasColorType-float16">float16</span> to <var>type</var>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to other reviewers. The use of the variable type here outside of an algorithm looks wrong at first, but all of the paragraphs in this section are specifying requirements on "a serialization of the bitmap as a file" and there are a few other uses of type. It seems OK to leave this style alone I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: canvas
Development

Successfully merging this pull request may close these issues.

4 participants