Re: The canvas element (detailed review)

On Sat, 4 Aug 2007, Philip Taylor wrote:
> 
> fill:
> 
> "The fill() method must fill each subpath of the current path in turn" - 
> that sounds incorrect - in current implementations it only paints one 
> shape, which is made from all the subpaths at once. (If two overlapping 
> but otherwise independent subpaths have opposite windings, they cancel 
> out and result in no fill. If they have the same winding, that area just 
> gets painted once.)

Is the new text enough? Or do I need to be more explicit?


> "the non-zero winding number rule" - that isn't defined, and it's not 
> obvious what it means, so I think it should be defined here. 
> http://www.cs.rit.edu/~icss571/filling/alt_parity.html and 
> http://www.w3.org/TR/SVGMobile12/painting.html#FillRuleProperty sound 
> like reasonable explanations.

Isn't it obvious what it means to anyone who is going to be implementing 
it? I'd love to add a reference, but if I add one I'd like it to be The 
Original Reference, and I haven't been able to track down who originally 
formulated this rule.


> stroke:
> 
> "The stroke() method must stroke each subpath of the current path in 
> turn" - that sounds incorrect too - it only paints one shape, which is 
> the union of all the subpaths' stroke shapes.

I've clarified it a bit, but not very well. Let me know if you have any 
suggestions for better text.


> clip:
> 
> "The clip() method must create a new clipping path [...]" - doesn't say 
> what should be done with the new clipping path that has been created.

Fixed.


> "[...] a new clipping path [...]" - doesn't use the term "clip region", 
> which is used elsewhere - the terminology should be consistent. "Clip 
> region" sounds a better term since the interesting thing is the region 
> that is clipped to, not the path that defines that region; and there 
> isn't even an easy-to-compute path once you've started intersecting 
> various regions.

Fixed.


> "When the context is created, the initial clipping path is the rectangle 
> with the top left corner at (0,0) and the width and height of the 
> coordinate space." - I don't think that's precise about the cases when 
> the canvas's size is changed and the context is not being created but is 
> "cleared back to [its] initial state and reinitialised with the newly 
> specified coordinate space dimensions". It would be clearer to say "When 
> the context is initialised, the clipping path is [...]", so it's obvious 
> that that's relevant to reinitialisation too.
> 
> "When the context is created, the initial clipping path is the rectangle 
> [...]" - should use "must".

Fixed.


> isPointInPath:
> 
> Is a point on the edge of a path considered inside or out or undefined? 
> (When doing a rect() path, FF3 says all points on the edge except the 
> top-left corner are in. Opera 9.5 says all points on the bottom edge or 
> right edge or bottom-right corner are in.)

Added "Points on the path itself are considered to be within the path".


> Should the clip region be taken into account? FF3 and O9.5 ignore it, 
> which seems logical since this is "is point in path" and not "is point 
> in the area that would actually get rendered if you filled (but don't 
> count shadows and alpha)". It could say "The isPointInPath(x, y) method 
> must return true if the point [...] is inside the current path (after 
> applying the current transformation), using the non-zero winding number 
> rule; [...]."

Ok.


> The Transformations section says "The transformation matrix is applied 
> to all drawing operations prior to their being rendered. It is also 
> applied when creating the clip region" - should add "and when invoking 
> the isPointInPath() method."

n/a.


> "canvas' coordinate space" - that is written "canvas coordinate space" 
> elsewhere (which is better since I wouldn't be tempted to complain about 
> "canvas'" vs "canvas's").

Fixed, though there's still another canvas', if you want to complain in 
vain. :-)


> drawImage:
> 
> "The image argument must be an instance of an HTMLImageElement or 
> HTMLCanvasElement. If the image is of the wrong type, the implementation 
> must raise a TYPE_MISMATCH_ERR exception." - what if it is the correct 
> type but null? (The same applies to createPattern). (Currently, FF 
> throws NS_ERROR_NOT_AVAILABLE, Opera throws TYPE_MISMATCH_ERR, Safari 
> crashes.)

Added "or null".


> What should happen if sw = 0 or sh = 0? (Firefox draws nothing, Opera 
> (9.2) draws something (top left pixel?), Safari 3 throws 
> INDEX_SIZE_ERR.)

INDEX_SIZE_ERR.


> What should happen if the image is animated? (Firefox and Safari draw 
> the current frame. Opera draws nothing). Similar issue with 
> createPattern. (In that case, Firefox 2 and Safari always draws the same 
> frame as was first drawn; Firefox 3 randomly (each time you reload the 
> page) either does the same as FF2 or else uses the current animation 
> frame each time it draws something with the pattern; Opera returns 
> undefined from createPattern.)

Fixed. (First frame or poster frame if that's not the same.)


> It might be nice to say "Implementations should use bilinear filtering 
> when resizing the image", so that implementations agree with each other 
> except when they have reasons not to (e.g. resource constraints which 
> make bilinear filtering too expensive). (At least, I think current 
> implementations are doing bilinear filtering - is that correct?)

I see no reason to constrain implementations here. It doesn't matter to 
interop, right? The only difference is quality. They could do something 
far better than bilinear filtering for all we care.


> It's not explicit that the transformation applies only to the 
> destination rectangle, and not the source rectangle.

Clarified, sort-of.


> "If not specified, the dw and dh arguments default to [...]. If the sx, 
> sy, sw, and sh arguments are omitted, they default to [...]" - it would 
> be more convenient to test those cases if they said "must".

Fixed.


> "If one of the sy, sw, sw, and sh arguments is outside the size of the 
> image [...]" - should be "sx, sy, sw, and sh", though it may be better 
> to say "If the source rectangle defined by the sx, sy, sw, and sh 
> arguments is outside the image [...]".

Yeah, I ended up changing it to something like what you propose 
independently.


> "the specified region of the image specified by the source rectangle" -> 
> "the region of the image specified by the source rectangle"

Fixed.

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

Received on Friday, 1 February 2008 01:58:52 UTC