It is important that animations on the main and impl thread have the same start time. With 81106, we will be ticking animations on the main thread and it is important that the same value be interpolated at time t on either thread. It is no longer important to set the final values of animations in response to an animation finishing -- animations on the main thread will finish naturally and set their final values.
Created attachment 132534 [details] Patch Will post another CL with the ChangeLog changes when 81106 lands.
Created attachment 132827 [details] Patch Eliminated animation finished events. Animations will finish naturally on the main thread and the final values will be set. Animation started events are now also used to synchronize the main thread animations' start times with their impl thread equivalents, preventing skew. Until main thread animations receive their synchronized start times, they will apply their initial values (i.e., they will be paused at time zero). This guarantees that we will not jump to the animation's final value on the main thread while we wait for the synchronized start time.
Comment on attachment 132827 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132827&action=review A thing of beauty. @Enne. > Source/WebCore/platform/graphics/chromium/cc/CCAnimationEvents.cpp:-33 > -CCAnimationEvent::CCAnimationEvent(int layerId, CCActiveAnimation::TargetProperty targetProperty) No need for the CPP anymore, methinks? Just put the constructor in the .h, c.f. CCLayerTreeHost.h's CCSettings
Comment on attachment 132827 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132827&action=review Drive by language question: is monotonic time a synonym for wall clock time here? > Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp:391 > + if (m_activeAnimations[i]->runState() == CCActiveAnimation::Running && !m_activeAnimations[i]->needsSynchronizedStartTime()) { > double trimmed = m_activeAnimations[i]->trimTimeToCurrentIteration(monotonicTime); > > + // Animation assumes its initial value until it gets the synchronized start time > + // from the impl thread and can start ticking. > + if (m_activeAnimations[i]->needsSynchronizedStartTime()) > + trimmed = 0; I don't understand this second conditional here. It looks like it will never be true, based on the first conditional.
Created attachment 132950 [details] Patch (In reply to comment #3) CCAnimationEvents.cpp is toast. (In reply to comment #4) > (From update of attachment 132827 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132827&action=review > > Drive by language question: is monotonic time a synonym for wall clock time here? > I've been using monotonicTime to refer to times obtained via monotonicallyIncreasingTime() and wallClockTime for times obtained using currentTime() > > Source/WebCore/platform/graphics/chromium/cc/CCLayerAnimationController.cpp:391 > > + if (m_activeAnimations[i]->runState() == CCActiveAnimation::Running && !m_activeAnimations[i]->needsSynchronizedStartTime()) { > > double trimmed = m_activeAnimations[i]->trimTimeToCurrentIteration(monotonicTime); > > > > + // Animation assumes its initial value until it gets the synchronized start time > > + // from the impl thread and can start ticking. > > + if (m_activeAnimations[i]->needsSynchronizedStartTime()) > > + trimmed = 0; > > I don't understand this second conditional here. It looks like it will never be true, based on the first conditional. Good catch. The first conditional was meant to be deleted. Fixed.
Comment on attachment 132950 [details] Patch Thanks for the language clarification. I must have misread some code to think that monotonic and wall clock were being interchangeably used. R=me, mostly due to Nat's review.
Comment on attachment 132950 [details] Patch Clearing flags on attachment: 132950 Committed r111525: <http://trac.webkit.org/changeset/111525>
All reviewed patches have been landed. Closing bug.