Bug 37955 - When webkitAnimationEnd event fires, on-screen rendering should show the last frame of animation
Summary: When webkitAnimationEnd event fires, on-screen rendering should show the last...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-21 14:57 PDT by Simon Fraser (smfr)
Modified: 2010-04-23 13:23 PDT (History)
2 users (show)

See Also:


Attachments
Testcase (1.43 KB, text/html)
2010-04-21 17:56 PDT, Simon Fraser (smfr)
no flags Details
Patch (5.53 KB, patch)
2010-04-23 10:40 PDT, Simon Fraser (smfr)
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2010-04-21 14:57:32 PDT
When the end-animation event fires, the rendering should show the last frame of animation. This way, JS can setup post-animation style without a flash.
Comment 1 Simon Fraser (smfr) 2010-04-21 17:56:32 PDT
This is also visible at http://www.jqtouch.com/preview/demos/main/#home
Comment 2 Simon Fraser (smfr) 2010-04-21 17:56:49 PDT
Created attachment 54010 [details]
Testcase
Comment 3 Simon Fraser (smfr) 2010-04-22 20:24:34 PDT
The changes in http://trac.webkit.org/changeset/37484 made it so that we fire webkitAnimationEnd events asynchronously, after we've restored the original, unanimated style. This opened up a window where the element has reverted to its original position before the event fires.
Comment 4 Simon Fraser (smfr) 2010-04-22 20:38:47 PDT
Possible fix:

diff --git a/WebCore/page/animation/AnimationController.cpp b/WebCore/page/animation/AnimationController.cpp
index cb609a5..a0da350 100644
--- a/WebCore/page/animation/AnimationController.cpp
+++ b/WebCore/page/animation/AnimationController.cpp
@@ -134,6 +134,11 @@ void AnimationControllerPrivate::updateAnimationTimer(bool callSetChanged/* = fa
 
 void AnimationControllerPrivate::updateStyleIfNeededDispatcherFired(Timer<AnimationControllerPrivate>*)
 {
+    fireEventsAndUpdateStyle();
+}
+
+void AnimationControllerPrivate::fireEventsAndUpdateStyle()
+{
     // Protect the frame from getting destroyed in the event handler
     RefPtr<Frame> protector = m_frame;
 
@@ -196,6 +201,10 @@ void AnimationControllerPrivate::animationTimerFired(Timer<AnimationControllerPr
     // When the timer fires, all we do is call setChanged on all DOM nodes with running animations and then do an immediate
     // updateStyleIfNeeded.  It will then call back to us with new information.
     updateAnimationTimer(true);
+
+    // Fire events right away, to avoid a flash of unanimated style after an animation completes, and before
+    // the 'end' event fires.
+    fireEventsAndUpdateStyle();
 }
 
 bool AnimationControllerPrivate::isAnimatingPropertyOnRenderer(RenderObject* renderer, CSSPropertyID property, bool isRunningNow) const
diff --git a/WebCore/page/animation/AnimationControllerPrivate.h b/WebCore/page/animation/AnimationControllerPrivate.h
index 682dd75..5ef9098 100644
--- a/WebCore/page/animation/AnimationControllerPrivate.h
+++ b/WebCore/page/animation/AnimationControllerPrivate.h
@@ -92,6 +92,7 @@ public:
     
 private:
     void styleAvailable();
+    void fireEventsAndUpdateStyle();
 
     typedef HashMap<RenderObject*, RefPtr<CompositeAnimation> > RenderObjectAnimationMap;
Comment 5 Simon Fraser (smfr) 2010-04-22 20:47:09 PDT
This patch causes some DRT assertions in AnimationBase::freezeAtTime(), because we try to freeze animations before we've started them. Maybe we only need to process 'end' events in this new code?
Comment 6 Simon Fraser (smfr) 2010-04-23 10:40:41 PDT
Created attachment 54172 [details]
Patch
Comment 7 Simon Fraser (smfr) 2010-04-23 10:59:00 PDT
Comment on attachment 54172 [details]
Patch

> +        Fix by firing these events in the same event cycle as the animation end, once all animations
> +        have been updated. This also required moving the place that start animations are fixed until

s/fixed/fired
Comment 8 Simon Fraser (smfr) 2010-04-23 13:23:11 PDT
http://trac.webkit.org/changeset/58186