1500820 - Interpolation of rotate(0deg) and rotateX(360deg) fails

Status: RESOLVED FIXED

(Core :: CSS Transitions and Animations, defect, P3)

Description

[Karl Dubost💡 :karlcow]

This is a spin-off of https://webcompat.com/issues/20078

1. Go to  https://thimbleprojects.org/mozillalearning/213517/
2. Hover the hat.

Actual:
at the top the hat disappears

Expected:
The hat rotates when at the top like it does in Chrome.


CSS is defined with 

.fox:hover .hat {
  animation-name: tip;                /* animation name */
  animation-duration: 2s;             /* how long it lasts */
  animation-delay: 0s;                /* dely before starting */
  animation-iteration-count: 3;       /* how many times to play it */
  animation-timing-function: linear;  /* easing style */
  animation-fill-mode: backwards;     /* apply beginning or end styles when it's over */
  animation-direction: normal;        /* direction it plays in */
}

.hat {
  transform-origin: center;     /* trasformations are applied from these cooridnates on the hat */
}

/* 
These are the keyframes in the hat tip animation.
Each keyframe includes a percentage and any number of css rules.
*/

@keyframes tip {
  0% {
    /* You can add more rules to each keyframe */
  }
  25% {
    transform: translateY(-100px) rotate(0deg);
  }
  50% {
    transform: translateY(-100px) rotateX(360deg);
  }
  100% {
    transform: translateY(0px) rotate(360deg);
  }
}

/* You can

Comment 1

[Brian Birtles (:birtles)]

I was concerned that I broke this last week but I can see Firefox 63 has the same problem.

Looks like while we're interpolating between 'translateY(-100px) rotate(0deg)' and 'translateY(-100px) rotateX(360deg)' we end up with a computed style of:

  "matrix3d(NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, 0, -100, 0, 1)"

I'll look into this later today.

Comment 2

[Brian Birtles (:birtles)]

Reproduced with Firefox 62 as well (and the original report was from Firefox 62 too) so if this is a regression, it's not a recent one.

Comment 3

[Brian Birtles (:birtles)]

From bug 1499862 comment #13:
> 3. When interpolation quaternions the spec has:
> 
>     if (product == 1.0)
>       quaternionDst = quaternionA
>       return
> 
>    But this fails to account for when product is -1 (which will cause divide
>    by zero). It should probably use abs().
> 
>    Servo also fails to account for this.

Wow, this bug I identified last Friday is precisely this bug -- that is, it's the cause of the NaN values, but not the cause of the failure to flip.

Comment 4

[Brian Birtles (:birtles)]

Looks like we fail to interpolate between Quaternion(0.0, 0.0, 0.0, 1.0) and Quaternion(0.00000000000000012246467991473532, 0.0, 0.0, -1.0).

We could either do something a little smarter than just call to_rotate_3d (and actually turn `rotate(0deg)` here into `rotateX(0deg)`) or I could actually try to understand quaternions properly. I guess I should do that latter.

Comment 5

[Brian Birtles (:birtles)]

Chromium will treat these two values as having a common axis since the angle of the first is zero:

  https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/transforms/rotation.cc?l=67&rcl=d89a726271a6b6934ec72510c2da3f4495729e5b

So, basically, it does the "do something a little smarter" approach.

Comment 6

[Brian Birtles (:birtles)]

Strictly speaking, the spec doesn't allow this[1] but it seems like the most useful behavior for the author so I think we should follow suit and file a spec issue for this.

[1] https://drafts.csswg.org/css-transforms-2/#interpolation-of-transform-functions

Comment 7

[Brian Birtles (:birtles)]

I notice Edge and Safari fail to interpolate this.

Comment 8

[Brian Birtles (:birtles)]

I will likely split this into two bugs:

1) (This bug) A simple patch to just handle the dotproduct == -1.0 case (match Edge and Safari, can be uplifted to beta)
2) A separate bug to do the smarter handling of zero rotation angles (needs spec change, no hurry to uplift)

Comment 9

[Brian Birtles (:birtles)]

Spec PR: https://github.com/w3c/csswg-drafts/pull/3235

Comment 10

[Brian Birtles (:birtles)]

Attachment: Bug 1500820 - Compare absolute dot-product to 1.0 when interpolating quaternions; r?hiro

See the extended commit message for the following spec change:

  https://github.com/w3c/csswg-drafts/commit/6b36d41ebc2e790c86cc768a6ea1dcfa81fffe75

Basically, by failing to take the absolute value, for certain content we can end
up doing division by zero which will mean that the test included in this patch
will cause an assertion to fail in debug builds and return
"matrix(NaN, NaN....)" in release builds.

Comment 11

[Brian Birtles (:birtles)]

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ce248abad81
Compare absolute dot-product to 1.0 when interpolating quaternions; r=hiro

Comment 13

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13704 for changes under testing/web-platform/tests

Comment 14

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/0ce248abad81

Comment 15

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged