[RFC] Decompose matrices individually to allow rotate(0deg) -> rotate(360deg) after a matrix

[joeytwiddle]

Currently in transformType.interpolate() the presence of a matrix forces itself and all later transforms to be decomposed into their components for interpolation. But decomposing rotate(0deg) and rotate(360deg) into matrices breaks the interpolation of the angle value. (Because both of those rotations decompose to a rotation of 0!)

This patch attempts to fix this: when a matrix (or incompatible types) are encountered, we only perform decomposition at that stage of the composition. Later interpolations can be performed without decomposition into matrices.

I added a test case auto-test-composite-transforms-matrix-rotate.html (a slight modification of auto-test-composite-transforms.html).

dt.play(new Animation(divs[0], [
  {transform: "matrix(1,0,0,1,0,0) rotate(0deg)"},
  {transform: "matrix(1,0,0,1,200,0) rotate(360deg)"},
], timing));

With the old behaviour the first two divs do not rotate at all. With the new behaviour you will see them perform a full rotation.

Concerns:

• It looks like from.decompositionPair = to; and from.decomposition were assigned as a way of caching the decomposed matrix, but I did not reproduce caching in this patch.

Alternatives:

• I thought a workaround might be to specify additional keyframes with intermediate angles, e.g. 0deg, 120deg, 240deg and 360deg but the decomposition still causes 240 to rotate anti-clockwise to "360" (because it was decomposed to 0).

[shans]

I think that this patch has the correct behavior for the example you've provided:

dt.play(new Animation(divs[0], [
  {transform: 'matrix(1,0,0,1,0,0) rotate(0deg)'},
  {transform: 'matrix(1,0,0,1,200,0) rotate(360deg)'},
], timing));

but incorrect behavior for cases such as:

dt.play(new Animation(divs[0], [
  {transform: 'rotate(45deg) rotate(0deg)'},
  {transform: 'translate(100px) rotate(360deg)'},
], timing));

This is because the former case is an example of matched transform components, which should interpolate component-wise, but the latter is an example of mismatched transform components, which should fall back on matrix interpolation from the first mismatch.

[joeytwiddle]

Edit: "which should fall back on matrix interpolation from the first mismatch" So asymmetry _is_ part of the spec. I think this answers my striked question below.

<verbose overly=true>My patch would interpolate the rotate(45deg) with the translate(100px) and then move on to interpolate the next pair.

The original behaviour would (due to the mismatch) collapse the rotate(45deg) rotate(0deg) into a matrix, and likewise for the other keyframe, then interpolate the decomposed matrices.

Which do you think is the correct behaviour?

My concern is that with the original behaviour, this would act "asymmetrically":

dt.play(new Animation(divs[0], [
  {transform: 'rotate(90deg)  rotate(45deg)    rotate(0deg)'},
  {transform: 'rotate(180deg) translate(100px) rotate(360deg)'},
], timing));

The first pair rotate(90deg) to rotate(180deg) would be interpolated, but the last pair rotate(0deg) to rotate(360deg) would not be interpolated. They would get collapsed "into" the second pair due to the mismatch. I just want to check that asymmetry is the expected behaviour.

(Thanks for checking! Certainly I agree the patch needs more work ... as noted in #619 I still find strange behaviour with an initial matrix animation.)

[joeytwiddle]

But then there is a remaining issue: currently the fall back is also used when the two components match but are both matrices. In that situation, shouldn't we interpolate the matrices, then move on to the next pair?

In other words, I believe the following shouldn't fall back. But currently it does.

dt.play(new Animation(divs[0], [
  {transform: 'matrix(1,0,0,1,0,0)  rotate(0deg)  '},
  {transform: 'matrix(0,1,-1,0,0,0) rotate(360deg)'},
], timing));

I see you have already answered in agreement: "I think that this patch has the correct behavior for the example you've provided"</verbose>

So I will try to fix this in a different branch...!