Editorial: putImageData: Clarify that the operation is not a memcpy #11042

ccameron-chromium · 2025-02-18T10:50:01Z

Ensure that it be clear that putImage will convert from the source ImageData's color space and pixel format to the output bitmap's color space and pixel format.

Also rename dirtyX,dirtyY,dirtyWidth,dirtyHeight to sx,sy,sw,sh. The original names reflect the use case where content is being rendered into an ImageData by JavaScript and that one wishes to copy the "dirty" part of the source ImageData to the destination canvas. This can be confusing because this process will "dirty" the destination canvas. Change this to use the same source and destination parameter names that are used by drawImage and related functions.

/canvas.html ( diff )

annevk · 2025-02-18T12:40:19Z

source

@@ -65537,7 +65537,7 @@ interface mixin <dfn interface>CanvasImageData</dfn> {
  <span>ImageData</span> <span data-x="dom-context-2d-createImageData-imagedata">createImageData</span>(<span>ImageData</span> imagedata);
  <span>ImageData</span> <span data-x="dom-context-2d-getImageData">getImageData</span>([EnforceRange] long sx, [EnforceRange] long sy, [EnforceRange] long sw, [EnforceRange] long sh, optional <span>ImageDataSettings</span> settings = {});
  undefined <span data-x="dom-context-2d-putImageData">putImageData</span>(<span>ImageData</span> imagedata, [EnforceRange] long dx, [EnforceRange] long dy);
-  undefined <span data-x="dom-context-2d-putImageData">putImageData</span>(<span>ImageData</span> imagedata, [EnforceRange] long dx, [EnforceRange] long dy, [EnforceRange] long dirtyX, [EnforceRange] long dirtyY, [EnforceRange] long dirtyWidth, [EnforceRange] long dirtyHeight);
+  undefined <span data-x="dom-context-2d-putImageData">putImageData</span>(<span>ImageData</span> imagedata, [EnforceRange] long dx, [EnforceRange] long dy, [EnforceRange] long sx, [EnforceRange] long sy, [EnforceRange] long sw, [EnforceRange] long sh);


We should probably do this separately, but I think it would be good to use non-abbreviated parameter names here.

Yeah, removed it. And I agree it should stay as the current names. It's an extremely bizarre API.

I was not suggesting the current names necessarily, but destinationX and sourceX would be better than dx and sx. Maybe we can fix imagedata -> imageData now?

Actually, it's such a strange behavior (I didn't realize the details of it until editing the spec here!) that I don't want to mess with the "dirty" variable names in this patch.

But imagedata to imageData is safe. I updated that throughout the full spec.

And I agree it should stay as the current names. It's an extremely bizarre API.

Yes, from the top comment I was going to say "beware! these are not the same!" and I agree dirtyX et al. are super weird. Every time I face them, I have to check again some old answer of mine onstackoverflow to remember what these represent exactly. I don't think we can touch it now, but "dirty" seems quite à-propos.

But all of sx, sy, sw, and sh in getImageData, and, dx and dy in putImageData can indeed be changed to sourceX, sourceY, sourceWidth, sourceHeight, destinationX, and destinationY. But maybe that's worth its own PR since drawImage would also need to be updated.

annevk · 2025-02-18T12:42:59Z

source

  <p>The <dfn method for="CanvasImageData"><code
-  data-x="dom-context-2d-putImageData">putImageData()</code></dfn> method writes data from
-  <code>ImageData</code> structures back to the rendering context's <span>output bitmap</span>. Its
-  arguments are: <var>imagedata</var>, <var>dx</var>, <var>dy</var>, <var>dirtyX</var>,
-  <var>dirtyY</var>, <var>dirtyWidth</var>, and <var>dirtyHeight</var>.</p>
+  data-x="dom-context-2d-putImageData">putImageData()</code></dfn> method converts and copies data
+  from <code>ImageData</code> structures to the rendering context's <span>output bitmap</span>. Its
+  arguments are: <var>imagedata</var>, <var>dx</var>, <var>dy</var>, <var>sx</var>,
+  <var>sy</var>, <var>sw</var>, and <var>sh</var>.</p>

-  <p>When the last four arguments to this method are omitted, they must be assumed to have the
-  values 0, 0, the <code data-x="dom-imagedata-width">width</code> member of the <var>imagedata</var> structure, and the <code data-x="dom-imagedata-height">height</code>
-  member of the <var>imagedata</var> structure, respectively.</p>
+  <p>Then last four arguments to this method indicate the source rectangle for the copy. When they
+  are omitted, they must be assumed to have the values 0, 0, the
+  <code data-x="dom-imagedata-width">width</code> member of the <var>imagedata</var> structure, and
+  the <code data-x="dom-imagedata-height">height</code> member of the <var>imagedata</var>
+  structure, respectively.</p>


Can we modernize this to

The putImageData(imageData, dx, dy, ...) method steps are:

and since there's an overload I guess we need that intro twice and then they both call into a shared algorithm. That would also remove the need for this weird "if they are omitted" language.

Changed it to a common algorithm (modernized) with both methods documented separately and calling the common algorithm.

annevk · 2025-02-19T09:38:11Z

source

@@ -65537,7 +65537,7 @@ interface mixin <dfn interface>CanvasImageData</dfn> {
  <span>ImageData</span> <span data-x="dom-context-2d-createImageData-imagedata">createImageData</span>(<span>ImageData</span> imagedata);
  <span>ImageData</span> <span data-x="dom-context-2d-getImageData">getImageData</span>([EnforceRange] long sx, [EnforceRange] long sy, [EnforceRange] long sw, [EnforceRange] long sh, optional <span>ImageDataSettings</span> settings = {});
  undefined <span data-x="dom-context-2d-putImageData">putImageData</span>(<span>ImageData</span> imagedata, [EnforceRange] long dx, [EnforceRange] long dy);
-  undefined <span data-x="dom-context-2d-putImageData">putImageData</span>(<span>ImageData</span> imagedata, [EnforceRange] long dx, [EnforceRange] long dy, [EnforceRange] long dirtyX, [EnforceRange] long dirtyY, [EnforceRange] long dirtyWidth, [EnforceRange] long dirtyHeight);
+  undefined <span data-x="dom-context-2d-putImageData">putImageData</span>(<span>ImageData</span> imagedata, [EnforceRange] long dx, [EnforceRange] long dy, [EnforceRange] long sx, [EnforceRange] long sy, [EnforceRange] long sw, [EnforceRange] long sh);


I was not suggesting the current names necessarily, but destinationX and sourceX would be better than dx and sx. Maybe we can fix imagedata -> imageData now?

annevk · 2025-02-19T09:40:15Z

source

-  arguments are: <var>imagedata</var>, <var>dx</var>, <var>dy</var>, <var>dirtyX</var>,
-  <var>dirtyY</var>, <var>dirtyWidth</var>, and <var>dirtyHeight</var>.</p>
+  data-x="dom-context-2d-putImageData-short">putImageData(<var>imagedata</var>,
+  <var>dx</var>, <var>dy</var>)</code></dfn> method will


The format is:

The X(...) method steps are to ...

you also want to supply the arguments in the same order and not rely on the reader to do some argument name mapping. That also means you don't set dirtyWidth to something but instead you'd pass imageData's width. (Though ideally not the public API for that, but maybe we only have public API at this point. Another thing to fix in due course.)

Changed this to do the "steps are to [algorithm name], given [params]" format.

Ensure that it be clear that putImage will convert from the source ImageData's color space and pixel format to the output bitmap's color space and pixel format. Also document the two separate putImageData methods as separate methods with a common algorithm, rather than a single method with optional parameters.

annevk

Thanks! Very close now. @Kaiido want to do a pass as well?

annevk · 2025-02-19T13:00:54Z

source

+  <var>dx</var>, <var>dy</var>)</code></dfn> method steps are to
+  <span data-x="dom-context2d-putImageData-common">put pixels from an
+  <code>ImageData</code> onto a bitmap</span>, given
+  <var>imageData</var>, the <span>output bitmap</span> of the rendering context,


We can make the second argument <span>this</span>'s <span>output bitmap</span>, right?

source

Kaiido

LGTM, modulo a small nit.
Though I'd have expected to see something about alpha-multiplication in the algo, but that's already an improvement. Thanks.

Kaiido · 2025-02-20T01:10:46Z

source

-   in the rendering context's <span>output bitmap</span>.</p></li>
+   data-x=""><var>dirtyY</var>+<var>dirtyHeight</var></span></span>,
+   set the pixel with coordinate (<span data-x=""><var>dx</var>+<var>x</var></span>,
+   <var>dy</var>+<var>y</var>) in <var>bitmap</var> to the color at coordinate


https://html.spec.whatwg.org/multipage/canvas.html#canvas-pixel-arraybuffer doesn't talk at all about "color" but really represents values in an ArrayBuffer, so maybe "to the color of the pixel at coordinate"?.
Also, this is not new, but maybe the alpha: false special case should be handled here explicitly? Though since here we're not in the correct CanvasSettings interface, that might be complex...

Updated it to read "to the color of the pixel at coordinate".

I can follow up with a patch that indicates the alpha behavior (and for that patch I'll need to include new WPT tests, since it doesn't look like that behavior is tested yet).

annevk · 2025-02-20T15:36:04Z

Thanks @ccameron-chromium!

annevk reviewed Feb 18, 2025

View reviewed changes

ccameron-chromium force-pushed the put_image_data branch 3 times, most recently from 3086f9a to ec10c44 Compare February 19, 2025 09:22

annevk reviewed Feb 19, 2025

View reviewed changes

ccameron-chromium added 2 commits February 19, 2025 11:44

Change imagedata to imageData

15275a5

ccameron-chromium force-pushed the put_image_data branch from ec10c44 to 15275a5 Compare February 19, 2025 11:50

annevk reviewed Feb 19, 2025

View reviewed changes

Incorporate review feedback

ab38752

Kaiido reviewed Feb 20, 2025

View reviewed changes

Incorporate review feedback

b476e94

annevk approved these changes Feb 20, 2025

View reviewed changes

annevk merged commit 6a3787f into whatwg:main Feb 20, 2025
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editorial: putImageData: Clarify that the operation is not a memcpy #11042

Editorial: putImageData: Clarify that the operation is not a memcpy #11042

ccameron-chromium commented Feb 18, 2025 •

edited by pr-preview bot

Loading

annevk Feb 18, 2025

ccameron-chromium Feb 19, 2025

annevk Feb 19, 2025

ccameron-chromium Feb 19, 2025

Kaiido Feb 20, 2025

annevk Feb 18, 2025

ccameron-chromium Feb 19, 2025

annevk Feb 19, 2025

annevk Feb 19, 2025

ccameron-chromium Feb 19, 2025

annevk left a comment

annevk Feb 19, 2025

ccameron-chromium Feb 19, 2025

Kaiido left a comment

Kaiido Feb 20, 2025

ccameron-chromium Feb 20, 2025

annevk commented Feb 20, 2025

Editorial: putImageData: Clarify that the operation is not a memcpy #11042

Editorial: putImageData: Clarify that the operation is not a memcpy #11042

Conversation

ccameron-chromium commented Feb 18, 2025 • edited by pr-preview bot Loading

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

annevk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kaiido left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Feb 20, 2025

ccameron-chromium commented Feb 18, 2025 •

edited by pr-preview bot

Loading