Bug 239906 - REGRESSION (r289032): rotate animation doesn't interpolate between 0 and 1turn without forced 50%
Summary: REGRESSION (r289032): rotate animation doesn't interpolate between 0 and 1tur...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: Safari Technology Preview
Hardware: Mac (Apple Silicon) macOS 12
: P2 Normal
Assignee: Martin Robinson
URL: https://codepen.io/mattaningram/pen/z...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-29 11:40 PDT by Mattan Ingram
Modified: 2022-09-13 00:32 PDT (History)
8 users (show)

See Also:


Attachments
Reduced test case using a single function for transform (449 bytes, text/html)
2022-05-11 07:54 PDT, Martin Robinson
no flags Details
Patch (8.02 KB, patch)
2022-05-13 07:02 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.