[webkit-reviews] review requested: [Bug 19663] Image animation timers do not account for paint lag : [Attachment 23857] patch for (1) - (7), v1

[bugzilla-daemon at webkit.org]

Peter Kasting <pkasting at google.com> has asked  for review:
Bug 19663: Image animation timers do not account for paint lag
https://bugs.webkit.org/show_bug.cgi?id=19663

Attachment 23857: patch for (1) - (7), v1
https://bugs.webkit.org/attachment.cgi?id=23857&action=edit

------- Additional Comments from Peter Kasting <pkasting at google.com>
OK, this is ready for review.

The last WIP patch had some bugs that caused animations to erroneously skip
frames and run too fast, oops.

With this patch, animated images use more CPU than on stock Safari 3.1 (though
much less than in the previous WIP patch).  However, I think most of this extra
CPU usage is due to animating at the correct speed, instead of noticeably
slower.  For example, on http://img2.abload.de/img/almost_failedovs.gif on my
machine, a loop now peaks (near the end) at about 33% CPU instead of 19%.  But
the loop also runs in about 11.5 seconds versus 16.7 (not very precise hand
timings, probably only accurate to about 0.3 seconds).	Thus we use about 3/2
the CPU to run in 2/3 the time.  So I don't think this patch is making things
less efficient per frame than before, we're just running a much larger number
of frames per second on images that take significant decoding time per-frame.

To check the accuracy versus IE (whose frame delay time limits we're
imitating), try loading
http://stevesfishingsupplies.com/AboutSteve/Turning_Lady.gif in IE and WebKit
at roughly the same time, side-by-side, and watching over time for whether they
stay in sync.  Without this patch, Safari falls behind fairly quickly, whereas
with it the two stay pretty well locked together, including when we switch tabs
for a while and switch back.

I think this can be checked in without solving (8), which I'm still hoping for
hyatt to do separately.  Incidentally, I also think the CG GIF decoder could
use dramatically less CPU-- Firefox 3 manages to animate
http://img2.abload.de/img/almost_failedovs.gif using an order of magnitude less
CPU than Safari.  But that's someone else's problem.