Bug 172896

Summary: Implement DOMPointReadOnly.matrixTransform()
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: DOMAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, sam, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar, WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 163505    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

Simon Fraser (smfr)
Reported 2017-06-03 08:15:25 PDT
DOMPointReadonly.idl has: // FIXME: No support for DOMMatrix yet (webkit.org/b/110001) // DOMPoint matrixTransform(optional DOMMatrixInit matrix); but we have DOMMatrix now.
Attachments
Patch (12.89 KB, patch)
2017-06-03 12:42 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (20.93 KB, patch)
2017-06-03 12:46 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (20.90 KB, patch)
2017-06-03 12:55 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch for landing (19.78 KB, patch)
2017-06-03 13:31 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2017-06-03 12:42:15 PDT
Created attachment 311938 [details] Patch
Simon Fraser (smfr)
Comment 2 2017-06-03 12:43:23 PDT
I just wrote this same patch. But mine is better :P. Coming soon.
Simon Fraser (smfr)
Comment 3 2017-06-03 12:46:10 PDT
Created attachment 311939 [details] Patch
Simon Fraser (smfr)
Comment 4 2017-06-03 12:55:10 PDT
Created attachment 311940 [details] Patch
Radar WebKit Bug Importer
Comment 5 2017-06-03 12:55:44 PDT
<rdar://problem/32554549>
Chris Dumez
Comment 6 2017-06-03 12:59:31 PDT
Comment on attachment 311940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311940&action=review r=me > LayoutTests/imported/w3c/ChangeLog:14 > + * web-platform-tests/css/geometry-1/DOMPoint-002.html: I don't think we should be modifying downstream tests unless the same change has landed upstream. Also, there is no behavior change here so this downstream change is not strictly needed. > LayoutTests/imported/w3c/ChangeLog:16 > + * web-platform-tests/css/geometry-1/DOMPoint-003.html: Added. Ditto, I don't think we should modify this folder unless we are sync'ing with upstream. We should consider landing this upstream first, and/or put this new test in LayoutTests/http/wpt/geometry/ in the mean time.
Simon Fraser (smfr)
Comment 7 2017-06-03 13:31:25 PDT
Created attachment 311941 [details] Patch for landing
WebKit Commit Bot
Comment 8 2017-06-03 14:09:04 PDT
Comment on attachment 311941 [details] Patch for landing Clearing flags on attachment: 311941 Committed r217763: <http://trac.webkit.org/changeset/217763>
WebKit Commit Bot
Comment 9 2017-06-03 14:09:06 PDT
All reviewed patches have been landed. Closing bug.
Sam Weinig
Comment 10 2017-06-04 08:34:02 PDT
Comment on attachment 311941 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=311941&action=review > Source/WebCore/dom/DOMPointReadOnly.cpp:52 > + double x = this->x(); > + double y = this->y(); > + double z = this->z(); > + double w = this->w(); > + matrix->transformationMatrix().map4ComponentPoint(x, y, z, w); > + > + return { DOMPoint::create(x, y, z, w) }; Is this supposed to return *this if it is an identity transformation? I noticed the IDL is not annotated with NewObject.
Chris Dumez
Comment 11 2017-06-04 08:49:20 PDT
Comment on attachment 311941 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=311941&action=review >> Source/WebCore/dom/DOMPointReadOnly.cpp:52 >> + return { DOMPoint::create(x, y, z, w) }; > > Is this supposed to return *this if it is an identity transformation? I noticed the IDL is not annotated with NewObject. They probably forgot [NewObject] in the spec. The spec is clear we're supposed to return a new DOMPoint, always: https://drafts.fxtf.org/geometry/#dom-dompointreadonly-matrixtransform
Note You need to log in before you can comment on or make changes to this bug.