Animation.rs: Iterate declared transition backwards #192

yezhizhen · 2025-05-29T06:46:37Z

Implement DoubleEndedIterator for TransitionPropertyIterator to iterate declared transition backwards:
Spec: "If a property is specified multiple times in the value of transition-property (either on its own, via a shorthand that contains it, or via the all value), then the transition that starts uses the duration, delay, and timing function at the index corresponding to the last item in the value of transition-property that calls for animating that property. "

Addresses Servo issue: servo #37164
Servo PR: servo/servo#37176

Any ancestors of this commit are from upstream mozilla-central, with some filtering and renaming. Our patches and sync tooling start here. The sync tooling has all been squashed into this commit, based on: https://github.com/servo/stylo/commits/64731e10dc8ef87ef52aa2fb9f988c3b2530f3a7

This is a rebase of 03ffc85 Signed-off-by: Oriol Brufau <[email protected]>

Signed-off-by: Oriol Brufau <[email protected]>

Signed-off-by: Nico Burns <[email protected]>

Signed-off-by: Oriol Brufau <[email protected]>

…ervo#179) Signed-off-by: Martin Robinson <[email protected]>

Servo is currently rewriting the incremental layout system. Many `RestyleDamage` variants only applied to the old layout system, so only keep the variants that Servo will need for the first part of incremental layout. Signed-off-by: Martin Robinson <[email protected]> Co-authored-by: Oriol Brufau <[email protected]>

) When a property has no restyle damage specified, to ensure proper behavior, it's better to be conservative and do a full reflow. That way, the addition of the `servo_restyle_damage` setting can improve the performacne of layout, but its absence can never break layout. Signed-off-by: Martin Robinson <[email protected]> Co-authored-by: Oriol Brufau <[email protected]>

- Make changes to `clip-path` no longer cause repaint-only restyle damage, as we are splitting out stacking context tree building from display list building and `clip-path` can cause stacking context tree changes. - Changes to `pointer-events` can be processed during display list building and are thus repaint only. Signed-off-by: Martin Robinson <[email protected]> Co-authored-by: Oriol Brufau <[email protected]>

This restyle damage is used in cases where we need to reconstruct the stacking contexts, because a style change has caused a box to establish or stop establishing a stacking context, but it's unnecessary to rebuild the box tree. Additionally, `ServoRestyleDamage::REBUILD` is renamed to `REBUILD_BOX`, and old `servo_restyle_damage` values are removed in favor of it. Signed-off-by: Oriol Brufau <[email protected]> Co-authored-by: Martin Robinson <[email protected]>

…o#191) In many cases, changes to filters do not require rebuilding the box tree and laying out again. This change uses REPAINT restyle damage in these cases, except when changing between no filter and a filter. In those cases we need to rebuild the box tree. Signed-off-by: Oriol Brufau <[email protected]> Co-authored-by: Martin Robinson <[email protected]>

Signed-off-by: Euclid Ye <[email protected]>

{"fail_fast": false, "matrix": [{"name": "Linux (WPT)", "workflow": "linux", "wpt": true, "profile": "release", "unit_tests": false, "build_libservo": false, "bencher": false, "build_args": "", "wpt_args": "", "number_of_wpt_chunks": 20}]}

mrobinson · 2025-05-29T07:33:54Z

Please explain a bit more what is going on here in the PR description. I know this code and I'm still having trouble understanding how this change fixes the issue. This should be both in the PR description and in the comment that you've added to the code. This is so that people reading the code in the future know why you are reversing this iterator.

Here is example 3 from the specification:

div {
  transition-property: opacity, left, top, width;
  transition-duration: 2s, 1s;
}
The above example defines a transition on the opacity property of 2 seconds duration, a transition on the left property of 1 second duration, a transition on the top property of 2 seconds duration and a transition on the width property of 1 second duration.

Why does this require stepping through the properties in reverse order?

mrobinson · 2025-05-29T07:54:16Z

style/animation.rs

+    // Reverse is needed
+    // See Example 3 of <https://www.w3.org/TR/css-transitions-1/#transitions>
+    let declared_transitions = {
+        let mut temp = new_style.transition_properties().collect::<Vec<_>>();
+        temp.reverse();
+        temp
+    };


I think this allocation is a bit unfortunate. It would be better to implement https://doc.rust-lang.org/std/iter/trait.DoubleEndedIterator.html for TransitionPropertyIterator. This should be possible since TransitionPropertyIterator is implemented using Range and Range is also implements DoubleEndedIterator.

I thought about the same when I draft this PR, to have DoubleEndedIterator. But this part also affects Firefox? I am not familiar with the syncing procedure.

I implemented this in a way that won't affect Gecko.

yezhizhen · 2025-05-29T08:03:00Z

Please explain a bit more what is going on here in the PR description. I know this code and I'm still having trouble understanding how this change fixes the issue. This should be both in the PR description and in the comment that you've added to the code. This is so that people reading the code in the future know why you are reversing this iterator.

Here is example 3 from the specification:
div {
  transition-property: opacity, left, top, width;
  transition-duration: 2s, 1s;
}
The above example defines a transition on the opacity property of 2 seconds duration, a transition on the left property of 1 second duration, a transition on the top property of 2 seconds duration and a transition on the width property of 1 second duration.
Why does this require stepping through the properties in reverse order?

For the detail, check
servo/servo#37164 (comment)

But I guess I added more details than needed.. Actually the following thing is the direct reason for this change: (already added to latest code)
"If a property is specified multiple times in the value of transition-property (either on its own, via a shorthand that contains it, or via the all value), then the transition that starts uses the duration, delay, and timing function at the index corresponding to the last item in the value of transition-property that calls for animating that property."

Signed-off-by: Euclid Ye <[email protected]>

{"fail_fast": false, "matrix": [{"name": "Linux (WPT)", "workflow": "linux", "wpt": true, "profile": "release", "unit_tests": false, "build_libservo": false, "bencher": false, "build_args": "", "wpt_args": "", "number_of_wpt_chunks": 60}]}

Signed-off-by: Euclid Ye <[email protected]>

yezhizhen · 2025-06-04T04:46:24Z

@mrobinson I know you might be busy with Web Engines Hackfest.
But is there any rough timeline to review this? :)

delan and others added 14 commits May 4, 2025 06:35

Commit our changes on top of upstream Stylo

2bc1e51

This is a rebase of 03ffc85 Signed-off-by: Oriol Brufau <[email protected]>

Fixup for https://phabricator.services.mozilla.com/D241392

d7161fc

Signed-off-by: Oriol Brufau <[email protected]>

Fixup for https://phabricator.services.mozilla.com/D244537

7e710ba

Signed-off-by: Oriol Brufau <[email protected]>

Fixup for https://phabricator.services.mozilla.com/D246780

6bbb671

Signed-off-by: Oriol Brufau <[email protected]>

Bump stylo_* version to 0.3.0

d5c8949

Signed-off-by: Nico Burns <[email protected]>

Bump selectors version to 0.28.0

7edd19e

Signed-off-by: Oriol Brufau <[email protected]>

Add release information to README and make sure it's used for crates (s…

1f2c9b1

…ervo#179) Signed-off-by: Martin Robinson <[email protected]>

Make more CSS properties only require a repaint for Servo (servo#183)

fb74b95

yezhizhen added a commit to yezhizhen/servo that referenced this pull request May 29, 2025

Bump Stylo to servo/stylo#192

a7b8cd9

Signed-off-by: Euclid Ye <[email protected]>

yezhizhen added a commit to yezhizhen/servo that referenced this pull request May 29, 2025

Bump Stylo to servo/stylo#192

fab9241

Signed-off-by: Euclid Ye <[email protected]>

yezhizhen mentioned this pull request May 29, 2025

Bump Stylo to servo/stylo#192 to fix more Transition issue servo/servo#37176

Open

yezhizhen added a commit to yezhizhen/servo that referenced this pull request May 29, 2025

Bump Stylo to servo/stylo#192

cba2616

Signed-off-by: Euclid Ye <[email protected]>

mrobinson requested changes May 29, 2025

View reviewed changes

yezhizhen requested a review from mrobinson May 30, 2025 06:52

yezhizhen added a commit to yezhizhen/servo that referenced this pull request May 30, 2025

Modify Cargo to use latest servo/stylo#192

beeecde

Signed-off-by: Euclid Ye <[email protected]>

Animation.rs: Iterate declared transition backwards

95b4609

Signed-off-by: Euclid Ye <[email protected]>

yezhizhen force-pushed the fix-transition-order branch from 7a1f9bb to b4a89c8 Compare May 30, 2025 07:07

yezhizhen added a commit to yezhizhen/servo that referenced this pull request May 30, 2025

Bump Stylo to servo/stylo#192

7a0a0ca

Signed-off-by: Euclid Ye <[email protected]>

yezhizhen added a commit to yezhizhen/servo that referenced this pull request May 30, 2025

Modify Cargo to use latest servo/stylo#192

cbb8ac5

Signed-off-by: Euclid Ye <[email protected]>

yezhizhen added a commit to yezhizhen/servo that referenced this pull request May 30, 2025

Modify Cargo to use latest servo/stylo#192

0e4bfde

Signed-off-by: Euclid Ye <[email protected]>

Implement DoubleEndedIterator for TransitionPropertyIterator

9447de4

Signed-off-by: Euclid Ye <[email protected]>

yezhizhen force-pushed the fix-transition-order branch from b4a89c8 to 9447de4 Compare May 30, 2025 07:50

yezhizhen added a commit to yezhizhen/servo that referenced this pull request May 30, 2025

Bump Stylo to servo/stylo#192

5078ff0

Signed-off-by: Euclid Ye <[email protected]>

yezhizhen mentioned this pull request Jun 6, 2025

servoshell: Do not focus and raise new auxiliary WebDriver-created WebViews servo/servo#37284

Merged

mrego mentioned this pull request Jun 9, 2025

Servo Coordination Meeting 2025/06/09 servo/project#149

Closed

Loirooriol force-pushed the main branch from 7fdb9ef to 2e1f66b Compare June 19, 2025 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Animation.rs: Iterate declared transition backwards #192

Animation.rs: Iterate declared transition backwards #192

Uh oh!

yezhizhen commented May 29, 2025 •

edited

Loading

Uh oh!

mrobinson commented May 29, 2025 •

edited

Loading

Uh oh!

mrobinson May 29, 2025 •

edited

Loading

Uh oh!

yezhizhen May 29, 2025 •

edited

Loading

Uh oh!

yezhizhen Jun 2, 2025

Uh oh!

yezhizhen commented May 29, 2025 •

edited

Loading

Uh oh!

yezhizhen commented Jun 4, 2025

Uh oh!

Uh oh!

Animation.rs: Iterate declared transition backwards #192

Are you sure you want to change the base?

Animation.rs: Iterate declared transition backwards #192

Uh oh!

Conversation

yezhizhen commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrobinson commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrobinson May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yezhizhen May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yezhizhen Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

yezhizhen commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yezhizhen commented Jun 4, 2025

Uh oh!

Uh oh!

yezhizhen commented May 29, 2025 •

edited

Loading

mrobinson commented May 29, 2025 •

edited

Loading

mrobinson May 29, 2025 •

edited

Loading

yezhizhen May 29, 2025 •

edited

Loading

yezhizhen commented May 29, 2025 •

edited

Loading