Bug 110295 - Transformed SVG background images can be blurry/pixelated
Summary: Transformed SVG background images can be blurry/pixelated
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philip Rogers
URL:
Keywords:
: 110272 (view as bug list)
Depends on: 110442
Blocks: 109609
  Show dependency treegraph
 
Reported: 2013-02-19 20:21 PST by Philip Rogers
Modified: 2013-02-21 02:49 PST (History)
7 users (show)

See Also:


Attachments
Testcase (654 bytes, text/html)
2013-02-19 20:21 PST, Philip Rogers
no flags Details
Account for transform in SVG background images (17.64 KB, patch)
2013-02-19 22:37 PST, Philip Rogers
krit: review+
Details | Formatted Diff | Diff
Update per reviewer comments (17.44 KB, patch)
2013-02-20 16:46 PST, Philip Rogers
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Rogers 2013-02-19 20:21:58 PST
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.
Comment 1 Philip Rogers 2013-02-19 20:28:13 PST
*** Bug 110272 has been marked as a duplicate of this bug. ***
Comment 2 Philip Rogers 2013-02-19 22:37:06 PST
Created attachment 189246 [details]
Account for transform in SVG background images
Comment 3 Dirk Schulze 2013-02-20 11:42:15 PST
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.
Comment 4 Philip Rogers 2013-02-20 16:46:17 PST
Created attachment 189414 [details]
Update per reviewer comments
Comment 5 Philip Rogers 2013-02-20 16:51:42 PST
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.
Comment 6 WebKit Review Bot 2013-02-20 17:21:09 PST
Comment on attachment 189414 [details]
Update per reviewer comments

Clearing flags on attachment: 189414

Committed r143541: <http://trac.webkit.org/changeset/143541>
Comment 7 WebKit Review Bot 2013-02-20 17:21:12 PST
All reviewed patches have been landed.  Closing bug.