Bug 216308

Summary: REGRESSION (r260360): Ionic modal dialog doesn't animate correctly when dragged and released
Product: WebKit Reporter: Antoine Quint <graouts>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, ews-watchlist, graouts, ldebeasi, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=215853
Attachments:
Description Flags
Ionic modal component demo
none
Reduction
none
Patch none

Antoine Quint
Reported 2020-09-09 05:41:22 PDT
This is a followup to bug 215853. Steps to reproduce: 1. Go to https://ionicframework.com/docs 2. In the iOS device on the right, scroll down to "Modal" and select that row 3. Click on "Open Modal" 4. Drag down a bit on the modal's title bar 5. Release it Prior to r260360 this would animate back nicely but now it no longer does that.
Attachments
Ionic modal component demo (1.86 KB, text/html)
2020-09-10 02:06 PDT, Antoine Quint
no flags
Reduction (497 bytes, text/html)
2020-09-10 07:37 PDT, Antoine Quint
no flags
Patch (6.54 KB, patch)
2020-09-10 08:16 PDT, Antoine Quint
no flags
Radar WebKit Bug Importer
Comment 1 2020-09-09 05:42:03 PDT
Antoine Quint
Comment 2 2020-09-09 06:06:14 PDT
Note that with r260360 the animations are generally broken, due to the easing functions not being applied correctly. This was addressed by r263466 but did not revert the behavior to what was expected and was correct pre-r260360.
Antoine Quint
Comment 3 2020-09-10 02:06:47 PDT
Created attachment 408423 [details] Ionic modal component demo Attached a demo of the modal component in isolation to help debug.
Antoine Quint
Comment 4 2020-09-10 03:38:29 PDT
The method called on the Ionic Animation object as the gesture ends is progressEnd(0, 0.45330467732456015, 500). I assume we attempt a 500ms animation started from a 0.45 point and we're not starting it from the provided progress but from the beginning which is what it looks like.
Antoine Quint
Comment 5 2020-09-10 03:41:10 PDT
Actually, the 0 probably means to play to the start value, so play the animation in reverse.
Antoine Quint
Comment 6 2020-09-10 07:37:41 PDT
Created attachment 408442 [details] Reduction Attached what I think is a reduction of the problem. In this code we run an animation for 1s with a linear easing and then update the timing at mid-way to change the easing to a bezier-curve and change the "direction" property. I think we fail to account for the "direction" when we set the current time of the accelerated animation.
Antoine Quint
Comment 7 2020-09-10 08:08:36 PDT
Yes, the reduction is correct, I have a fix that addresses it and the Ionic demo.
Antoine Quint
Comment 8 2020-09-10 08:16:50 PDT
Simon Fraser (smfr)
Comment 9 2020-09-10 10:06:39 PDT
Comment on attachment 408446 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408446&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1032 > + if (anim->playbackRate() != 1 || !anim->directionIsForwards()) > return false; Why don't we just adjust the timings given to CA so we can still accelerate these?
EWS
Comment 10 2020-09-10 10:31:44 PDT
Committed r266834: <https://trac.webkit.org/changeset/266834> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408446 [details].
Antoine Quint
Comment 11 2020-09-10 10:48:49 PDT
(In reply to Simon Fraser (smfr) from comment #9) > Comment on attachment 408446 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=408446&action=review > > > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1032 > > + if (anim->playbackRate() != 1 || !anim->directionIsForwards()) > > return false; > > Why don't we just adjust the timings given to CA so we can still accelerate > these? We could. However, my plan is to address both the case of non-1 playback rates and directions in one fell swoop with CAAnimationGroup. See bug 211839 for details.
Note You need to log in before you can comment on or make changes to this bug.