977311 - animation of transform: perspective(constant) rotateY(changing) rotates the long way around when reversed in the middle of the transition

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Vincze Márton]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20140212131424

Steps to reproduce:

I have transformed an element by rotating it: transform: perspective(800px) rotateY(300deg);

I have set up an 1 sec transition on it: transition: transform 1s;

I have attached a class to it with: transform: perspective(800px) rotateY(180deg);

I toggle that class either by hand, or by setting a timeout on it: http://jsfiddle.net/6VmGU/7/

Please compare the animation in ff and chrome/ie/opera.


Actual results:

If I remove the class in the middle of the transition, firefox does a more than 240 degrees turn. Between 300deg and 180deg.


Expected results:

In like any other browsers I have tried (chrome, IE11, opera) removing the class in the middle of the transition reverses the animation direction.

The same transition between transform: perspective(800px) rotateY(120deg);
and transform: perspective(800px) rotateY(0deg); works as expected in firefox too.

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: reporter's testcase

from http://jsfiddle.net/6VmGU/7/ with URL's absolutized where needed; not minimized

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: reporter's testcase

same, except with the https jQuery link, and with the http style sheet link removed

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

My guess would be that this code:
http://hg.mozilla.org/mozilla-central/file/fcd802ee7945/layout/style/nsStyleAnimation.cpp#l2309
is somehow falling back into the "transform lists that don't match" codepath.

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I believe this is actually a bug in the CSS Transforms spec.

The spec says in http://dev.w3.org/csswg/css-transforms/#interpolation-of-transforms :

  If from- and to-transform have the same number of transform 
  functions, each transform function pair has either the same
  name, or is a derivative of the same primitive.

Then in http://dev.w3.org/csswg/css-transforms/#interpolation-of-transform-functions it says :

  The transform functions matrix(), matrix3d() and perspective()
  get converted into 4x4 matrices first and interpolated as
  defined in section Interpolation of Matrices afterwards.

This means that if you interpolate two lists that consist of:
  perspective() rotateY()
  perspective() rotateY()
you get an interpolation result that is:
  matrix3D() rotateY()

When you try to interpolate that result with another value of the form:
  perspective() rotateY()
we fall back to the "Lists don't match" path.

(That said, our code would still fall back even if that final value were matrix3D() rotateY(), since we use a special interpolatematrix() that we probably should match, but fail to.)



(If you want to work around the bug, the simplest fix would probably be to pull the perspective out into the 'perspective' property on the parent element.)

Comment 5

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: debugging patch

On the testcase, this yielded results such as:

Comparing serialization:
  perspective(800px) rotateY(180deg)
  perspective(800px) rotateY(300deg)
  match=true

Comparing serialization:
  interpolatematrix(perspective(800px), perspective(800px), 97.0987%) rotateY(5.17522rad)
  perspective(800px) rotateY(180deg)
  match=false

Comment 6

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Posted http://lists.w3.org/Archives/Public/public-fx/2014JanMar/0076.html

Comment 7

[Vincze Márton]

Hi,

is anything in progress regarding this? Some of my older animations look really wonky in ff because of this and I'm considering rewriting them or suggesting an other browser for the viewer.

I was originally planning on waiting for the ff team to fix this, but it seems that this issue has fallen into the black hole called W3.org mailing lists.

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I guess we should just treat matrix(), matrix3d(), and perspective() as identical functions for the purposes of determining if lists match.  That seems to be the simplest sensible fix to the spec.

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

... which requires changing TransformFunctionsMatch(), and making sure AddTransformLists() can deal with the change.  (in layout/style/StyleAnimationValue.cpp.)

(It might be easier not to change ToPrimitive(), although I don't really remember why interpolatematrix() works the way it does and tries to preserve lists so long.  Making ToPrimitive() convert things might involve breaking whatever required that.)

We'd also need to include interpolatematrix in the set of things to be equal.

Comment 10

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Oh, the reason we need interpolatematrix (from bug 505115) was to deal with percentages, since we're interpolating without knowing what the percentage basis is.

Comment 11

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Untested patch at:
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/bb1b0bd0ebb4/matching-transform-functions

Comment 12

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(This doesn't quite handle the unmatched-rotate3d() case.  I need to look at the code a little more closely for that one.)

Comment 13

[Kenan]

This bug was fixed at 2022-03-23
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:105.0) Gecko/20100101 Firefox/105.0

Now the animation is the same as on Chrome.

Mozregression:

2022-08-13T15:13:28.610000: DEBUG : Found commit message:
Bug 1760918 - Disable partial pre-render animations on all channels. r=boris

The reason why we change the OMTAdiv's width value is that with the original
200px width, the elment in question will be overflowed in our mochitest iframe.

Differential Revision: https://phabricator.services.mozilla.com/D141812

2022-08-13T15:13:28.610000: DEBUG : Did not find a branch, checking all integration branches
2022-08-13T15:13:28.610000: INFO : The bisection is done.
2022-08-13T15:13:28.610000: INFO : Stopped