Bug 172896 – Implement DOMPointReadOnly.matrixTransform()

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

[Simon Fraser (smfr)]

DOMPointReadonly.idl has:

    // FIXME: No support for DOMMatrix yet (webkit.org/b/110001)
    // DOMPoint matrixTransform(optional DOMMatrixInit matrix);

but we have DOMMatrix now.

[Chris Dumez]

Created attachment 311938 [details]
Patch

[Simon Fraser (smfr)]

I just wrote this same patch. But mine is better :P. Coming soon.

[Simon Fraser (smfr)]

Created attachment 311939 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 311940 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/32554549>

[Chris Dumez]

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)]

Created attachment 311941 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 311941 [details]
Patch for landing

Clearing flags on attachment: 311941

Committed r217763: <http://trac.webkit.org/changeset/217763>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Sam Weinig]

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 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