1365851 - stylo: Interpolate mismatched transform lists

Status: RESOLVED DUPLICATE of bug 1335998

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Brian Birtles (:birtles)]

Attachment: Test case

Servo doesn't support interpolating mismatched transform lists[1] so the attached test case won't produce the correct result.

[1] http://searchfox.org/mozilla-central/rev/f55349994fdac101d121b11dac769f3f17fbec4b/servo/components/style/properties/helpers/animated_properties.mako.rs#1692

Comment 1

[Boris Chiou [:boris]]

Not sure this intermittent [1] is related to this bug. table-overflowed-by-animation.html uses mismatched transform lists [2].

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1358266
[2] http://searchfox.org/mozilla-central/source/layout/reftests/transform/table-overflowed-by-animation.html

Comment 2

[Boris Chiou [:boris]]

(In reply to Boris Chiou [:boris] from comment #1)
> Not sure this intermittent [1] is related to this bug.
> table-overflowed-by-animation.html uses mismatched transform lists [2].
> 
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1358266
> [2]
> http://searchfox.org/mozilla-central/source/layout/reftests/transform/table-
> overflowed-by-animation.html

Sometimes, I see this on stylo try.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

(In reply to Boris Chiou [:boris] from comment #1)
> Not sure this intermittent [1] is related to this bug.
> table-overflowed-by-animation.html uses mismatched transform lists [2].

Oh, I just realized this comment. Actually I read before but forgot it completely.
I think we should disable the test until this bug is fixed.

Comment 4

[Brian Birtles (:birtles)]

Now that we're starting to test real Web content more often, this bug seems to be coming up a lot. This would be a great bug to tackle in the next week or so (and could be done entirely on the Servo side by a volunteer/new contributor even, provided they don't take too long).

Comment 5

[Brian Birtles (:birtles)]

Corresponding servo issue: https://github.com/servo/servo/issues/13267

Comment 6

[Boris Chiou [:boris]]

Maybe we should add a special arm which represents eCSSKeyword_interpolatematrix for Servo, and we can just pass this type to Gecko, so when Gecko sees this type in nsDisplayList.cpp, it can read the transform matrix because we already know the frame info and so process the interpolation.

For the github issues, "percent and length for translate", I guess we should replace
  ComputedOperation::Translate(computed::LengthOrPercentage, ...)
with
  ComputedOperation::Translate(computed::CalcLengthOrPercentage, ...),
so we can also pass this calc type into Gecko, and nsDisplayList can handle it [1].

BUT this solution only fix this for "stylo".

[1] http://searchfox.org/mozilla-central/rev/35b37316149108d53a02fb8498e250ea8a22ab5d/layout/style/nsStyleTransformMatrix.cpp#137

Comment 7

[Hiroyuki Ikezoe (:hiro)]

I did not think initially this bug means the servo issue of 13267.   We have already bug 1335998 for it.  Anyway we will need it eventually though.

Comment 8

[Boris Chiou [:boris]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #7)
> I did not think initially this bug means the servo issue of 13267.   We have
> already bug 1335998 for it.  Anyway we will need it eventually though.

I guess this is the same as bug 1335998, right? We use interpolatematrix to handle mismatched transform lists. (i.e. delay the interpolation of two mismatched transform lists until nsDisplayList)

Comment 10

[Boris Chiou [:boris]]

Attachment: Bug 1365851 - Part 1: Define ComputedOperation::InterpolateMatrix.

We use this type to store the interpolated result of two mismatched
transform lists, and we resolve it until we know the reference box size
(on Gecko side). The conversion from ComputedOperation::InterpolateMatrix
to eCSSKeyword_interpolatematrix will be implemented in the next patch.

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

Comment 11

[Boris Chiou [:boris]]

Attachment: Bug 1365851 - Part 2: Convert ComputedOperation::InterpolateMatrix into gecko type.

We convert ComputedOperation::InterpolateMatrix into gecko type not on
the main thread, so we cannot use nsCSSValueList_heap (which is not
thread safe so we cannot create it and destroy it on different threads).
Therefore, we use nsCSSValueSharedList to represent the cloned
from_list/to_list.

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

Comment 12

[Boris Chiou [:boris]]

Attachment: Bug 1365851 - Part 3: Add some unit tests for basic transform operations for Servo.

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

Comment 13

[Boris Chiou [:boris]]

Attachment: Bug 1365851 - Part 4: Enable table-overflowed-by-animation.html on stylo.

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

Comment 14

[Boris Chiou [:boris]]

Sorry, wrong bug number