[css-transforms-1] Correct translation calculation in recomposition of decomposed transform

[jlfwong]

As far as I can tell, this calculation for recomposing the transform is incorrect.

If you apply the given pseudocode description of decomposing and recomposing to the following matrix:

[ 1 1 10
  0 1 20 ]

(where (t_x, t_y) = (10, 20))

Then you'd expect to get back the same matrix (decompose & recompose should be inverse functions).

However, if you follow the specification as given, then I think you get back the following instead:

[ 1 1 14.14
  0 1 24.14 ]

More generally, you can see that the spec is wrong from inspection.

The decomposition logic does the following:

translate[0] = matrix[3][0]
translate[1] = matrix[3][1]

And nothing changes these translate values after-the-fact.

The recomposition logic (before this PR) then does:

matrix[3][0] = translate[0] * m11 + translate[1] * m21
matrix[3][1] = translate[0] * m12 + translate[1] * m22

Where mNN are as specified in the decomposition logic.

Which, by substitution, could alternatively be stated as:

matrix[3][0] = matrix[3][0] * m11 + matrix[3][1] * m21
matrix[3][1] = matrix[3][0] * m12 + matrix[3][1] * m22

Which I'm reasonably confident is incorrect.

As far as I can tell, browsers' implementations are more or less correct, but I suspect are correct by not following this specification.

See: https://codepen.io/jlfwong/pen/ZEEedwN. If they used the specified interpolation logic, there would be a sudden change in translation at some point during the transition. Note that the above codepen seems to behave the same in Chrome & Firefox, but differently in Safari.

[monfera]

@jlfwong nice find!

The non-compliance could be reported to the browser vendors to see what happens 😄

Also, indeed it looks different on Safari vs the others, easiest to see as the left edge moves on the top vs bottom. Wondering which browsers are even compliant with the intent of the spec, likely they can't be both correct, even if we ignore the alleged spec bug.

Out of curiosity, have you tried or eyeballed the equivalent thing with matrix3d?

(Adding the spec section for reference)

[tabatkins]

@smfr, @chrishtr, @emilio, what's right here?

[emilio]

I think this is right, and this is a bug in the spec.

Firefox code is here for decomposition, and here for recomposition.

I found some other bugs in the 3d algorithm when dealing with compat issues (#2713 for example).

But yeah, I think the spec as written doesn't preserve the matrix across decomposition and recomposition, which is clearly a bug.

cc @dirkschulze