Adjust keyboard scrolling animator to be a bit more physical
Created attachment 351763 [details] Patch
I'll flesh out the changelog a bit. Also I think maybe we can unit test the class independent of a scroll view, which would be nice, so I'll try that.
Created attachment 351794 [details] Patch
Created attachment 351795 [details] Patch
Comment on attachment 351795 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351795&action=review > Source/WebKit/ChangeLog:3 > + Adjust keyboard scrolling animator to be a bit more physical "a bit more" > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3854 > + return self.bounds.size.height * selfScale; Wouldn't that be the content view height? > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3858 > + return WebCore::Scrollbar::pixelsPerLineStep() * selfScale; Maybe get this from the content view too? > Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:46 > + WebCore::FloatSize offset; // Points per increment. > + WebCore::FloatSize maximumVelocity; // Points per second. The comments indicate that the members could be named better (or maybe some typedef types) > Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:142 > + return WebCore::FloatSize(0, -1); return { 0., -1. } ? > Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:292 > + _velocity = WebCore::FloatSize(); = { } > Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:380 > + _velocity = WebCore::FloatSize(); = { } > Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:391 > + WebCore::RectEdges<bool> scrollableDirections = [_scrollable scrollableDirectionsFromOffset:_currentPosition]; auto > Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:421 > + // Compute the spring's force, and apply it in allowed directions. > + // F_spring = -k * x - c * v > + auto springForce = - displacement.scaled(self.parameters.springStiffness) - _velocity.scaled(self.parameters.springDamping); > + force += springForce * axesToApplySpring; I kinda feel like all the spring logic should be packaged up in a separate helper class. > Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:435 > + _velocity = WebCore::FloatSize(); = { } > Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm:580 > + return CGSizeMake(scrollView._horizontalVelocity * 1000, scrollView._verticalVelocity * 1000); Please document the * 1000
Created attachment 351799 [details] Patch
Created attachment 351806 [details] Patch
Comment on attachment 351806 [details] Patch Clearing flags on attachment: 351806 Committed r236935: <https://trac.webkit.org/changeset/236935>
All reviewed patches have been landed. Closing bug.
<rdar://problem/45101935>
(In reply to WebKit Commit Bot from comment #8) > Comment on attachment 351806 [details] > Patch > > Clearing flags on attachment: 351806 > > Committed r236935: <https://trac.webkit.org/changeset/236935> This regressed key repeats for arrow keys on iOS device. See bug #191408 for more details.