Bug 41596 – Pattern fill with image not rendered after reload

• Component: SVG
• Status: RESOLVED FIXED

[Tony Sung]

Steps to reproduce:

1. Empty cache
2. Open the attachment in WebKit
3. Observe the image being rendered
4. Reload

Expected result:

1. An image is rendered

Actual result:

1. Nothing is rendered

[Tony Sung]

Created attachment 60508 [details]
Test case

[Simon Fraser (smfr)]

<rdar://problem/8159531>

[Simon Fraser (smfr)]

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

[Simon Fraser (smfr)]

The pattern is rendered into an ImageBuffer in RenderSVGResourcePattern::createTileImage(), by calling SVGRenderSupport::renderSubtreeToImage() with the RenderSVGImage. If this happens too early, there's no image yet, but there's also nothing to update the pattern once the image finally loads.

[Nikolas Zimmermann]

(In reply to comment #4)
> The pattern is rendered into an ImageBuffer in RenderSVGResourcePattern::createTileImage(), by calling SVGRenderSupport::renderSubtreeToImage() with the RenderSVGImage. If this happens too early, there's no image yet, but there's also nothing to update the pattern once the image finally loads.

There is, RenderSVGImage::imageChanged, contains following code to handle it:

    // The image resource defaults to nullImage until the resource arrives.
    // This empty image may be cached by SVG resources which must be invalidated.
    if (SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this))
        resources->invalidateClient(this);

Not sure what is happening, will check the testcase soon.

[Nikolas Zimmermann]

Created attachment 63807 [details]
Testcase that tiles

Same testcase as SVG only file, with a pattern that actually tiles.

[Nikolas Zimmermann]

(In reply to comment #5)
> (In reply to comment #4)
> > The pattern is rendered into an ImageBuffer in RenderSVGResourcePattern::createTileImage(), by calling SVGRenderSupport::renderSubtreeToImage() with the RenderSVGImage. If this happens too early, there's no image yet, but there's also nothing to update the pattern once the image finally loads.
> 
> There is, RenderSVGImage::imageChanged, contains following code to handle it:
> 
>     // The image resource defaults to nullImage until the resource arrives.
>     // This empty image may be cached by SVG resources which must be invalidated.
>     if (SVGResources* resources = SVGResourcesCache::cachedResourcesForRenderObject(this))
>         resources->invalidateClient(this);
> 
> Not sure what is happening, will check the testcase soon.

Ah I see, how stupid. The code above looks up resources defined on the _image_ itself.
eg. <image clip-path="..." and would notify the clip-path, that it has to recalculate. But if the <image> itself lives in a resources, that one is not updated. should be an easy fix.

[Nikolas Zimmermann]

Created attachment 63809 [details]
Patch

Bug was very easy to fix, creating a testcase was not that trivial.
I've chosen to create a http test, that utilizes a perl script to slowly load an image (1s delay, as used for the mask image test). Have a look :-)

[Nikolas Zimmermann]

Created attachment 63810 [details]
Patch v2

First patch contained a wrong version of the test, uncommented and with a delay of 2s.

[Daniel Bates]

Comment on attachment 63810 [details]
Patch v2

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 64899)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,18 @@
> [phone]  Nikolas Zimmermann  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Pattern fill with image not rendered after reload
> +        https://bugs.webkit.org/show_bug.cgi?id=41596
> +
> +        Fixed by a one-liner. Instead of just invalidating it's own resources, images also have to update

Nit: "it's" should be "its".

> +        resources in the ancestor chain, if imageChanged() has been called (for example, a slow loading image)
> +
> +        Test: http/tests/misc/slow-loading-image-in-pattern.html

Looks good to me.

r=me.

[Nikolas Zimmermann]

Landed in r64901.