Implement steps(jump-*) timing functions and drop frames() timing function

RESOLVED FIXED in Firefox 65

Status

()

enhancement
P3
normal
RESOLVED FIXED
8 months ago
a month ago

People

(Reporter: birtles, Assigned: boris, NeedInfo)

Tracking

({dev-doc-complete})

Trunk
mozilla65
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox64 wontfix, firefox65 fixed)

Details

Attachments

(8 attachments)

(Reporter)

Description

8 months ago
The CSS Timing spec has finally been updated to include the replacement to the previously specced frames() timing function:

  https://drafts.csswg.org/css-timing-1/#step-timing-functions

Basically it consists of four new step position keywords:

  jump-start
  jump-end
  jump-none
  jump-both

The first two are mostly aliases but at least 'jump-start' will need to be preserved as a separate keyword to 'start' so that they serialize differently. Both 'jump-end' and 'end' are not serialized so we don't need to preserve that distinction.

We should drop our implementation of frames() and add these keywords instead.
(Assignee)

Comment 1

8 months ago
Maybe I have time to work on this, given my other bugs have some spec issues.
(Assignee)

Updated

8 months ago
Assignee: nobody → boris.chiou
(Reporter)

Comment 2

7 months ago
Note that the CSS Timing Functions spec has just been renamed to CSS Easing Functions so if there are links to css-timing in tests and so on, it might be worthwhile to update them to css-easing while we're touching those files.

  https://drafts.csswg.org/css-easing/
(Assignee)

Comment 3

7 months ago
(In reply to Brian Birtles (:birtles) from comment #2)
> Note that the CSS Timing Functions spec has just been renamed to CSS Easing
> Functions so if there are links to css-timing in tests and so on, it might
> be worthwhile to update them to css-easing while we're touching those files.
> 
>   https://drafts.csswg.org/css-easing/

One of my patch will rename this. Besides, I'm start to think is it possible to drop/simplify the conversion between computed::TimingFunction and nsTimingFunction (i.e. gecko_bindings/sugar/ns_timing_function.rs)? Maybe we could do that after [1] is merged into cbindgen.

[1] https://github.com/eqrion/cbindgen/pull/219
Keywords: dev-doc-needed
(Assignee)

Updated

7 months ago
Keywords: dev-doc-needed
(Assignee)

Comment 4

7 months ago
frames() timing function was removed from the spec, so we drop it.
Besides, some devtool tests are removed because they use frame(). I will
add them back by using new step function later.
(Assignee)

Comment 5

7 months ago
Rename css-timing to css-easing according to the spec update.

Depends on D9309
(Assignee)

Comment 6

7 months ago
TimingFunction is defined in a separate spec (i.e. css-easing), instead
of transform, so we move it into a different file.

Depends on D9310
(Assignee)

Comment 7

7 months ago
So we can generate generic enum by cbindgen (for TimingFunction).

Depends on D9311
(Assignee)

Comment 8

7 months ago
First, we generate StyleComputedTimingFunction by cbindgen from Rust, and use
it in nsTimingFunction, so we could copy it directly without handling
the different memory layout. However, we have to rewrite the
nsTimingFunction and mozilla::ComputedTimingFunction for this.

Second, the rust-bindgen seems cannot generate the correct generic members
from complex C++ templates, especially for the nested template struct:
e.g.

template<A, B>
union Foo {
  struct Bar1 {
    A mA;
  };
  struct Bar2 {
    B mB;
  };

  Bar1 bar1;
  Bar2 bar2;
};

The generated rust code may look like:

pub struct Foo<A, B> {
  // Foo_Bar1_Body should have one template parameter.
  pub bar1: root::__BindgenUnionField<root::Foo_Bar1_Body>,
  // Foo_Bar2_Body should have one template parameter.
  pub bar2: root::__BindgenUnionField<root::Foo_Bar2_Body>,
}
pub struct Foo_Bar1_Body<A> { ... }
pub struct Foo_Bar2_Body<B> { ... }

So we have to hide StyleTimingFunction to avoid the compilation errors.

Depends on D9312
(Assignee)

Comment 9

7 months ago
1. Add a new preference, layout.css.step-position-jump.enabled, for
   step(_, jump-*) timing functions.
2. We still keep Start and End tags, even though there is no difference
   between them. Therefore, we could disable the preference if needed.
3. Update the calculation of StepTiming to match the algorithm in the spec.

Depends on D9313
(Assignee)

Comment 10

7 months ago
Add new mochitests, and web-platform tests, and devtool tests.

Depends on D9314
(Assignee)

Updated

7 months ago
Status: NEW → ASSIGNED
(Assignee)

Comment 11

7 months ago
We make sure the step number is always positive, so using
computed::Integer is safe and can derive ToComputedValue.

Depends on D9311
Attachment #9018748 - Attachment description: Bug 1496619 - Part 4: Bump cbindgen to 0.6.6 → Bug 1496619 - Part 5: Bump cbindgen to 0.6.6
Attachment #9018749 - Attachment description: Bug 1496619 - part 5: Generate StyleTimingFunction and drop ns_timing_function.rs → Bug 1496619 - part 6: Generate StyleTimingFunction and drop ns_timing_function.rs
Attachment #9018750 - Attachment description: Bug 1496619 - part 6: Implement steps(jump-*) functions → Bug 1496619 - part 7: Implement steps(jump-*) functions
Attachment #9018751 - Attachment description: Bug 1496619 - part 7: Tests → Bug 1496619 - part 8: Tests

Comment 14

7 months ago
Pushed by bchiou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b46aa2883e42
Part 1: Drop frames() timing function r=birtles
https://hg.mozilla.org/integration/autoland/rev/e9bf605e24a2
Part 2: Rename css-timing to css-easing r=birtles
https://hg.mozilla.org/integration/autoland/rev/dca7e62232c1
Part 3: Split TimingFunction into a separate file to match spec r=emilio
https://hg.mozilla.org/integration/autoland/rev/d6e5970c62be
Part 4: Replace u32 with computed::Integer for computed::TimingFunction r=emilio
https://hg.mozilla.org/integration/autoland/rev/5b744b960fe0
Part 5: Bump cbindgen to 0.6.6 r=emilio
https://hg.mozilla.org/integration/autoland/rev/79675a8112d6
part 6: Generate StyleTimingFunction and drop ns_timing_function.rs r=emilio,birtles
https://hg.mozilla.org/integration/autoland/rev/77d9e79ed3b9
part 7: Implement steps(jump-*) functions r=birtles
https://hg.mozilla.org/integration/autoland/rev/24936aee7543
part 8: Tests r=birtles

Comment 16

7 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd47bdbd4efa
Update min cbindgen version in osx.py.

Comment 17

7 months ago
Pushed by rgurzau@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/6771e96bb376
Update min cbindgen version in osx.py. a=emilio
(Assignee)

Comment 18

7 months ago
(In reply to Pulsebot from comment #16)
> Pushed by emilio@crisal.io:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/cd47bdbd4efa
> Update min cbindgen version in osx.py

Thanks for this!
(Reporter)

Updated

7 months ago
Depends on: 1502788
(Reporter)

Comment 20

7 months ago
Looks like the test changes didn't get upstreamed. Filed bug 1502788 for that.
Flags: in-testsuite+
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13868 for changes under testing/web-platform/tests
Upstream PR merged
See Also: → 1504602
Note to MDN writers:

I have added a note to the Fx65 rel notes to cover this (two actually):
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/65#CSS

In terms of the documentation, there are a number of places this is mentioned that will need to be updated to remove mention of frames(), and add in description of the new keywords and how they are used:
https://developer.mozilla.org/en-US/docs/Web/CSS/transition-timing-function
https://developer.mozilla.org/en-US/docs/Web/CSS/animation-timing-function
https://developer.mozilla.org/en-US/docs/Web/CSS/single-transition-timing-function
https://developer.mozilla.org/en-US/docs/Web/CSS/animation
https://developer.mozilla.org/en-US/docs/Web/CSS/transition

Syntax in the MDN data repo and browser compat data will also need updating.

For the spec definitions, see:
https://drafts.csswg.org/css-easing-1/#step-timing-functions

Comment 24

4 months ago

https://github.com/mdn/sprints/issues/800 lists most of the tasks and PRs for dev-doc-complete. Updates included updating BCD, syntax, and examples for:

it will take a bit before the csssyntax and bcd are reflected directly on MDN

(Reporter)

Comment 25

4 months ago

(In reply to Estelle Weyl from comment #24)

https://github.com/mdn/sprints/issues/800 lists most of the tasks and PRs for dev-doc-complete. Updates included updating BCD, syntax, and examples for:

it will take a bit before the csssyntax and bcd are reflected directly on MDN

Thanks for doing this.

Would you mind updating https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/easing too?

Flags: needinfo?(eweyl)

Comment 26

a month ago

Steps has been added to https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/easing too. Was done a bit ago - and BCD is live.

(Reporter)

Comment 27

a month ago

(In reply to Estelle Weyl from comment #26)

Steps has been added to https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/easing too. Was done a bit ago - and BCD is live.

That page doesn't cover the new jump keywords however (e.g. jump-both etc.). I mean, the diagram does, but the prose doesn't.

You need to log in before you can comment on or make changes to this bug.