Bug 92395

Summary: Animated SVGs do not clear previous frame completely in hidpi mode.
Product: WebKit Reporter: Varun Jain <varunjain>
Component: New BugsAssignee: Varun Jain <varunjain>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, eae, eric, leviw, pdr, vollick, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from gce-cr-linux-02
none
Patch
none
Patch
none
Patch none

Description Varun Jain 2012-07-26 10:36:02 PDT
Animated SVGs do not clear previous frame completely in hidpi mode.
Comment 1 Varun Jain 2012-07-26 10:38:44 PDT
Created attachment 154684 [details]
Patch
Comment 2 WebKit Review Bot 2012-07-26 11:31:47 PDT
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
Comment 3 WebKit Review Bot 2012-07-26 11:31:51 PDT
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
Comment 4 Varun Jain 2012-07-26 17:19:55 PDT
Created attachment 154795 [details]
Patch
Comment 5 Varun Jain 2012-07-27 09:06:46 PDT
Created attachment 154969 [details]
Patch
Comment 6 Varun Jain 2012-07-27 09:16:08 PDT
Created attachment 154972 [details]
Patch
Comment 7 Adam Barth 2012-07-27 10:59:57 PDT
Comment on attachment 154972 [details]
Patch

Makes sense.
Comment 8 Adam Barth 2012-07-27 11:00:39 PDT
@Eric or Wildfox, would either of you be willing to review this patch?  Thanks!
Comment 9 Eric Seidel (no email) 2012-07-27 11:51:35 PDT
Seems reasonable to me.
Comment 10 Eric Seidel (no email) 2012-07-27 11:53:09 PDT
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.
Comment 11 Varun Jain 2012-07-27 11:57:25 PDT
(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.
Comment 12 Darin Adler 2012-07-27 13:05:59 PDT
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.
Comment 13 WebKit Review Bot 2012-07-27 13:43:50 PDT
Comment on attachment 154972 [details]
Patch

Clearing flags on attachment: 154972

Committed r123914: <http://trac.webkit.org/changeset/123914>
Comment 14 WebKit Review Bot 2012-07-27 13:43:55 PDT
All reviewed patches have been landed.  Closing bug.