Bug 239906

Summary: REGRESSION (r289032): rotate animation doesn't interpolate between 0 and 1turn without forced 50%
Product: WebKit Reporter: Mattan Ingram <mattaningram>
Component: AnimationsAssignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, ews-watchlist, graouts, graouts, mattaningram, mrobinson, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Mac (Apple Silicon)   
OS: macOS 12   
URL: https://codepen.io/mattaningram/pen/zYROyqL
See Also: https://bugs.webkit.org/show_bug.cgi?id=235311
Attachments:
Description Flags
Reduced test case using a single function for transform
none
Patch none

Description Mattan Ingram 2022-04-29 11:40:45 PDT 
In certain situations Safari Technology Preview is not interpolating properly between 0 and 1turn in a keyframe animation.

I noticed my loading spinner wasn't working anymore in Safari Technology Preview so I created a codepen to illustrate the bug:

https://codepen.io/mattaningram/pen/zYROyqL

If you uncomment the second spinner animation keyframe you will see that forcing Safari to interpolate to a 50% position in the 0 to 1 turn make the animation behave the way it does in older Safari versions and other browsers. However this behavior of not automatically interpolating between 0 and 1turn is not reproducible in a simpler example.
Comment 1 Radar WebKit Bug Importer 2022-05-06 11:41:13 PDT 
<rdar://problem/92872316>
Comment 2 Antoine Quint 2022-05-11 05:35:19 PDT 
Regressed with r289032, the fix for bug 235311. Cc'ing Martin Robinson, who wrote the regressing commit.
Comment 3 Martin Robinson 2022-05-11 07:40:37 PDT 
This seems to be an issue with CoreAnimation. I confirmed that the test case worked for WebKitGTK (which just animates using WebCore interpolation), but fails for hardware animations using the CoreAnimation backend.
Comment 4 Martin Robinson 2022-05-11 07:54:47 PDT 
Created attachment 459166 [details]
Reduced test case using a single function for transform

I suspect that this was an issue before, but was hidden by the fact that this animation was previously falling back to software. I've attached a reduced test case that fails in Mac WebKit and only uses a single transform function (ie should not be affected by my change).
Comment 5 Antoine Quint 2022-05-11 08:49:38 PDT 
It's perfectly fine to not run an accelerated animation for this case if we can't find a way to run it with CA, especially if that was our previous behavior.
Comment 6 Martin Robinson 2022-05-11 08:50:54 PDT 
(In reply to Antoine Quint from comment #5)
> It's perfectly fine to not run an accelerated animation for this case if we
> can't find a way to run it with CA, especially if that was our previous
> behavior.

It's quite possible this is indeed a regression, but I don't understand how yet. I'll do a deeper investigation.
Comment 7 Martin Robinson 2022-05-13 07:02:10 PDT 
Created attachment 459298 [details]
Patch
Comment 8 Antoine Quint 2022-05-13 07:04:55 PDT 
Writing tests for the runtime behavior of an accelerated animation is notoriously difficult :(
Comment 9 Martin Robinson 2022-05-13 07:06:02 PDT 
I've posted what I think is a fix for this, but haven't yet marked it for review. I'm investigating now how to write a test. When pausing an animation (an approach that's often used for reference testing), I believe that WebKit uses the software path because rendering is correct without the fix.
Comment 10 Martin Robinson 2022-05-19 08:02:41 PDT 
Pull request: https://github.com/WebKit/WebKit/pull/781
Comment 11 EWS 2022-05-24 09:58:37 PDT 
Committed r294752 (250920@main): <https://commits.webkit.org/250920@main>

Reviewed commits have been landed. Closing PR #781 and removing active labels.
Comment 12 Antoine Quint 2022-09-13 00:32:10 PDT 
This caused bug 243864.