Bug 67242 – Don't round-trip through TransformationMatrix in SVGImageBufferTools::clearAffineTransform2DRotation

• Component: SVG
• Status: RESOLVED FIXED

[Tim Horton]

https://bugs.webkit.org/show_bug.cgi?id=53055 will introduce SVGImageBufferTools::clearAffineTransform2DRotation, which currently round-trips through TransformationMatrix to decompose and clear the 2D rotation. In that bug, Dirk raised concerns about this, performance-wise, so it would be good to fix.

[Tim Horton]

After all that fuss, we already have the code, we just don't expose it. Patch coming shortly.

[Radar WebKit Bug Importer]

<rdar://problem/10069770>

[Tim Horton]

Created attachment 106214 [details]
initial patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/graphics/transforms/AffineTransform.h:192:  The parameter name "decomp" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/transforms/AffineTransform.h:193:  The parameter name "decomp" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 4 files


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

[Tim Horton]

Created attachment 106215 [details]
style fix

[WebKit Review Bot]

Comment on attachment 106215 [details]
style fix

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

New failing tests:
svg/custom/stroked-pattern.svg
svg/custom/non-scaling-stroke.svg
svg/transforms/text-with-pattern-inside-transformed-html.xhtml
svg/custom/pattern-no-pixelation.svg
svg/custom/nested-pattern-boundingBoxModeContent.svg
svg/custom/js-late-pattern-and-object-creation.svg
svg/custom/js-late-pattern-creation.svg

[Dirk Schulze]

Comment on attachment 106215 [details]
style fix

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

Missmatch on chromium:  svg/custom/svg-fonts-word-spacing.html = IMAGE+TEXT
Is that related to your patch? Please check this before landing.

> Source/WebCore/platform/graphics/transforms/AffineTransform.cpp:310
> +            srA.angle -= piDouble * 2.0;

If you are already touching the code... s/2.0/2/ for every appearance.

> Source/WebCore/platform/graphics/transforms/AffineTransform.cpp:344
> +    if (m.a() * m.d() - m.c() * m.b() < 0.0) {

s/0.0/0/

> Source/WebCore/rendering/svg/SVGImageBufferTools.cpp:130
> +    decomposition.angle = 0.0;

s/0.0/0/

[Tim Horton]

Something is going quite wrong; look at svg/custom/pattern-no-pixelation.svg for example. I'll take a look, but this definitely can't land as-is!

[Tim Horton]

(In reply to comment #8)
> Something is going quite wrong; look at svg/custom/pattern-no-pixelation.svg for example. I'll take a look, but this definitely can't land as-is!

Ah, I reordered the recompose completely incorrectly.

[Tim Horton]

Created attachment 106496 [details]
new patch, fixing test failures

[Tim Horton]

Created attachment 106504 [details]
patch

[Dirk Schulze]

Comment on attachment 106504 [details]
patch

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

r=me with one snippet.

> Source/WebCore/rendering/svg/SVGImageBufferTools.cpp:130
> +    decomposition.angle = 0.0;

s/0.0/0/ :)

[Tim Horton]

Landed in r94679