873060 - [meta] Make entering and exiting customization mode feel smooth

Status: RESOLVED WORKSFORME

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Entering and exiting customization mode can sometimes feel a bit janky. If your machine is under heavy load, sometimes you only get one or two frames of the transition animation, which looks bad.

We might want to try investigating what we can do to smooth that out.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Ugh, yes. This was a concern very early on too (September last year), when it was being designed. Snippet from an email I sent after doing the initial implementation of it:

> Well, the zoom animation for entering the customization mode is very smooth on
> my main desktop PC - but that's a powerful workstation. On my Win8 slate the
> animation is very jerky, which is disappointing :\

And then it became a "we'll look at it later" type of problem, and sadly I forgot about it.

Roughs ideas of things that may help:
* Wait until about:customizing has loaded before doing any transition
* ???

Comment 2

[Morpheus3k]

I've played with this on OS X. If I disable the about:newtab, the closing transition is far smoother. This is probably related to changing the size of the thumbnails during transition.
One solution could be to limit the size of the thumbnails in such a way, that the shrink of the chrome does not shrink the size of the thumbnails.

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Morpheus3k from comment #2)
> I've played with this on OS X. If I disable the about:newtab, the closing
> transition is far smoother. This is probably related to changing the size of
> the thumbnails during transition.
> One solution could be to limit the size of the thumbnails in such a way,
> that the shrink of the chrome does not shrink the size of the thumbnails.

Hmm, should be able to get around that by only closing about:customizing (or going back in history) *after* the transition has ended.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Fixing bug 755598 will also help here, since there will be less changing of styles when in customization mode vs. not in customization mode.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

It's particularly janky on OSX - here's a screencast from shorlander showing it off (red added for emphasis):

http://people.mozilla.com/~shorlander/bugs/customize-mode-transition.webm

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Tentatively assigning to Blake.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Here is a profile where I had ~30 tabs opened and overflowing:

http://people.mozilla.com/~bgirard/cleopatra/#report=91cc1585f6a96b89f55f6841f91216074eb63a43

I opened and closed customization mode several times.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

It looks like we're causing async reflows in a bunch of places, which would account for the sluggishness.

So a few ideas on how to make this better:

1) Do not enter or exit customization mode until the tab has finished opening or closing
2) Temporarily disable smooth-scroll on the tab strip when entering or existing customization mode? Though I'm not entirely convinced that the tabstrip is at fault here.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Another thing - on exit, I notice that we switch back to normal browser content and *then* reduce the padding. I think this means that we have to reflow the content while the padding is reduced.

I've also noticed that the sluggishness seems to be a function of how big the window is. If I make the window small, performance improves dramatically.

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Talking with Benoit, I learned several things…

1) We should file a bug to ask someone (dbaron?) why calling replaceChild on a XUL node (CustomizeMode.jsm:334) causes a sync reflow.
2) The call to _wrapToolbarItems (CustomizeMode.jsm:419) takes around 80ms, which is a little bigger than the 15ms we aim for in browser code.  (Otherwise, videos playing on the page will stutter, and other bad stuff will happen.)
3) Animating the padding will cause a reflow every frame, which is saddening.  Changing it to a scale transform, and then re-setting it on transitionend will look about the same, and be much more performanter.

So, I'm going to take on #3, and claim that #1 and #2 are out of scope for now…  ;)

Comment 11

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch implementing scale transition, and removing the sync reflow when wrapping items.

So, we implemented 1 and 3 in the attached patch, and they helped a little, but we should also split the call to #2 into multiple goes through the run loop.
And, we found out that there's a sync-reflow in tabbrowser.xml's adjustTabstrip (called from _handleNewTab, called from onxbltransitionend) which is taking 146ms.  And while I'm poking around that, I found an anonymous function at CustomizeMode.jsm line 922->935->93 that's taking 172ms.  And finally, the switchToTabHavingURI called from CustomizeMode.jsm is taking 145ms that runs before the animation even starts.

Comment 12

[Mike Conley (:mconley) (:⚙️)]

So the sync reflow caused by the menu panel having its items wrapped is being taken care of in bug 880701.

I filed bug 881909 to take care of wrapping and unwrapping the toolbaritems in a smoother way.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Both bug 880701 and bug 881909 have landed.

I'm going to keep this bug open, since there are a few other things we probably want to do to smooth things out. But I don't think any of those things could possibly land in the User Research Build.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Removing the items from M7 that do not block landing on m-c.

Comment 16

[Blake Winton (:bwinton) (:☕️)]

(This was fixed by another perf bug, wasn't it, mconley?  Either way, I'm not really working on it, and so should free it up for someone else to take.)

Comment 17

[Mike Conley (:mconley) (:⚙️)]

(In reply to Blake Winton (:bwinton) from comment #16)
> (This was fixed by another perf bug, wasn't it, mconley?  Either way, I'm
> not really working on it, and so should free it up for someone else to take.)

Yes, this seems to have become a metabug dealing with improving customization mode transition performance.

Comment 18

[Mike de Boer [:mikedeboer]]

Attachment: WIP Patch: experiment to make animations smoother

Mike/ Jared: I submitted the patch here, because I wanted to post it up as an experiment.

I decided to this because I noticed that pointer events were handled during the transition into cust mode. This is in any case undesirable, but I thought that the attribute could also be used to start minimizing the amount of elements requiring a reflow during each animation step.

Please let me know what you think!

Comment 19

[Mike Conley (:mconley) (:⚙️)]

Hm. I just tried this patch on OSX, and did a comparison with trunk. I'm not feeling much in the way of performance gains. :/ Have you done a measurement with comparing times (via window.performance.now()) to determine if this change actually makes a difference?

Because if not, it's likely not worth the churn / added effort.

Comment 20

[Mike de Boer [:mikedeboer]]

(In reply to Mike Conley (:mconley) from comment #19)
> Hm. I just tried this patch on OSX, and did a comparison with trunk. I'm not
> feeling much in the way of performance gains. :/ Have you done a measurement
> with comparing times (via window.performance.now()) to determine if this
> change actually makes a difference?
> 
> Because if not, it's likely not worth the churn / added effort.

No I didn't do measurements. The main thing I tried to tackle is disabling mouse events during the animation, because not just hovering item but also clicking items is potentially destructive for layout during the animation.

Comment 21

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 784372 [details] [diff] [review]
WIP Patch: experiment to make animations smoother

Review of attachment 784372 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/osx/browser.css
@@ +1035,5 @@
>    #main-window:not([customizing]) .toolbarbutton-1 > .toolbarbutton-menubutton-button[disabled="true"] > .toolbarbutton-icon,
>    #restore-button[disabled="true"] > .toolbarbutton-icon,
>    #main-window:not([customizing]) .toolbarbutton-1[disabled="true"] > .toolbarbutton-menu-dropmarker,
>    #main-window:not([customizing]) .toolbarbutton-1[disabled="true"] > .toolbarbutton-menubutton-dropmarker,
> +  #main-window:not([customize-transitioning]) .toolbarbutton-1:not(:hover):-moz-window-inactive > .toolbarbutton-icon,

Hm. In what case could we be transitioning and have -moz-window-inactive? Are you handling the case where customization mode has been kicked off, and the user quickly focused something else during the transition?

::: browser/themes/shared/customizableui/customizeMode.inc.css
@@ +7,5 @@
>  #tab-view-deck {
>    transition-property: padding;
>    transition-duration: 150ms;
>    transition-timing-function: ease-out;
> +  transition-delay: 50ms;

Why the delay?

Comment 22

[Mike de Boer [:mikedeboer]]

Mike, thanks for checking it out. Your review comments are spot on; consider 'em remnants of some the experiments I've done. The only sorta useful part is to disable pointer events during the animation; since we're animating a rather large piece of the UI (ehm, all of it!), it's not unlikely that a user will move her mouse and/ or click anywhere. That's the only thing left standing after my exercise here methinks...

Comment 23

[Mike Conley (:mconley) (:⚙️)]

Ok, to kick us off here, I added some profile markers to the start and end of both the entering and exiting customization mode transitions. I then entered and exited customization mode a number of times, and used a tool to extract *just* the samples during the transitions.

I did this on my OS X 10.8 Mac Book, but this can be easily replicated on Windows and Linux as well.

So here are samples for ENTERING customization mode:

http://tests.themasta.com/cleopatra/?report=7891484fd3daa2d151f2254a68b43db9a619add5

and here are samples for EXITING customization mode:

http://tests.themasta.com/cleopatra/?report=48bdd391e490b5ddc6e7af6593f5308dc2a81a11

Comment 24

[Mike Conley (:mconley) (:⚙️)]

It's clear from these that we're spending an enormous time painting, both when entering and when exiting.

Comment 25

[Mike Conley (:mconley) (:⚙️)]

Gonna be looking at this stuff again once I get the BrowserUITelemetry stuff wrapped.

Comment 26

[Mike Conley (:mconley) (:⚙️)]

Actually, I believe we discussed with madhava that delaying perf work on the customize mode transition for Aurora might not be advantageous, since we want to make a good first impression on folks who are upgrading...

Comment 27

[Mike Conley (:mconley) (:⚙️)]

Here's a link to a recent reflow profile for the Enter transition on Windows 7:

http://tests.themasta.com/cleopatra/?report=2f8bce629c9fa5756ebd4278e48f8e6ec2218227

Comment 28

[Mike Conley (:mconley) (:⚙️)]

PSA:

The Gecko Profiler on Windows with stackwalking enabled introduces a _ton_ of overhead when switching in and out of customize mode.

If you're seeing a lot of jank while profiling, and wondering why you're not seeing it...it's because it's the profiler doing it. Disable the profiler (or turn of stackwalking) to check.

If that's the case, you can still probably get decent data if you increase the animation duration from 150ms to something like 450ms.