Re: Canvas ImageData comments

On Wed, 30 Apr 2008, Philip Taylor wrote:
> 
> From the "Pixel manipulation" section:
> 
> "readonly attribute int[] data;" - should that look like a JS Array 
> object? I've seen one person write "imagedata.data.join(', ')" (for 
> debugging), relying on the Array methods being there, so it might be 
> good (for functionality and for interoperability) if that worked.

It won't work, but join() is generic and I believe should work on a 
CanvasPixelArray, so just do the prototype magic -- something like:

   CanvasPixelArray.prototype.join = Array.prototype.join;

...or something.


> "If the first argment to the method is null or not an ImageData object 
> that was returned by createImageData() or getImageData()" - how can you 
> get an ImageData object that wasn't returned by one of those methods? If 
> it's possible, things should be changed to make that impossible (since 
> such an object is unusable). If it's impossible, it's pointless to list 
> those methods, and makes it harder for anyone to add a new way to return 
> ImageData (e.g. a getImageData on a 3D context) since they'll have to 
> update that putImageData definition each time.

Fair enough.


> "If any of the arguments to createImageData() or getImageData() are infinite
> or NaN, or if either the sw or sh arguments are zero, the method must instead
> raise an INDEX_SIZE_ERR exception.", and "If any of the arguments to the
> method are infinite or NaN, the method must raise an INDEX_SIZE_ERR
> exception." - shouldn't infinite/NaN be handled with NOT_SUPPORTED_ERR, for
> consistency with every other method? If so, that case shouldn't need
> mentioning here, since it's implicitly handled by "Common conformance
> requirements for APIs exposed to JavaScript".

The common requirements are overruled by a requirement for all of the 
canvas section that causes infinite and NaN values to be ignored. The 
cases where they raise INDEX_SIZE_ERR are the cases where they can't be 
ignored.

For addColorStop(), NOT_SUPPORTED_ERR seems wrong for offset=1.1, and 
having a different exception for offset=1.1 and offset=Inf seems wrong.

Fixed the rest to throw NOT_SUPPORTED_ERR.


> "If the number is not an integer, it must be rounded to the nearest 
> integer using the IEEE 754r roundTiesToEven rounding mode." - perhaps 
> that should be convertToIntegerTiesToEven to be slightly more precise, 
> since roundTiesToEven is a generic (not necessarily integer) algorithm.

Done.


> "for all values of x and y where dirtyX ≤ x < dirtyX+dirtyWidth and dirtyY
> ≤ y < dirtyY+dirtyHeight" - should say "for all integer values of x and y
> ..."

Fixed.


> Step 5 ("If, after those changes, either dirtyWidth or dirtyHeight is negative
> or zero, stop these steps without affecting the canvas.") is redundant and
> useless, since step 6 will have no effect anyway when either of those
> conditions holds.

The step is in there to satisfy people who otherwise would spend hours 
trying to work out how step 6 does anything in those cases.


> "the pixel with coordinate (x_{device}+x, y_{device}+y)" - they were called
> "dx_{device}" etc earlier - should be consistent.

Fixed.


> "Thus, while one could create an ImageData object, one would not 
> necessarily know what resolution the canvas expected" - that doesn't 
> make sense to me. It would make sense as "Thus, while one might wish to 
> create an ImageData object by specifying the <code>width</code> and 
> <code>height</code> attributes directly, one would not ..."

Removed the entire sentence.


> "In the following example, the script first obtains the size of the 
> canvas backing store, and then generates a few new ImageData objects 
> which can be used." - no it doesn't.

Fixed.


> "The putImageData(imagedata, dx, dy, dirtyX, dirtyY, dirtyWidth, 
> dirtyHeight) method writes data from ImageData structures back to the 
> canvas." - should say "The putImageData(imagedata, dx, dy) and 
> putImageData(imagedata, dx, dy, ...) methods write ...", else it'll 
> confuse people into thinking the dirty arguments are required.

Done.


> (Also, it'd be nice if there was link from here back up to the IDL so 
> people can check the declarations.)

Yeah, well, that's a problem throughout the spec. Maybe gsnedders will 
give us that feature at some point. :-)


> "the following must result in no visible changes to the rendering: 
> context.putImageData(context.getImageData(x, y, w, h), x, y); ...for any 
> value of x and y." - should say "for any values of x, y, w and h."

Done.


> "The values of the data array may be changed" - abuse of RFC2119 
> keyword. ('MAY' is defined in terms of implementations optionally 
> supporting a feature, and having to interoperate with implementations 
> that include and exclude that feature, so it makes no sense here.)

This got fixed a while back.


> Typos:
>  "rectanglewhose"
>  "argment"
>  "the heightmember of the imagedata structure"
>  "thr pixels"
>  "crateImageData"

Most of these seemed to have been fixed already. Fixed those I could find.

I like the idea of crating ImageData...

-- 
Ian Hickson               U+1047E                )\._.,--....,'``.    fL
http://ln.hixie.ch/       U+263A                /,   _.. \   _\  ;`._ ,.
Things that are impossible just take longer.   `._.-(,_..'--(,_..'`-.;.'