SVGImageCache isn't invalidated for <img> on dynamic page scale changes

• Component: SVG
• Status: RESOLVED FIXED

[Tim Horton]

SVGImageCache's cached understanding of the scale of an image gets updated in CachedImage::setContainerSizeForRenderer.

SVG-as-background-image updates the scale because RenderBoxModelObject::calculateBackgroundImageGeometry calls setContainerSizeForRenderer, so the scale is updated. However, SVG-as-<img> only updates the scale when *layout* happens.

We don't always layout during gesture zoom, but we do change the page scale and repaint.

I have a fix.

[Radar WebKit Bug Importer]

<rdar://problem/11714677>

[Radar WebKit Bug Importer]

<rdar://problem/11714681>

[Tim Horton]

Created attachment 148730 [details]
patch

Have to talk to Xianzhu Wang and see if there was a reason for the Renderer->Client change in lookupOrCreateBitmapImageForRenderer. Seems like it's just losing us valuable type information (especially for this patch).

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
Source/WebCore/svg/graphics/SVGImageCache.cpp:147:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 8 files


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

[Tim Horton]

> INVALID: Image lacks a checksum. This will fail with a MISSING error in run-webkit-tests. Always generate new png files using run-webkit-tests.

Wot. That's how I generated them. I've also never seen this message before.

[Xianzhu Wang]

(In reply to comment #3)
> Created an attachment (id=148730) [details]
> patch
> 
> Have to talk to Xianzhu Wang and see if there was a reason for the Renderer->Client change in lookupOrCreateBitmapImageForRenderer. Seems like it's just losing us valuable type information (especially for this patch).

The change was because at that time the renderer was only used as a client. It's absolutely OK to change back to renderer if the renderer interface is needed.

[Simon Fraser (smfr)]

Comment on attachment 148730 [details]
patch

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

> LayoutTests/platform/mac/svg/as-image/image-respects-pageScaleFactor-change-expected.txt:17
> +layer at (0,0) size 785x585
> +  RenderView at (0,0) size 785x585
> +layer at (0,0) size 785x585
> +  RenderBlock {HTML} at (0,0) size 785x585
> +    RenderBody {BODY} at (8,8) size 769x561
> +      RenderBlock (anonymous) at (0,0) size 769x132
> +        RenderImage {IMG} at (0,96) size 32x32
> +        RenderText {#text} at (32,114) size 4x18
> +          text run at (32,114) width 4: " "
> +        RenderImage {IMG} at (36,0) size 128x128
> +        RenderText {#text} at (0,0) size 0x0
> +      RenderBlock {P} at (0,148) size 769x18
> +        RenderText {#text} at (0,0) size 342x18
> +          text run at (0,0) width 342: "This test passes if both of the circles have sharp edges."
> +      RenderBlock {P} at (0,182) size 769x18
> +        RenderText {#text} at (0,0) size 440x18
> +          text run at (0,0) width 440: "To run manually, use full-page zoom to increase the page scale factor."

Is this useful? Maybe dumpAsText(true).

> Source/WebCore/svg/graphics/SVGImageCache.cpp:148
> +        scale = renderer->document()->page()->deviceScaleFactor() * renderer->document()->page()->pageScaleFactor();

Might as well fetch renderer->document()->page() just once.

[WebKit Review Bot]

Comment on attachment 148730 [details]
patch

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

New failing tests:
svg/as-image/image-respects-pageScaleFactor-change.html

[WebKit Review Bot]

Created attachment 148739 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Tim Horton]

(In reply to comment #6)
> The change was because at that time the renderer was only used as a client. It's absolutely OK to change back to renderer if the renderer interface is needed.

Woo! That's what I was hoping to hear.

> Is this useful? Maybe dumpAsText(true).

Quite right.

> Might as well fetch renderer->document()->page() just once.

Sure.

Thanks, Simon!

[Tim Horton]

http://trac.webkit.org/changeset/120953