1458715 - Strange CSS transition with transform: perspective

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: testcase.html

STR: Please look at the test case and hover the the cursor over it.

Note:

I am trying to do a 3d scene where the foreground object will jump out when hovered.

The first problem is with the first box: the 3d transition is obvious wrong; moving the mouse a little but will cause the div to become very small and back, even then the transform value set is rotateY and rotateX, not translateZ nor perspective.

The second box tried to remove that transition while the cursor is over the box (and also make it more responsive while active). Doing so however will also remove the transition when the cursor leaves the area.

The desired result should be no strange transition when the cursor is moving over the box *and* keeping the transition of restoring the box when the cursor leaves. Safari/Chrome currently do that correctly.

I am not sure what the spec says in these cases; this test case is already a reduced test case. Let me know if I can reduce it further.

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: with-perspective-property.html

It looks like with a little change (not setting transform: perspective on the transition divs but using perspective property on its parent) will fix this.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Probably more of a Graphics or animations issue. Could you try to find whether this is a regression?

Comment 3

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5f721a664bf64fed99184a866b60c24a6afcb3a0&tochange=62cebea1d1578461a423a9ca7848706455db9ea5

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Thanks a lot Alice, you're awesome!

Manish, Hiro, this looks like some more (pre-existing, though) transform interpolation issue. Any chance any of you could look into it? I can do it when I'm back from PTO otherwise.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I tried to find a more useful regression range but this go back as far as stylo is built on automation unfortunately.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

I kept looking at it. I cannot make sense of that code.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1458715: Fix perspective interpolation.

It's not sound to insert random matrices in random positions in the transform
operation list.

I cannot make any sense of what the old code was trying to do.

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

Comment 8

[Emilio Cobos Álvarez (:emilio)]

I guess the alternative fix is returning false for is_matched_component for perspective, but that seems unnecessarily expensive IMO. I don't know why this shouldn't work.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Try looks happy: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ab6ae1746d2721182bad2ad3f5e139dd97c965ad

Which means that it's time to write a test, d'oh :)

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8972797 [details]
Bug 1458715: Fix perspective interpolation.

https://reviewboard.mozilla.org/r/241370/#review247190

Good catch!  This fix makes quite sense to me. (I was wondering why I did implement this in bug 1332657, but actually it's not by me. :) )

I think we should fix ComputeSquaredDistance as well.  Would you mind opening a new bug or fix it in this bug?

Comment 11

[Emilio Cobos Álvarez (:emilio)]

I couldn't come up with a reliable test-case... :(

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Note that after this patch you can still glitch it out a bit, but I think that's a different issue related to how DOM events are dispatched and handled in the demo. Could you confirm once this is on Nightly & file new bugs if appropriate?

Comment 13

[Emilio Cobos Álvarez (:emilio)]

(ni? for comment 12)

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d477ed8d62d
Fix perspective interpolation. r=hiro

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62a80b3ee6fa
followup: don't fix distance computation so fast. r=me

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Emilio Cobos Álvarez [:emilio] (Away-ish from 27/04 to 09/05) from comment #12)
> Note that after this patch you can still glitch it out a bit, but I think
> that's a different issue related to how DOM events are dispatched and
> handled in the demo. Could you confirm once this is on Nightly & file new
> bugs if appropriate?

Will do after this reaches Nightly. Thanks for the quick fix!

Comment 17

[Manish Goregaokar [:manishearth]]

Yeah this is a bug that slipped through during implementation

https://github.com/servo/servo/pull/13188/files#diff-588c9761e23f8c7c9a0338224c058967R750

Comment 18

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/5d477ed8d62d
https://hg.mozilla.org/mozilla-central/rev/62a80b3ee6fa

Comment 19

[Hiroyuki Ikezoe (:hiro)]

(In reply to Emilio Cobos Álvarez [:emilio] (Away-ish from 27/04 to 09/05) from comment #12)
> Note that after this patch you can still glitch it out a bit, but I think
> that's a different issue related to how DOM events are dispatched and
> handled in the demo.

That's interesting.  It's another variant of bug 1354501 (Tim reported a duplicate of this bug).  Though I am surprised that Emilio noticed this symptoms quickly, that's he. :)

Comment 20

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I don't see this fixed on the current Nightly. The behavior of the test case remained unchanged.

about:buildconfig Built from https://hg.mozilla.org/mozilla-central/rev/d07a4da682a2f8a2df811d8f0734a5a632213c9b

Firefox 61.0a1 (2018-05-04) (64-bit) (macOS)

Comment 21

[Emilio Cobos Álvarez (:emilio)]

Attachment: recording.ogv

So, in current nightly, once I get the hover effect to stabilize, the transition does work as expected, which clearly didn't before. See left (older build) vs. right (nightly). Does it match what you see?

However, as I said even with that patch you can still glitch it out if you hover in between the two squares at the beginning of the effect (which is also seen on the video).

I haven't diagnosed whether that's another interpolation bug or another thing, but in any case should probably be a bug other than this one.

Comment 22

[Hiroyuki Ikezoe (:hiro)]

There seems to be another interpolation bug.  I guess it's a case of singular matrix that we handled somehow on the old style system.  Anyway, I guess the issue will be fixed by implementing smarter interpolation for transform functions. <https://github.com/w3c/csswg-drafts/issues/927>

Comment 23

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Yes, the eventual state is correct on Nightly.

So I assume we are looking at 3 bugs here?

1. For box 1, transition b/t two states seems "crazy" and off.
2. For box 2, transition when the cursor leaves the box did not end at the right size.
3. The eventual state of box 1 is wrong (fixed)

I don't understand CSS style impl well enough to describe these bugs separately each with the correct descriptions, so perhaps it's better for who do to file them.

Comment 24

[Emilio Cobos Álvarez (:emilio)]

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #23)
> Yes, the eventual state is correct on Nightly.
> 
> So I assume we are looking at 3 bugs here?
> 
> 1. For box 1, transition b/t two states seems "crazy" and off.
> 2. For box 2, transition when the cursor leaves the box did not end at the
> right size.
> 3. The eventual state of box 1 is wrong (fixed)
> 
> I don't understand CSS style impl well enough to describe these bugs
> separately each with the correct descriptions, so perhaps it's better for
> who do to file them.

Filed bug 1459403, I think both are related, but icbw. If you could reduce them a bit more, so that all the event handling is gone, just proving the interpolation issue, that'd be extremely helpful.