Bug 82800 – Support pattern transformations in canvas

• Component: Canvas
• Status: NEW

[Dean Jackson]

Canvas v5 supports transformations in patterns

 - ability to transform a pattern
     var identity = new SVGMatrix();
     context.fillStyle = context.createPattern(img, 'repeat');
     context.fillStyle.setTransform(identity.rotate(angle));
     context.fillRect(0, 0, canvas.width, canvas.height);

[Radar WebKit Bug Importer]

<rdar://problem/11159356>

[Dongseong Hwang]

Created attachment 149858 [details]
patch

[Dongseong Hwang]

Canvas v5 [1] supports transformations in patterns.

[1] http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-canvaspattern-settransform

[WebKit Review Bot]

Comment on attachment 149858 [details]
patch

Attachment 149858 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13098972

[Dongseong Hwang]

Oops, I'll survey how to solve and patch again.

Actually, I don't know why V8 does not compile but I'll survey.

I'll be very glad if someone give me hints.

[Dongseong Hwang]

Created attachment 149923 [details]
patch v.2

[Dongseong Hwang]

CanvasPattern::setTransform needs ENABLE(SVG) because it uses SVGMatrix.

[Dongseong Hwang]

Created attachment 149930 [details]
patch v.3

[Dongseong Hwang]

Comment on attachment 149930 [details]
patch v.3

View in context: https://bugs.webkit.org/attachment.cgi?id=149930&action=review

> Source/WebCore/html/canvas/CanvasPattern.h:32
> +#include "SVGPropertyTearOff.h"

It is solution that I found for building with V8.
However, it is ugly.

Is it Ok? or I need advice.

Sorry for lack of knowledge of binding.

[WebKit Review Bot]

Attachment 149930 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/WebCore/html/canvas/CanvasPattern.h:54:  The parameter name "transform" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dongseong Hwang]

Created attachment 150051 [details]
patch v.4

[Dongseong Hwang]

Comment on attachment 150051 [details]
patch v.4

View in context: https://bugs.webkit.org/attachment.cgi?id=150051&action=review

> Source/WebCore/svg/SVGMatrix.h:28
> +#include "SVGPropertyTearOff.h"

It is solution that I found for building with V8.

Is it Ok? or I need advice, again.

[Dongseong Hwang]

I'll very appreciate if Dean or Simon take a look at this bug.

[Simon Fraser (smfr)]

Comment on attachment 150051 [details]
patch v.4

View in context: https://bugs.webkit.org/attachment.cgi?id=150051&action=review

> LayoutTests/fast/canvas/script-tests/pattern-with-settransform.js:53
> +var svgElement = document.createElementNS("http://www.w3.org/2000/svg", "svg");
> +var matrix = svgElement.createSVGMatrix();

Does new SVGMatrix() not work?

> LayoutTests/fast/canvas/script-tests/pattern-with-settransform.js:55
> +matrix = matrix.translate(20, 20);
> +pattern.setTransform(matrix);

I think you need some more complex tests here. e.g. drawing a transformed pattern when the context is already transformed. Testing transforms like rotates and skews. Testing that clipping of such transforms works.

> Source/WebCore/html/canvas/CanvasPattern.cpp:70
> +    m_pattern->setPatternSpaceTransform(static_cast<const AffineTransform&>(transform));

Why is the cast necessary? SVGMatrix _is_ a AffineTransform.

> Source/WebCore/html/canvas/CanvasPattern.h:52
> +#if ENABLE(SVG)
> +        void setTransform(const SVGMatrix&);
> +#endif

It's unfortunate that this feature is tied to SVG, but I guess that's how the spec is.

>> Source/WebCore/svg/SVGMatrix.h:28
>> +#if USE(V8)
>> +// Only used from the bindings.
>> +#include "SVGPropertyTearOff.h"
> 
> It is solution that I found for building with V8.
> 
> Is it Ok? or I need advice, again.

Seems like this should be fixed on the V8 side, not here.

[Dongseong Hwang]

Thank you for reviewing.

(In reply to comment #14)
> (From update of attachment 150051 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150051&action=review
> 
> > LayoutTests/fast/canvas/script-tests/pattern-with-settransform.js:53
> > +var svgElement = document.createElementNS("http://www.w3.org/2000/svg", "svg");
> > +var matrix = svgElement.createSVGMatrix();
> 
> Does new SVGMatrix() not work?

Yes, 'new SVGMatrix()' works! sorry.
> 
> > LayoutTests/fast/canvas/script-tests/pattern-with-settransform.js:55
> > +matrix = matrix.translate(20, 20);
> > +pattern.setTransform(matrix);
> 
> I think you need some more complex tests here. e.g. drawing a transformed pattern when the context is already transformed. Testing transforms like rotates and skews. Testing that clipping of such transforms works.

Ok, I'll update.
> 
> > Source/WebCore/html/canvas/CanvasPattern.cpp:70
> > +    m_pattern->setPatternSpaceTransform(static_cast<const AffineTransform&>(transform));
> 
> Why is the cast necessary? SVGMatrix _is_ a AffineTransform.

It is not necessary. I'll fix it.
> 
> > Source/WebCore/html/canvas/CanvasPattern.h:52
> > +#if ENABLE(SVG)
> > +        void setTransform(const SVGMatrix&);
> > +#endif
> 
> It's unfortunate that this feature is tied to SVG, but I guess that's how the spec is.

Do You mean "#if ENABLE(SVG)" is proper and I don't need to change it?
> 
> >> Source/WebCore/svg/SVGMatrix.h:28
> >> +#if USE(V8)
> >> +// Only used from the bindings.
> >> +#include "SVGPropertyTearOff.h"
> > 
> > It is solution that I found for building with V8.
> > 
> > Is it Ok? or I need advice, again.
> 
> Seems like this should be fixed on the V8 side, not here.

I'll implement v8 binding in V8CanvasPatternCustom.cpp. Is it Ok?

However, I think the correct solution is to amend CodeGeneratorV8.pm, but I don't have enough knowledge.
If you think the later is correct, I'll guard '#if !USE(V8)' to CanvasPattern::setTransform and create new bug to enable CanvasPattern::setTransform on V8.

[kangax]

@Huang Dongsung Any progress on this?

[Dongseong Hwang]

(In reply to comment #16)
> @Huang Dongsung Any progress on this?

Sorry. I forgot this. I'll update soon.

[Ahmad Saleem]

Isn't it supported now?

https://searchfox.org/wubkat/rev/7051b8f5cd90af80f03a58d74741cdb04c800b6e/Source/WebCore/html/canvas/CanvasPattern.cpp#75