Have animations more closely match the HTML spec #26407

mrobinson · 2020-05-04T11:56:30Z

These two commits do two major things:

Have animations ticks trigger a restyle: This corrects synchronization issues with animations,
where the values used in layout are out of sync with what is returned by getComputedStyle.

Tick the animation timer in script according to spec: This greatly reduces the flakiness of
animation and transitions tests.

Fixes #13865.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #13865
There are tests for these changes

highfive · 2020-05-04T11:56:36Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/window.rs, components/script/dom/testbinding.rs, components/script/dom/webidls/TestBinding.webidl, components/script/script_thread.rs, components/script/dom/node.rs and 1 more
@cbrewster: components/constellation/constellation.rs
@KiChjang: components/script/dom/window.rs, components/script_traits/lib.rs, components/script/dom/testbinding.rs, components/script/dom/webidls/TestBinding.webidl, components/script/script_thread.rs and 2 more
@emilio: components/layout/animation.rs, components/style/matching.rs, components/style/animation.rs, components/style/context.rs

highfive · 2020-05-04T11:56:38Z

Warning

These commits modify unsafe code. Please review it carefully!

mrobinson · 2020-05-04T12:04:24Z

@bors-servo try=wpt

bors-servo · 2020-05-04T12:04:28Z

⌛ Trying commit 04d8e27 with merge 101c6e7...


        Auto merge of #26407 - mrobinson:animation-restyle-model, r=<try>

[RFC] Use a restyle for animation ticks This change corrects synchronization issues with animations, by reworking the animation processing model to do a quick restyle and incremental layout when ticking animations. While this change adds overhead to animation ticks, the idea is that this will be the fallback when synchronous behavior is required to fulfill specification requirements. In the optimistic case, many animations could be updated and applied off-the-main-thread and then resynchronized when style information is queried by script. Fixes #13865.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #13865  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2020-05-04T12:47:46Z

💔 Test failed - status-taskcluster

jdm · 2020-05-04T15:04:41Z

Should this PR enable the disabled tests, or are there other prerequisites?

mrobinson · 2020-05-04T15:30:15Z

@jdm, yep. Currently, I think there are only two disabled:

mozilla/css/animations/basic-transition.html
mozilla/css/animations/transition-raf.html

These are enabled by removing the ini files which were disabling them. Could it be that there are more disabled in another directory?

mrobinson · 2020-05-04T15:34:30Z

@bors-servo try=wpt

Removing two expectations for newly passing tests.

bors-servo · 2020-05-04T15:34:35Z

⌛ Trying commit 8131ee3 with merge 5875d05...


        Auto merge of #26407 - mrobinson:animation-restyle-model, r=<try>

[RFC] Use a restyle for animation ticks This change corrects synchronization issues with animations, by reworking the animation processing model to do a quick restyle and incremental layout when ticking animations. While this change adds overhead to animation ticks, the idea is that this will be the fallback when synchronous behavior is required to fulfill specification requirements. In the optimistic case, many animations could be updated and applied off-the-main-thread and then resynchronized when style information is queried by script. Fixes #13865.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #13865  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

jdm · 2020-05-04T15:47:24Z

What about the animation-related test directories skipped in include.ini?

bors-servo · 2020-05-04T16:17:41Z

☀️ Test successful - status-taskcluster
State: approved= try=True

mrobinson · 2020-05-04T16:20:39Z

I wasn't sure if it was better to unskip intermittent tests later, but I'm happy to give it a shot in this change.

mrobinson · 2020-05-04T16:42:31Z

It looks like there is more required to eliminate the flakiness from animations and transitions tests. My suspicion is that this is #19242 and perhaps, in addition, making sure that transition and animation events arrive to the script thread before the next animation tick is triggered. I'll start taking a look at this tomorrow. It seems like it would be good to have this all sorted out before going too much further.

jdm · 2020-05-04T18:05:00Z

components/script/dom/node.rs

-        const CLICK_IN_PROGRESS = 1 << 2;
+        const CLICK_IN_PROGRESS = 1 << 3;

        #[doc = "Specifies whether this node is focusable and whether it is supposed \
                 to be reachable with using sequential focus navigation."]
-        const SEQUENTIALLY_FOCUSABLE = 1 << 3;
+        const SEQUENTIALLY_FOCUSABLE = 1 << 4;

-        // There are two free bits here.
+        // There is one bit free here.


Why these changes?

Whoops. This is left over from a previous version of this change where I added NodeFlags::HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS. It turns out that isn't necessary for this change, so I'll remove these changes as well.


        Use a restyle for animation ticks

This change corrects synchronization issues with animations, by reworking the animation processing model to do a quick restyle and incremental layout when ticking animations. While this change adds overhead to animation ticks, the idea is that this will be the fallback when synchronous behavior is required to fulfill specification requirements. In the optimistic case, many animations could be updated and applied off-the-main-thread and then resynchronized when style information is queried by script. Fixes #13865.

mrobinson · 2020-05-05T13:14:16Z

@bors-servo try=wpt


        Start having animations conform to the HTML spec

This is a small step toward fixing #19242. The main idea is that the clock for animations should advance as the event loop ticks. We accomplish this by moving the clock from layout and naming it the "animation timeline" which is the spec language. This should fix flakiness with animations and transitions tests where a reflow could move animations forward while script was running. This change also starts to break out transition and animation events into their own data structure, because it's quite likely that the next step in fixing #19242 is to no longer send these events through a channel.

mrobinson · 2020-05-05T18:13:12Z

@bors-servo try=wpt


        Auto merge of #26407 - mrobinson:animation-restyle-model, r=<try>

[RFC] Have animations more closely match the HTML spec These two commits do two major things: **Have animations ticks trigger a restyle**: This corrects synchronization issues with animations, where the values used in layout are out of sync with what is returned by `getComputedStyle`. **Tick the animation timer in script according to spec**: This greatly reduces the flakiness of animation and transitions tests. Fixes #13865.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #13865  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2020-05-05T18:13:18Z

⌛ Trying commit 3a74013 with merge 47ba832...

bors-servo · 2020-05-05T19:07:28Z

☀️ Test successful - status-taskcluster
State: approved= try=True

mrobinson · 2020-05-05T19:46:25Z

@bors-servo try=wpt-mac

jdm · 2020-05-05T19:52:36Z

@bors-servo retry

bors-servo · 2020-05-05T19:52:42Z

⌛ Trying commit 3a74013 with merge 473e368...


        Auto merge of #26407 - mrobinson:animation-restyle-model, r=<try>

[RFC] Have animations more closely match the HTML spec These two commits do two major things: **Have animations ticks trigger a restyle**: This corrects synchronization issues with animations, where the values used in layout are out of sync with what is returned by `getComputedStyle`. **Tick the animation timer in script according to spec**: This greatly reduces the flakiness of animation and transitions tests. Fixes #13865.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #13865  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2020-05-05T21:09:55Z

☀️ Test successful - status-taskcluster
State: approved= try=True

mrobinson · 2020-05-06T08:12:27Z

@bors-servo r=jdm

bors-servo · 2020-05-06T08:12:28Z

📌 Commit 3a74013 has been approved by jdm

bors-servo · 2020-05-06T08:12:34Z

⌛ Testing commit 3a74013 with merge b290ad9...

bors-servo · 2020-05-06T09:13:17Z

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing b290ad9 to master...

highfive assigned jdm May 4, 2020

highfive added the S-awaiting-review label May 4, 2020

highfive added the S-tests-failed label May 4, 2020

mrobinson mentioned this pull request May 4, 2020

Improvements to the CSS Transitions and Animations implementation #26328

Closed

20 of 21 tasks complete

mrobinson force-pushed the mrobinson:animation-restyle-model branch from 04d8e27 to 8131ee3 May 4, 2020

highfive removed the S-tests-failed label May 4, 2020

jdm approved these changes May 4, 2020

View changes

jdm added S-awaiting-answer and removed S-awaiting-review labels May 4, 2020

mrobinson changed the title ~~[RFC] Use a restyle for animation ticks~~ [RFC] Have animations more closely match the HTML spec May 5, 2020

mrobinson force-pushed the mrobinson:animation-restyle-model branch from 8131ee3 to 84b5f97 May 5, 2020

highfive added the S-awaiting-review label May 5, 2020

mrobinson force-pushed the mrobinson:animation-restyle-model branch from 84b5f97 to 7ff8900 May 5, 2020

highfive added the S-tests-failed label May 5, 2020

mrobinson force-pushed the mrobinson:animation-restyle-model branch from 3f3171e to 3a74013 May 5, 2020

highfive removed the S-tests-failed label May 5, 2020

jdm approved these changes May 5, 2020

View changes

mrobinson changed the title ~~[RFC] Have animations more closely match the HTML spec~~ Have animations more closely match the HTML spec May 6, 2020

highfive added S-awaiting-merge and removed S-awaiting-review S-awaiting-answer labels May 6, 2020

bors-servo merged commit b290ad9 into servo:master May 6, 2020
2 checks passed

2 checks passed

Community-TC (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label May 6, 2020

bors-servo mentioned this pull request May 6, 2020

Correctly paint the CSS canvas’ background #26414

Merged

mrobinson mentioned this pull request May 6, 2020

Hover color transition does not work with incremental layout #17667

Closed

atouchet mentioned this pull request May 6, 2020

Closed issues in source code #19685

Open

4 of 13 tasks complete

mrobinson deleted the mrobinson:animation-restyle-model branch May 6, 2020

mrobinson mentioned this pull request May 14, 2020

Is CSS transition on font-size property implemented? #12745

Closed

servo / servo

Have animations more closely match the HTML spec #26407

Have animations more closely match the HTML spec #26407

mrobinson commented May 4, 2020 •

edited

highfive commented May 4, 2020

highfive commented May 4, 2020

mrobinson commented May 4, 2020

bors-servo commented May 4, 2020

bors-servo commented May 4, 2020

jdm commented May 4, 2020

mrobinson commented May 4, 2020

mrobinson commented May 4, 2020

bors-servo commented May 4, 2020

jdm commented May 4, 2020

bors-servo commented May 4, 2020

mrobinson commented May 4, 2020

mrobinson commented May 4, 2020

This comment has been minimized.

This comment has been minimized.

mrobinson commented May 5, 2020

mrobinson commented May 5, 2020

bors-servo commented May 5, 2020

bors-servo commented May 5, 2020

mrobinson commented May 5, 2020

jdm commented May 5, 2020

bors-servo commented May 5, 2020

bors-servo commented May 5, 2020

mrobinson commented May 6, 2020

bors-servo commented May 6, 2020

bors-servo commented May 6, 2020

bors-servo commented May 6, 2020

servo / servo

Join GitHub today

Have animations more closely match the HTML spec #26407

Have animations more closely match the HTML spec #26407

Conversation

mrobinson commented May 4, 2020 • edited

highfive commented May 4, 2020

highfive commented May 4, 2020

mrobinson commented May 4, 2020

bors-servo commented May 4, 2020

bors-servo commented May 4, 2020

jdm commented May 4, 2020

mrobinson commented May 4, 2020

mrobinson commented May 4, 2020

bors-servo commented May 4, 2020

jdm commented May 4, 2020

bors-servo commented May 4, 2020

mrobinson commented May 4, 2020

mrobinson commented May 4, 2020

This comment has been minimized.

jdm May 4, 2020

This comment has been minimized.

mrobinson May 4, 2020

mrobinson commented May 5, 2020

mrobinson commented May 5, 2020

bors-servo commented May 5, 2020

bors-servo commented May 5, 2020

mrobinson commented May 5, 2020

jdm commented May 5, 2020

bors-servo commented May 5, 2020

bors-servo commented May 5, 2020

mrobinson commented May 6, 2020

bors-servo commented May 6, 2020

bors-servo commented May 6, 2020

bors-servo commented May 6, 2020

mrobinson commented May 4, 2020 •

edited