Bug 172896 - Implement DOMPointReadOnly.matrixTransform()
Summary: Implement DOMPointReadOnly.matrixTransform()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar, WebExposed
Depends on:
Blocks: 163505
  Show dependency treegraph
 
Reported: 2017-06-03 08:15 PDT by Simon Fraser (smfr)
Modified: 2017-06-04 08:49 PDT (History)
5 users (show)

See Also:


Attachments
Patch (12.89 KB, patch)
2017-06-03 12:42 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (20.93 KB, patch)
2017-06-03 12:46 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (20.90 KB, patch)
2017-06-03 12:55 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch for landing (19.78 KB, patch)
2017-06-03 13:31 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 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.
Comment 1 Chris Dumez 2017-06-03 12:42:15 PDT
Created attachment 311938 [details]
Patch
Comment 2 Simon Fraser (smfr) 2017-06-03 12:43:23 PDT
I just wrote this same patch. But mine is better :P. Coming soon.
Comment 3 Simon Fraser (smfr) 2017-06-03 12:46:10 PDT
Created attachment 311939 [details]
Patch
Comment 4 Simon Fraser (smfr) 2017-06-03 12:55:10 PDT
Created attachment 311940 [details]
Patch
Comment 5 Radar WebKit Bug Importer 2017-06-03 12:55:44 PDT
<rdar://problem/32554549>
Comment 6 Chris Dumez 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.
Comment 7 Simon Fraser (smfr) 2017-06-03 13:31:25 PDT
Created attachment 311941 [details]
Patch for landing
Comment 8 WebKit Commit Bot 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>
Comment 9 WebKit Commit Bot 2017-06-03 14:09:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Sam Weinig 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.
Comment 11 Chris Dumez 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