Expose a prototype implementation of what will eventually be called animation-timebase, mapping the scroll location to the duration of an animation. This only applies if the animation has a start and end trigger.
<rdar://problem/20227244>
Created attachment 249067 [details] Patch
Comment on attachment 249067 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249067&action=review > Source/WebCore/css/CSSToStyleMap.cpp:539 > + bool hasEnd = false; I don't think you need the hasEnd variable. > Source/WebCore/page/animation/AnimationBase.cpp:482 > + // time is determined by the scroll position. We should stop calling this elapsedTime > Source/WebCore/page/animation/AnimationBase.cpp:710 > + if (scrollTrigger.hasEndValue() && m_object) { Can we make m_object a reference (later)? > Source/WebCore/page/animation/AnimationBase.cpp:711 > + float offset = m_object->view().frameView().scrollOffsetForFixedPosition().height().toFloat(); I think we should snapshot the scroll position when we snapshot the animation time. Fetching it all over the place feels error-prone. > Source/WebCore/page/animation/AnimationBase.cpp:718 > + return m_animation->duration() * (offset - startValue) / (endValue - startValue); What if endValue == startValue? Please add a test for this case. > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:871 > + ScrollAnimationTrigger& scrollTrigger = downcast<ScrollAnimationTrigger>(*anim->trigger().get()); > + if (scrollTrigger.hasEndValue()) What about startValue?
Created attachment 249076 [details] Patch
Comment on attachment 249067 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249067&action=review >> Source/WebCore/css/CSSToStyleMap.cpp:539 >> + bool hasEnd = false; > > I don't think you need the hasEnd variable. I removed it but it means I have two calls to setTrigger, each side of the conditional. >> Source/WebCore/page/animation/AnimationBase.cpp:482 >> + // time is determined by the scroll position. > > We should stop calling this elapsedTime Agreed. I'll do that in a followup patch. >> Source/WebCore/page/animation/AnimationBase.cpp:710 >> + if (scrollTrigger.hasEndValue() && m_object) { > > Can we make m_object a reference (later)? Yes. >> Source/WebCore/page/animation/AnimationBase.cpp:711 >> + float offset = m_object->view().frameView().scrollOffsetForFixedPosition().height().toFloat(); > > I think we should snapshot the scroll position when we snapshot the animation time. Fetching it all over the place feels error-prone. Done >> Source/WebCore/page/animation/AnimationBase.cpp:718 >> + return m_animation->duration() * (offset - startValue) / (endValue - startValue); > > What if endValue == startValue? > > Please add a test for this case. OK!
Checking the patch on EWS before landing.
Attachment 249076 [details] did not pass style-queue: ERROR: Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1241: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r181778: <http://trac.webkit.org/changeset/181778>
(In reply to comment #8) > Committed r181778: <http://trac.webkit.org/changeset/181778> This broke animations/trigger-container-scroll-empty.html on Windows and EFL: https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=animations%2Ftrigger-container-scroll-empty.html