Bug 110295 – Transformed SVG background images can be blurry/pixelated

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Philip Rogers]

Created attachment 189227 [details]
Testcase

When an SVG image is used as a background image and there are CSS transforms, the result can be blurry/pixelated. This is because our buffer used for tiling does not account for CSS transforms.

See the attached testcase for an example.

[Philip Rogers]

*** Bug 110272 has been marked as a duplicate of this bug. ***

[Philip Rogers]

Created attachment 189246 [details]
Account for transform in SVG background images

[Dirk Schulze]

Comment on attachment 189246 [details]
Account for transform in SVG background images

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

LGTM. Just some snippets.

> Source/WebCore/loader/cache/CachedImage.cpp:198
> +        UNUSED_PARAM(renderer);

indentation wrong.

> Source/WebCore/svg/graphics/SVGImage.cpp:153
> +    ASSERT(imageBufferScale.width() && imageBufferScale.height());

Can you make this two asserts please?

> Source/WebCore/svg/graphics/SVGImage.cpp:159
> +    drawForContainer(buffer->context(), containerSize, zoom, imageBufferSize, zoomedContainerRect, ColorSpaceDeviceRGB, CompositeSourceOver, BlendModeNormal);

Aren't the last two or three options default for this function anyway? Not sure if we should keep it or remove it.

> LayoutTests/svg/as-background-image/svg-transformed-background.html:17
> +  -ie-transform:scale(5);
> +  -ie-transform-origin:0 0;
> +  -moz-transform:scale(5);
> +  -moz-transform-origin:0 0;

This is unnecessary. please remove these.

> LayoutTests/svg/as-background-image/svg-transformed-background.html:33
> +  -ie-transform:scale(2.5, 10);
> +  -ie-transform-origin:0 0;
> +  -moz-transform:scale(2.5, 10);
> +  -moz-transform-origin:0 0;

ditto.

[Philip Rogers]

Created attachment 189414 [details]
Update per reviewer comments

[Philip Rogers]

Thank you for the review as always Dirk!

(In reply to comment #3)
> (From update of attachment 189246 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=189246&action=review
> 
> LGTM. Just some snippets.
> 
> > Source/WebCore/loader/cache/CachedImage.cpp:198
> > +        UNUSED_PARAM(renderer);
> 
> indentation wrong.

Fixed.

> 
> > Source/WebCore/svg/graphics/SVGImage.cpp:153
> > +    ASSERT(imageBufferScale.width() && imageBufferScale.height());
> 
> Can you make this two asserts please?

Fixed.

> 
> > Source/WebCore/svg/graphics/SVGImage.cpp:159
> > +    drawForContainer(buffer->context(), containerSize, zoom, imageBufferSize, zoomedContainerRect, ColorSpaceDeviceRGB, CompositeSourceOver, BlendModeNormal);
> 
> Aren't the last two or three options default for this function anyway? Not sure if we should keep it or remove it.

They aren't default for this one yet. I'd like to hold-off on making these default at the moment, but lets definitely address this in a followup.

> 
> > LayoutTests/svg/as-background-image/svg-transformed-background.html:17
> > +  -ie-transform:scale(5);
> > +  -ie-transform-origin:0 0;
> > +  -moz-transform:scale(5);
> > +  -moz-transform-origin:0 0;
> 
> This is unnecessary. please remove these.

Done.

> 
> > LayoutTests/svg/as-background-image/svg-transformed-background.html:33
> > +  -ie-transform:scale(2.5, 10);
> > +  -ie-transform-origin:0 0;
> > +  -moz-transform:scale(2.5, 10);
> > +  -moz-transform-origin:0 0;
> 
> ditto.

Done.

[WebKit Review Bot]

Comment on attachment 189414 [details]
Update per reviewer comments

Clearing flags on attachment: 189414

Committed r143541: <http://trac.webkit.org/changeset/143541>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.