Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Feb 20, 2025
Merged

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

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d0a8cd8
putImageData: Clarify that the operation is not a memcpy
ccameron-chromium Feb 18, 2025
15275a5
Change imagedata to imageData
ccameron-chromium Feb 19, 2025
ab38752
Incorporate review feedback
ccameron-chromium Feb 19, 2025
b476e94
Incorporate review feedback
ccameron-chromium Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 53 additions & 38 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65534,10 +65534,10 @@ interface mixin <dfn interface>CanvasDrawImage</dfn> {
interface mixin <dfn interface>CanvasImageData</dfn> {
// <span>pixel manipulation</span>
<span>ImageData</span> <span data-x="dom-context-2d-createImageData">createImageData</span>([EnforceRange] long sw, [EnforceRange] long sh, optional <span>ImageDataSettings</span> settings = {});
<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-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-short">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);
Copy link
Member

@annevk annevk Feb 18, 2025

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@ccameron-chromium ccameron-chromium Feb 19, 2025

Choose a reason for hiding this comment

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

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

Copy link
Member

@annevk annevk Feb 19, 2025

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@ccameron-chromium ccameron-chromium Feb 19, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Member

@Kaiido Kaiido Feb 20, 2025

Choose a reason for hiding this comment

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

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.

};

enum <dfn enum>CanvasLineCap</dfn> { "butt", "round", "square" };
Expand Down Expand Up @@ -69918,7 +69918,7 @@ try {
<h6><dfn>Pixel manipulation</dfn></h6>

<dl class="domintro">
<dt><code data-x=""><var>imagedata</var> = new <span subdfn data-x="dom-imagedata">ImageData</span>(<var>sw</var>, <var>sh</var> [, <var>settings</var>])</code></dt>
<dt><code data-x=""><var>imageData</var> = new <span subdfn data-x="dom-imagedata">ImageData</span>(<var>sw</var>, <var>sh</var> [, <var>settings</var>])</code></dt>
<dd>
<p>Returns an <code>ImageData</code> object with the given dimensions and the color space
indicated by <var>settings</var>. All the pixels in the returned object are <span>transparent
Expand All @@ -69928,7 +69928,7 @@ try {
the width or height arguments are zero.</p>
</dd>

<dt><code data-x=""><var>imagedata</var> = new <span subdfn data-x="dom-imagedata-with-data">ImageData</span>(<var>data</var>, <var>sw</var> [, <var>sh</var> [, <var>settings</var> ] ])</code></dt>
<dt><code data-x=""><var>imageData</var> = new <span subdfn data-x="dom-imagedata-with-data">ImageData</span>(<var>data</var>, <var>sw</var> [, <var>sh</var> [, <var>settings</var> ] ])</code></dt>
<dd>
<p>Returns an <code>ImageData</code> object using the data provided in the <code
data-x="idl-Uint8ClampedArray">Uint8ClampedArray</code> argument, interpreted using the given
Expand All @@ -69942,11 +69942,11 @@ try {
data and dimensions can't be interpreted consistently, or if either dimension is zero.</p>
</dd>

<dt><code data-x=""><var>imagedata</var> = <var>context</var>.<span subdfn data-x="dom-context-2d-createImageData-imagedata">createImageData</span>(<var>imagedata</var>)</code></dt>
<dt><code data-x=""><var>imageData</var> = <var>context</var>.<span subdfn data-x="dom-context-2d-createImageData-imagedata">createImageData</span>(<var>imageData</var>)</code></dt>
<dd><p>Returns an <code>ImageData</code> object with the same dimensions and color space as the
argument. All the pixels in the returned object are <span>transparent black</span>.</p></dd>

<dt><code data-x=""><var>imagedata</var> = <var>context</var>.<span subdfn data-x="dom-context-2d-createImageData">createImageData</span>(<var>sw</var>, <var>sh</var> [, <var>settings</var>])</code></dt>
<dt><code data-x=""><var>imageData</var> = <var>context</var>.<span subdfn data-x="dom-context-2d-createImageData">createImageData</span>(<var>sw</var>, <var>sh</var> [, <var>settings</var>])</code></dt>
<dd>
<p>Returns an <code>ImageData</code> object with the given dimensions. The color space of the
returned object is the <span data-x="concept-canvas-color-space">color space</span> of
Expand All @@ -69957,7 +69957,7 @@ try {
the width or height arguments are zero.</p>
</dd>

<dt><code data-x=""><var>imagedata</var> = <var>context</var>.<span subdfn data-x="dom-context-2d-getImageData">getImageData</span>(<var>sx</var>, <var>sy</var>, <var>sw</var>, <var>sh</var> [, <var>settings</var>])</code></dt>
<dt><code data-x=""><var>imageData</var> = <var>context</var>.<span subdfn data-x="dom-context-2d-getImageData">getImageData</span>(<var>sx</var>, <var>sy</var>, <var>sw</var>, <var>sh</var> [, <var>settings</var>])</code></dt>
<dd>
<p>Returns an <code>ImageData</code> object containing the image data for the given rectangle of
the bitmap. The color space of the returned object is the <span
Expand All @@ -69968,19 +69968,19 @@ try {
of the width or height arguments are zero.</p>
</dd>

<dt><code data-x=""><var>imagedata</var>.<span subdfn data-x="dom-imagedata-width">width</span></code></dt>
<dt><code data-x=""><var>imagedata</var>.<span subdfn data-x="dom-imagedata-height">height</span></code></dt>
<dt><code data-x=""><var>imageData</var>.<span subdfn data-x="dom-imagedata-width">width</span></code></dt>
<dt><code data-x=""><var>imageData</var>.<span subdfn data-x="dom-imagedata-height">height</span></code></dt>
<dd><p>Returns the actual dimensions of the data in the <code>ImageData</code> object, in
pixels.</p></dd>

<dt><code data-x=""><var>imagedata</var>.<span subdfn data-x="dom-imagedata-data">data</span></code></dt>
<dt><code data-x=""><var>imageData</var>.<span subdfn data-x="dom-imagedata-data">data</span></code></dt>
<dd><p>Returns the one-dimensional array containing the data in RGBA order, as integers in the
range 0 to 255.</p></dd>

<dt><code data-x=""><var>imagedata</var>.<span subdfn data-x="dom-imagedata-colorSpace">colorSpace</span></code></dt>
<dt><code data-x=""><var>imageData</var>.<span subdfn data-x="dom-imagedata-colorSpace">colorSpace</span></code></dt>
<dd><p>Returns the color space of the pixels.</p></dd>

<dt><code data-x=""><var>context</var>.<span subdfn data-x="dom-context-2d-putImageData">putImageData</span>(<var>imagedata</var>, <var>dx</var>, <var>dy</var> [, <var>dirtyX</var>, <var>dirtyY</var>, <var>dirtyWidth</var>, <var>dirtyHeight</var> ])</code></dt>
<dt><code data-x=""><var>context</var>.<span subdfn data-x="dom-context-2d-putImageData">putImageData</span>(<var>imageData</var>, <var>dx</var>, <var>dy</var> [, <var>dirtyX</var>, <var>dirtyY</var>, <var>dirtyWidth</var>, <var>dirtyHeight</var> ])</code></dt>
<dd>
<p>Paints the data from the given <code>ImageData</code> object onto the bitmap. If a dirty
rectangle is provided, only the pixels from that rectangle are painted.</p>
Expand All @@ -69992,7 +69992,7 @@ try {
blending, no shadows, etc.</p>

<p>Throws an <span>"<code>InvalidStateError</code>"</span> <code>DOMException</code> if the
<var>imagedata</var> object's <code data-x="dom-imagedata-data">data</code> attribute value's
<var>imageData</var> object's <code data-x="dom-imagedata-data">data</code> attribute value's
[[ViewedArrayBuffer]] internal slot is detached.</p>
</dd>
</dl>
Expand Down Expand Up @@ -70077,18 +70077,18 @@ try {
</ol>

<p>The <dfn method for="CanvasImageData"><code
data-x="dom-context-2d-createimagedata-imagedata">createImageData(<var>imagedata</var>)</code></dfn>
data-x="dom-context-2d-createimagedata-imagedata">createImageData(<var>imageData</var>)</code></dfn>
method steps are:</p>

<ol>
<li><p>Let <var>newImageData</var> be a <span>new</span> <code>ImageData</code> object.</p></li>

<li><p><span data-x="Initialize an ImageData object">Initialize</span> <var>newImageData</var>
given the value of <var>imagedata</var>'s <code data-x="dom-imagedata-width">width</code>
attribute, the value of <var>imagedata</var>'s <code data-x="dom-imagedata-height">height</code>
given the value of <var>imageData</var>'s <code data-x="dom-imagedata-width">width</code>
attribute, the value of <var>imageData</var>'s <code data-x="dom-imagedata-height">height</code>
attribute, and <i><span
data-x="initialize-imagedata-defaultcolorspace">defaultColorSpace</span></i> set to the value of
<var>imagedata</var>'s <code data-x="dom-imagedata-colorSpace">colorSpace</code> attribute.</p></li>
<var>imageData</var>'s <code data-x="dom-imagedata-colorSpace">colorSpace</code> attribute.</p></li>

<li><p>Initialize the image data of <var>newImageData</var> to <span>transparent
black</span>.</p></li>
Expand Down Expand Up @@ -70232,19 +70232,32 @@ try {
component.</p>

<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-short">putImageData(<var>imageData</var>,
<var>dx</var>, <var>dy</var>)</code></dfn> method steps are to
Copy link
Member

@annevk annevk Feb 19, 2025

Choose a reason for hiding this comment

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

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.)

Copy link
Contributor Author

@ccameron-chromium ccameron-chromium Feb 19, 2025

Choose a reason for hiding this comment

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

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

<span data-x="dom-context2d-putImageData-common">put pixels from an
<code>ImageData</code> onto a bitmap</span>, given
<var>imageData</var>, <span>this</span>'s <span>output bitmap</span>,
Copy link
Member

@annevk annevk Feb 19, 2025

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@ccameron-chromium ccameron-chromium Feb 19, 2025

Choose a reason for hiding this comment

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

Yes, done.

<var>dx</var>, <var>dy</var>, 0, 0,
<var>imageData</var>'s <code data-x="dom-imagedata-width">width</code>, and
<var>imageData</var>'s <code data-x="dom-imagedata-height">height</code>.</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>The method, when invoked, must act as follows:</p>

<ol>
<li><p>Let <var>buffer</var> be <var>imagedata</var>'s <code
<p>The <dfn method for="CanvasImageData"><code
data-x="dom-context-2d-putImageData">putImageData(<var>imageData</var>,
<var>dx</var>, <var>dy</var>, <var>dirtyX</var>, <var>dirtyY</var>,
<var>dirtyWidth</var>, <var>dirtyHeight</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>, <span>this</span>'s <span>output bitmap</span>,
<var>dx</var>, <var>dy</var>, <var>dirtyX</var>, <var>dirtyY</var>,
<var>dirtyWidth</var>, and <var>dirtyHeight</var>.</p>

<p>To <dfn data-x="dom-context2d-putImageData-common">put pixels from an <code>ImageData</code>
onto a bitmap</dfn>, given an <code>ImageData</code> <var>imageData</var>,
an <span>output bitmap</span> <var>bitmap</var>, and numbers <var>dx</var>, <var>dy</var>,
<var>dirtyX</var>, <var>dirtyY</var>, <var>dirtyWidth</var>, and <var>dirtyHeight</var>:</p>

<ol>
<li><p>Let <var>buffer</var> be <var>imageData</var>'s <code
data-x="dom-imagedata-data">data</code> attribute value's [[ViewedArrayBuffer]] internal
slot.</p></li>

Expand All @@ -70271,12 +70284,12 @@ try {

<li>
<p>If <var>dirtyX</var>+<var>dirtyWidth</var> is greater than the <code
data-x="dom-imagedata-width">width</code> attribute of the <var>imagedata</var> argument, then
data-x="dom-imagedata-width">width</code> attribute of the <var>imageData</var> argument, then
let <var>dirtyWidth</var> be the value of that <code data-x="dom-imagedata-width">width</code>
attribute, minus the value of <var>dirtyX</var>.</p>

<p>If <var>dirtyY</var>+<var>dirtyHeight</var> is greater than the <code
data-x="dom-imagedata-height">height</code> attribute of the <var>imagedata</var> argument, then
data-x="dom-imagedata-height">height</code> attribute of the <var>imageData</var> argument, then
let <var>dirtyHeight</var> be the value of that <code
data-x="dom-imagedata-height">height</code> attribute, minus the value of <var>dirtyY</var>.</p>
</li>
Expand All @@ -70290,12 +70303,14 @@ try {
data-x=""><var>dirtyX</var>&nbsp;&le;&nbsp;<var>x</var>&nbsp;&lt;&nbsp;<span
data-x=""><var>dirtyX</var>+<var>dirtyWidth</var></span></span> and <span
data-x=""><var>dirtyY</var>&nbsp;&le;&nbsp;<var>y</var>&nbsp;&lt;&nbsp;<span
data-x=""><var>dirtyY</var>+<var>dirtyHeight</var></span></span>, copy the
four channels of the pixel with coordinate (<var>x</var>, <var>y</var>) in
the <var>imagedata</var> data structure's <span>Canvas Pixel
<code data-x="idl-ArrayBuffer">ArrayBuffer</code></span> to the pixel with coordinate (<span
data-x=""><var>dx</var>+<var>x</var></span>, <var>dy</var>+<var>y</var>)
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 of the pixel at coordinate
Copy link
Member

@Kaiido Kaiido Feb 20, 2025

Choose a reason for hiding this comment

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

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...

Copy link
Contributor Author

@ccameron-chromium ccameron-chromium Feb 20, 2025

Choose a reason for hiding this comment

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

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).

(<var>x</var>, <var>y</var>) in the <var>imageData</var> data structure's
<span>Canvas Pixel <code data-x="idl-ArrayBuffer">ArrayBuffer</code></span>,
converted from <var>imageData</var>'s <code data-x="dom-imagedata-colorSpace">colorSpace</code>
to the <span data-x="concept-canvas-color-space">color space</span> of <var>bitmap</var>
using <span>'relative-colorimetric'</span> rendering intent.</p></li>
</ol>

<p class="note">Due to the lossy nature of converting between color spaces and converting to and
Expand Down