Bug 161014 – Mark CanvasPath operations' parameters as mandatory when they should be

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

Mark CanvasPath operations as mandatory when they should be:
- https://html.spec.whatwg.org/multipage/scripting.html#canvaspath

We should move the CanvasPath methods from CanvasRenderingContext2D and Path2D to a new CanvasPath interface, to avoid duplication and match the specification.
The parameters were correctly marked as mandatory in WebKit on CanvasRenderingContext2D but not on Path2D.

Those parameters are correctly marked as mandatory in Chrome and Firefox.

[Chris Dumez]

Created attachment 286481 [details]
Patch

[Chris Dumez]

Created attachment 286483 [details]
Patch

[Darin Adler]

Comment on attachment 286483 [details]
Patch

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

> Source/WebCore/html/canvas/CanvasPath.h:51
> +    void closePath();
> +    void moveTo(float x, float y);
> +    void lineTo(float x, float y);
> +    void quadraticCurveTo(float cpx, float cpy, float x, float y);
> +    void bezierCurveTo(float cp1x, float cp1y, float cp2x, float cp2y, float x, float y);
> +    void arcTo(float x0, float y0, float x1, float y1, float radius, ExceptionCode&);
> +    void arc(float x, float y, float r, float sa, float ea, bool anticlockwise, ExceptionCode&);
> +    void ellipse(float x, float y, float radiusX, float radiusY, float rotation, float startAngle, float endAngled, bool anticlockwise, ExceptionCode&);
> +    void rect(float x, float y, float width, float height);

Why is it OK that these all say float in the IDL file, but double here?

[Chris Dumez]

Comment on attachment 286483 [details]
Patch

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

>> Source/WebCore/html/canvas/CanvasPath.h:51
>> +    void rect(float x, float y, float width, float height);
> 
> Why is it OK that these all say float in the IDL file, but double here?

Currently, all our implementation uses float for Canvas. I updated the IDL to match the specification and use double but updating the implementation to use double would be a much larger effort.

If you look at our bindings code, you'll see that we anyway call value.toNumber() which returns a double. If we use float in the IDL, we merely cast the value returned to a float. There is therefore no change here AFAIK, we still call toNumber(), get a double and then implicitly convert it to a float when calling the implementation function instead of doing an explicit cast to float.

[WebKit Commit Bot]

Comment on attachment 286483 [details]
Patch

Clearing flags on attachment: 286483

Committed r204669: <http://trac.webkit.org/changeset/204669>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.