Bug 161014 - Mark CanvasPath operations' parameters as mandatory when they should be
Summary: Mark CanvasPath operations' parameters as mandatory when they should be
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://html.spec.whatwg.org/multipag...
Keywords: WebExposed
Depends on:
Blocks:
 
Reported: 2016-08-19 14:26 PDT by Chris Dumez
Modified: 2016-08-19 17:47 PDT (History)
7 users (show)

See Also:


Attachments
Patch (48.79 KB, patch)
2016-08-19 14:32 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (49.06 KB, patch)
2016-08-19 14:39 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-08-19 14:26:42 PDT
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.
Comment 1 Chris Dumez 2016-08-19 14:32:51 PDT
Created attachment 286481 [details]
Patch
Comment 2 Chris Dumez 2016-08-19 14:39:05 PDT
Created attachment 286483 [details]
Patch
Comment 3 Darin Adler 2016-08-19 15:34:12 PDT
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?
Comment 4 Chris Dumez 2016-08-19 15:44:01 PDT
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.
Comment 5 WebKit Commit Bot 2016-08-19 17:47:11 PDT
Comment on attachment 286483 [details]
Patch

Clearing flags on attachment: 286483

Committed r204669: <http://trac.webkit.org/changeset/204669>
Comment 6 WebKit Commit Bot 2016-08-19 17:47:16 PDT
All reviewed patches have been landed.  Closing bug.