1363639 - stylo: Interpolated result of CSSRect is different from gecko.

Status: RESOLVED FIXED

Description

[Mantaroh Yoshinaga[:mantaroh]]

As mentioned bug 1353918 comment 9, current stylo's intepolation of CSSRect differ from gecko's implementation.

The gecko treat whole rect as discrete when each offset's unit type differ.

For example:
  // The result is 'rect(100px, 100px, auto, 100px)' not 'rect(50px, 50px, auto, 50px)'
  var anim = elem.animate({ clip: ['rect(0px, 0px, 0px, 0px)', 'rect(100px, 100px, auto, 100px)']}, 1000);
  anim.currentTime = 500;

If each offset's unit type is same, the gecko interpolate each offset except auto value.

For example:
  // The result is 'rect(50px, 50px, auto, 50px)'
  var anim = elem.animate({ clip: ['rect(0px, 0px, auto, 0px)', 'rect(100px, 100px, auto, 100px)']}, 1000);
  anim.currentTime = 500;

We should modify the following implementation in order to be behavior same to gecko.
 - Return None when interpolating Option<> if self or other value is None
 - Check the offset of self and other when interpolating ClipRect.
   If this offset differ, throw the error.

Comment 1

[Matt Brubeck (:mbrubeck)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0db0c93f2f29d0bd44e50d2f9490e158ca4c8f9

Comment 2

[Matt Brubeck (:mbrubeck)]

Amended to fix `compute_distance` too, and remove some redundant checks:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=278a490ec4a55766aef1fe9d96b40c8eed2baeb7

Comment 3

[Matt Brubeck (:mbrubeck)]

Added a test:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c67fc038ed3dfa81f478cad0ce70f1533eb11b4

Comment 4

[Matt Brubeck (:mbrubeck)]

Created attachment 8870613 [details]
Bug 1363639 - stylo: Make interpolation of rect() match Gecko.

Review commit: https://reviewboard.mozilla.org/r/142064/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/142064/

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 8870613 [details]
Bug 1363639 - stylo: Make interpolation of rect() match Gecko.

https://reviewboard.mozilla.org/r/142064/#review145732

::: layout/reftests/web-animations/1363639-1.html:26
(Diff revision 1)
> +          {clip: 'rect(0px, auto, 200px, 0px)'}
> +      ], 2000);
> +      anim.pause();
> +      anim.currentTime = 1000;
> +      document.documentElement.removeAttribute("class");
> +      console.log("done");

nit: remove `console.log`.

Comment 6

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8870613 [details]
Bug 1363639 - stylo: Make interpolation of rect() match Gecko.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/142064/diff/1-2/

Comment 7

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8870613 [details]
Bug 1363639 - stylo: Make interpolation of rect() match Gecko.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/142064/diff/2-3/

Comment 8

[Matt Brubeck (:mbrubeck)]

https://github.com/servo/servo/pull/17013

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/101fb6274f8c
stylo: Make interpolation of rect() match Gecko. r=emilio

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/101fb6274f8c