|Summary:||Animated SVGs do not clear previous frame completely in hidpi mode.|
|Product:||WebKit||Reporter:||Varun Jain <varunjain>|
|Component:||New Bugs||Assignee:||Varun Jain <varunjain>|
|Severity:||Normal||CC:||abarth, dglazkov, eae, eric, leviw, pdr, vollick, webkit.review.bot, zimmermann|
|Version:||528+ (Nightly build)|
Description Varun Jain 2012-07-26 10:36:02 PDT
Animated SVGs do not clear previous frame completely in hidpi mode.
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 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.