Bug 92395 – Animated SVGs do not clear previous frame completely in hidpi mode.

• Component: New Bugs
• Status: RESOLVED FIXED

[Varun Jain]

Animated SVGs do not clear previous frame completely in hidpi mode.

[Varun Jain]

Created attachment 154684 [details]
Patch

[WebKit Review Bot]

Comment on attachment 154684 [details]
Patch

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

New failing tests:
svg/as-image/animated-svg-as-image-no-fixed-intrinsic-size.html
svg/as-image/animated-svg-as-image-no-fixed-intrinsic-size-hidpi.html

[WebKit Review Bot]

Created attachment 154701 [details]
Archive of layout-test-results from gce-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Varun Jain]

Created attachment 154795 [details]
Patch

[Varun Jain]

Created attachment 154969 [details]
Patch

[Varun Jain]

Created attachment 154972 [details]
Patch

[Adam Barth]

Comment on attachment 154972 [details]
Patch

Makes sense.

[Adam Barth]

@Eric or Wildfox, would either of you be willing to review this patch?  Thanks!

[Eric Seidel (no email)]

Seems reasonable to me.

[Eric Seidel (no email)]

Comment on attachment 154972 [details]
Patch

It looks like you're using a repaint test, but it's repainting the whole area, which doesn't seem right.  The change looks OK, but it's difficult to tell from you test.

[Varun Jain]

(In reply to comment #10)
> (From update of attachment 154972 [details])
> It looks like you're using a repaint test, but it's repainting the whole area, which doesn't seem right.  The change looks OK, but it's difficult to tell from you test.

If the test is run without the change, its very easy to see that its broken. This test is exactly the same as svg/as-image/animated-svg-as-image-no-fixed-intrinsic-size.html.. only difference is the call to setBackingScale(2) before starting the test.

[Darin Adler]

Comment on attachment 154972 [details]
Patch

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

> Source/WebCore/svg/graphics/SVGImage.cpp:175
> +    if (shouldClear == ClearImageBuffer)
> +        buffer->context()->clearRect(enclosingIntRect(scaledRect));
> +
>      // Draw SVG on top of ImageBuffer.
>      draw(buffer->context(), enclosingIntRect(scaledRect), rect, ColorSpaceDeviceRGB, CompositeSourceOver);

It’s a bit inelegant to compute enclosingIntRect(scaledRect) twice.

[WebKit Review Bot]

Comment on attachment 154972 [details]
Patch

Clearing flags on attachment: 154972

Committed r123914: <http://trac.webkit.org/changeset/123914>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.