1375812 - stylo: Should build an identify matrix for InterpolateMatrix

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P1)

Description

[Boris Chiou [:boris]]

These is a case we crash because we didn't build an identity matrix for InterpolateMatrix:

  <style>
    #target {
      width: 100px;
      height: 100px;
      background-color: blue;
      transition: all 10s linear;
      transform: translateX(100px);
    }
  </style>
  <script>
    var div = document.getElementById('target');
    div.style.setProperty("transform", "rotate(60deg)", "");
    window.getComputedStyle(div).transform;
    div.style.setProperty("transform", "none", "");
  </script>
  <div id="target"></div>

We might trigger an transition from an InterpolateMatrix to none, so panic [1].

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

Comment 1

[Brian Birtles (:birtles)]

I think we've been using P1 for stability issues like this.

Comment 2

[Boris Chiou [:boris]]

Attachment: Bug 1375812 - Build an identity matrix for InterpolateMatrix.

We have to build an identity matrix while add_weighted() between
InterpolateMatrix and none transform in some cases, e.g. trigger a
transition from a mid-point of another transition to none.

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

Comment 3

[Boris Chiou [:boris]]

Attachment: Bug 1375812 - Add crashtest for interpolation between interpolatematrix and none.

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

Comment 4

[Boris Chiou [:boris]]

Comment on attachment 8881396 [details]
Bug 1375812 - Add crashtest for interpolation between interpolatematrix and none.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152550/diff/1-2/

Comment 5

[Brian Birtles (:birtles)]

Comment on attachment 8881395 [details]
Bug 1375812 - Build an identity matrix for InterpolateMatrix.

https://reviewboard.mozilla.org/r/152548/#review157850

r=me assuming we also have a test that shows this gives the correct result

Comment 6

[Brian Birtles (:birtles)]

Comment on attachment 8881396 [details]
Bug 1375812 - Add crashtest for interpolation between interpolatematrix and none.

https://reviewboard.mozilla.org/r/152550/#review157852

Comment 7

[Boris Chiou [:boris]]

Attachment: Bug 1375812 - Test the result of interpolation from interpolatematrix to none.

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

Comment 8

[Brian Birtles (:birtles)]

Comment on attachment 8881846 [details]
Bug 1375812 - Test the result of interpolation from interpolatematrix to none.

https://reviewboard.mozilla.org/r/152916/#review158128

Comment 9

[Boris Chiou [:boris]]

Comment on attachment 8881396 [details]
Bug 1375812 - Add crashtest for interpolation between interpolatematrix and none.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152550/diff/2-3/

Comment 10

[Boris Chiou [:boris]]

Comment on attachment 8881846 [details]
Bug 1375812 - Test the result of interpolation from interpolatematrix to none.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152916/diff/1-2/

Comment 11

[Boris Chiou [:boris]]

Attachment: Servo PR, #17553

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb9ac25d6756
Add crashtest for interpolation between interpolatematrix and none. r=birtles
https://hg.mozilla.org/integration/autoland/rev/5394e6b2347b
Test the result of interpolation from interpolatematrix to none. r=birtles

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bb9ac25d6756
https://hg.mozilla.org/mozilla-central/rev/5394e6b2347b