1424506 - CSS3D translate3d transition is not smooth in Firefox Quantum

Status: ASSIGNED

(Core :: Graphics: WebRender, defect, P2)

Description

[henrik.ingo]

Attachment: css3d.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171115220812

Steps to reproduce:

Open attached css3d.html in Firefox Quantum. Click on the "left" and "right" buttons to animate transitions between boxes "ONE" and "TWO".

Exact version: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0


Actual results:

The transition to center the other box in the center of the screen, using CSS3D translate3d and transition, is not smooth.


Expected results:

In previous versions of Firefox, and also Chrome, the transition between the two boxes is smooth.

Comment 1

[henrik.ingo]

The attached repro of this issue is a simplification of what impress.js presentations do. See for example http://impress.github.io/impress.js/

Comment 2

[Alice0775 White]

[Tracking Requested - why for this release]:

I can reproduce the problem on 58.0b11 x64 windows10.
And sometimes,there are no transition animation...

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=401840a241b9861cac205bd866ba24e69d11b7e2&tochange=fa94f7205173d34f23975c6af9cb95237b28c8b8

Regressed by:fa94f7205173	Hiroyuki Ikezoe — Bug 1190721 - Throttle animations that produce any transform change hint if the target element is out-of-view. r=birtles


@:hiro,
Your patch seems to cause the regression.  Can you look into this?

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Thank you Alice, I will take a look.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: A reduced test case

With 'transform-style:preserve-3d', GetVisualOverflowRectRelativeToSelf() returns {0, 0, 0, 0} for the animating frame.  I don't know the reason yet.

Comment 5

[Julien Cristau [:jcristau]]

New regression in 58 with a testcase, tracking.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Another test case using color animation

Actually this bug was introduced by bug 1166500.  As for transform animation we started the optimization for offscreen animation in bug 1190721 though.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
> Created attachment 8936453 [details]
> A reduced test case
> 
> With 'transform-style:preserve-3d', GetVisualOverflowRectRelativeToSelf()
> returns {0, 0, 0, 0} for the animating frame.  I don't know the reason yet.

So IIUC bug 1274962, it's not a good idea to use over flow area for checking whether the frame is scrolled out or not if the frame is a preserve-3d leaf.

I think we should not optimize the preserve-3d case.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a71076d3d7aad515cc7092129d16dc2efeb071e4

Comment 8

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bdbacaf9f9ce9c4a0215f99984d744a16e612c6

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1424506 - Don't throttle preserve-3d frame even if the frame is scrolled out.

If the target frame is a preserve-3d leaf, we can't tell whether the frame is
scrolled out or not since we don't calculate the overflow area for the frame
precisely.

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

Comment 10

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8938785 [details]
Bug 1424506 - Don't throttle preserve-3d frame even if the frame is scrolled out.

https://reviewboard.mozilla.org/r/209318/#review215588

It looks like IsScrolledOutOfView() is using nsIFrame::GetVisualOverflowRectRelativeToSelf(), which should be valid for true leaves (nsIFrame::IsPreserve3DLeaf()), but isn't valid for the root preserve-3d frame.

GetVisualOverflowRectRelativeToSelf() should be correct if !Extend3DContext(), and GetVisualOverflowRect() should be valid if !Combines3DTransformWithAncestors(). So the root frame has a valid post-transform overflow, leaves have valid pre-transform overflows and intermediate frames have neither.

Given that, I think we want to check !frame->Extend3DContext() here.

We could also probably do slightly better if you think the performance of not throttling these animations is likely to be a problem.

* We could optionally use nsIFrame::GetVisualOverflowRect() if the animated frame is the root of the preserve-3d context (and use the parent frame's coordinate space), so that we can get a result for these frames.

* When we compute all the overflow rects for a preserve-3d scene, we should know the overflow rect for an intermediate frame (in the post-transform coordinate space of the preserve-3d root). We could store those as a frame property (maybe only if there's an animation?) so that we can use it during IsScrolledOutOfView().

Sorry, the coordinate spaces and overflow rects for preserve-3d are really complicated. Please let me know if you want me to try expain the current state in more detail.

Comment 11

[Brian Birtles (:birtles)]

Comment on attachment 8938785 [details]
Bug 1424506 - Don't throttle preserve-3d frame even if the frame is scrolled out.

https://reviewboard.mozilla.org/r/209318/#review215594

r=me for the test case. I'll leave the code changes to Matt to review.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Wow!  Thanks for the detailed explanations!

(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> Comment on attachment 8938785 [details]
> Bug 1424506 - Don't throttle preserve-3d frame even if the frame is scrolled
> out.
> 
> https://reviewboard.mozilla.org/r/209318/#review215588
> 
> It looks like IsScrolledOutOfView() is using
> nsIFrame::GetVisualOverflowRectRelativeToSelf(), which should be valid for
> true leaves (nsIFrame::IsPreserve3DLeaf()), but isn't valid for the root
> preserve-3d frame.
> 
> GetVisualOverflowRectRelativeToSelf() should be correct if
> !Extend3DContext(), and GetVisualOverflowRect() should be valid if
> !Combines3DTransformWithAncestors(). So the root frame has a valid
> post-transform overflow, leaves have valid pre-transform overflows and
> intermediate frames have neither.
> 
> Given that, I think we want to check !frame->Extend3DContext() here.
> 
> We could also probably do slightly better if you think the performance of
> not throttling these animations is likely to be a problem.
> 
> * We could optionally use nsIFrame::GetVisualOverflowRect() if the animated
> frame is the root of the preserve-3d context (and use the parent frame's
> coordinate space), so that we can get a result for these frames.
> 
> * When we compute all the overflow rects for a preserve-3d scene, we should
> know the overflow rect for an intermediate frame (in the post-transform
> coordinate space of the preserve-3d root). We could store those as a frame
> property (maybe only if there's an animation?) so that we can use it during
> IsScrolledOutOfView().

I don't yet understand this second part, but it seem to me that the second part requires some extent code changes.  I will only try the first star change in this bug (because I'd like to uplift this to beta) and fix the second part in a later bug.  And if it couldn't happen I will use '!frame->Extend3DContext()' there.

Anyway thanks for you kindness, I will ping you on IRC if I can't understand the second part.

Comment 13

[Julien Cristau [:jcristau]]

Any updates?

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Matt, is this still waiting on a review from you?

Comment 15

[Matt Woodrow (:mattwoodrow)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #14)
> Matt, is this still waiting on a review from you?

I think I'm waiting on an updated patch to address my comments in Comment 10.

Comment 16

[Steve]

I can confirm this is still an issue. Its really disappointing that in order to give presentations I have to use a non-free browser (Chrome).

Is there anything I can do to provide more information to help get this solved?

Comment 17

[Firefox User]

Can you still reproduce this issue in the latest Firefox Nightly?

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:boris, since the bug has recent activity, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 19

[Boris Chiou [:boris]]

The performance is still not good, per my test in Nightly today.

Comment 20

[Firefox User]

The component should be moved to Graphics: WebRender.

Comment 21

[Boris Chiou [:boris]]

Per comment 20, move this to webrender