Bug 225263 – Update operations in CanvasImageData.idl to use long rather than float as per spec

• Component: New Bugs
• Status: RESOLVED FIXED

[Sam Weinig]

Update operations in CanvasImageData.idl to use long rather than float as per spec

[Sam Weinig]

Created attachment 427480 [details]
Patch

[Sam Weinig]

Created attachment 427498 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 427498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427498&action=review

I'm a bit nervous about compat fallout from this one.

> Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:2172
> +    IntSize size { std::abs(sw), std::abs(sh) };

Is this what the spec says; that negative values become positive? Are there tests for it?

> Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:2196
> +    IntRect imageDataRect { sx, sy, sw, sh };

Presumably the getImageData() call will fail if this rect ends up being outside the backing store rect? What if it's half outside?

[Sam Weinig]

(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 427498 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=427498&action=review
> 
> I'm a bit nervous about compat fallout from this one.

Anything you would like me to do to make you feel less nervous? All the other engines, according to WPT and my testing have this new behavior.

> 
> > Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:2172
> > +    IntSize size { std::abs(sw), std::abs(sh) };
> 
> Is this what the spec says; that negative values become positive? Are there
> tests for it?

Yes (it's also not new in the change, I just added some braces). The spec says:

"When the createImageData() method is invoked with two numeric arguments sw and sh, it must create an ImageData object, with parameter pixelsPerRow set to the absolute magnitude of sw, and parameter rows set to the absolute magnitude of sh."

And yes, we have tests with negatives for these values. 

> 
> > Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:2196
> > +    IntRect imageDataRect { sx, sy, sw, sh };
> 
> Presumably the getImageData() call will fail if this rect ends up being
> outside the backing store rect? What if it's half outside?

This is also not new behavior but no, getImageData() returns transparent black pixels for any pixel outside the backing store. We also have testing of this.

[EWS]

Committed r276877 (237223@main): <https://commits.webkit.org/237223@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427498 [details].

[Radar WebKit Bug Importer]

<rdar://problem/77421906>

[Anne van Kesteren]

*** Bug 220496 has been marked as a duplicate of this bug. ***