Bug 94874 - [mac] REGRESSION (r122215): Animated GIF outside the viewport doesn't play when scrolled into view
Summary: [mac] REGRESSION (r122215): Animated GIF outside the viewport doesn't play wh...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Nobody
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2012-08-23 16:56 PDT by Tim Horton
Modified: 2012-10-05 18:28 PDT (History)
11 users (show)

See Also:


Attachments
Patch (5.88 KB, patch)
2012-10-05 17:46 PDT, Dongseong Hwang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2012-08-23 16:56:19 PDT
Steps to Reproduce:

1. On Mountain Lion, with the Mac port (where tiled scrolling is enabled):
2. With a relatively small window:
3. Load http://whenapthuntinginsf.tumblr.com.
4. Scroll down.

Expected: All of the GIFs animate.

Actual: The GIFs which are scrolled into view do not animate.

This is probably due to the fact that they are offscreen (on offscreen tiles) when they are first painted, so they don't animate. Then, when they are scrolled into view, they don't need to repaint, so they don't realize they should start animating.

This will probably require some special logic in the tiled scrolling case.

<rdar://problem/12067641>
Comment 1 Tim Horton 2012-08-23 17:04:49 PDT
This regressed in http://trac.webkit.org/changeset/122215. The ChangeLog entry for that patch includes the comment: "This patch makes GIF animation outside the viewport be paused."
Comment 2 Dongseong Hwang 2012-08-23 18:57:41 PDT
(In reply to comment #1)
> This regressed in http://trac.webkit.org/changeset/122215. The ChangeLog entry for that patch includes the comment: "This patch makes GIF animation outside the viewport be paused."

I'm sorry for this regression.

I think the tiled scrolling disables RenderObject::willRenderImage from checking if the renderer is outside the viewport. "viewRect().intersects(absoluteClippedOverflowRect())" is often used in WebKit, such as spatial navigation.
I think the tiled scrolling should return the proper value.

bool RenderObject::willRenderImage(CachedImage*)
{
    ....
    // If a renderer is outside the viewport, we won't render.
    return viewRect().intersects(absoluteClippedOverflowRect());
}
Comment 3 Dongseong Hwang 2012-10-05 17:46:10 PDT
Created attachment 167425 [details]
Patch
Comment 4 Dongseong Hwang 2012-10-05 17:48:55 PDT
(In reply to comment #3)
> Created an attachment (id=167425) [details]
> Patch

Rollback previous patch because this patch caused two problems.
1. GIF animation is occasionally paused when tiled scrolling is enabled.
2. This change regressed Apple's Membuster benchmark by ~20% (80MB.)
Comment 5 WebKit Review Bot 2012-10-05 18:27:57 PDT
Comment on attachment 167425 [details]
Patch

Clearing flags on attachment: 167425

Committed r130573: <http://trac.webkit.org/changeset/130573>
Comment 6 WebKit Review Bot 2012-10-05 18:28:01 PDT
All reviewed patches have been landed.  Closing bug.