Bug 161014

Summary: Mark CanvasPath operations' parameters as mandatory when they should be
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, darin, dbates, dino, rniwa, sam
Priority: P2 Keywords: WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://html.spec.whatwg.org/multipage/scripting.html#canvaspath
Attachments:
Description Flags
Patch
none
Patch none

Chris Dumez
Reported 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.
Attachments
Patch (48.79 KB, patch)
2016-08-19 14:32 PDT, Chris Dumez
no flags
Patch (49.06 KB, patch)
2016-08-19 14:39 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2016-08-19 14:32:51 PDT
Chris Dumez
Comment 2 2016-08-19 14:39:05 PDT
Darin Adler
Comment 3 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?
Chris Dumez
Comment 4 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.
WebKit Commit Bot
Comment 5 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>
WebKit Commit Bot
Comment 6 2016-08-19 17:47:16 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.