The animation callback 'setCurrentProgress' is never called.
<rdar://problem/38070914>
Created attachment 334900 [details] Patch
Created attachment 334901 [details] Patch
Using a timer here gets rid of the animation curve. I don't think that's what you intended to do.
Comment on attachment 334900 [details] Patch What Anders said.
Created attachment 335010 [details] Patch
(In reply to Tim Horton from comment #5) > Comment on attachment 334900 [details] > Patch > > What Anders said. I have created a patch where the animation curve is based on a sin formula. Thanks for reviewing, all!
Comment on attachment 335010 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335010&action=review > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:382 > + // Use a formula based on sin to create an animation curve where the opacity rate of change is low > + // in the beginning and at the end of the animation, but higher in the middle of the animation. > + // This should be similar to the 'NSAnimationEaseInOut' animation curve. > + auto t = elapsed / _duration; > + progress = 0.5 * sin(M_PI * t - M_PI / 2) + 0.5; Is it? We have code in WebKit for CA's ease-in-out curve. You could use that instead.
(In reply to Dean Jackson from comment #8) > Comment on attachment 335010 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335010&action=review > > > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:382 > > + // Use a formula based on sin to create an animation curve where the opacity rate of change is low > > + // in the beginning and at the end of the animation, but higher in the middle of the animation. > > + // This should be similar to the 'NSAnimationEaseInOut' animation curve. > > + auto t = elapsed / _duration; > > + progress = 0.5 * sin(M_PI * t - M_PI / 2) + 0.5; > > Is it? We have code in WebKit for CA's ease-in-out curve. You could use that > instead. Thanks! That is a good point. I was planning on implementing the scrollbar animation using CA layer animations as suggested earlier. The track and thumb of the scrollbar needs to be animated individually, but this might not be trivial with the CA layer approach, since the thumb and track then would need to be rendered in separate layers, I believe.
Created attachment 335160 [details] Patch
Created attachment 335161 [details] Patch
Comment on attachment 335161 [details] Patch Attachment 335161 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/6835370 New failing tests: http/wpt/resource-timing/rt-initiatorType-media.html
Created attachment 335172 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 335160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335160&action=review I'm giving this an r+ but I think it would be better if we got the animation data from AppKit rather than simulate it ourselves. That way, if/when AppKit decides to tweak the animation function, we remain in sync. > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:382 > + auto t = elapsed / _duration; We should probably have a guarantee that _duration is never 0. > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:418 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED < 101400 > [self stopAnimation]; > +#else It's a bit confusing that this calls stopAnimation on older OS versions, but you implement a separate stopAnimation call on newer versions.
(In reply to Dean Jackson from comment #14) > Comment on attachment 335160 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=335160&action=review > > I'm giving this an r+ but I think it would be better if we got the animation > data from AppKit rather than simulate it ourselves. That way, if/when AppKit > decides to tweak the animation function, we remain in sync. > Yes, I completely agree. I think there also is an agreement on that it would be better to use CA layer animation to do this, but since we need to individually animate the track and thumb of the scrollbar, this is more work. But, we should look further into this. > > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:382 > > + auto t = elapsed / _duration; > > We should probably have a guarantee that _duration is never 0. > Good catch! I will fix that. > > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:418 > > +#if __MAC_OS_X_VERSION_MIN_REQUIRED < 101400 > > [self stopAnimation]; > > +#else > > It's a bit confusing that this calls stopAnimation on older OS versions, but > you implement a separate stopAnimation call on newer versions. Yes, definitely, I will fix this before landing. Thanks for reviewing!
Created attachment 335316 [details] Patch
Comment on attachment 335316 [details] Patch Clearing flags on attachment: 335316 Committed r229419: <https://trac.webkit.org/changeset/229419>