Closed
Bug 873060
Opened 12 years ago
Closed 6 years ago
[meta] Make entering and exiting customization mode feel smooth
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: mconley, Assigned: mconley)
References
(Blocks 1 open bug, )
Details
(Keywords: meta, Whiteboard: [Australis:P-])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
(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•12 years ago
|
||
Fixing bug 755598 will also help here, since there will be less changing of styles when in customization mode vs. not in customization mode.
Assignee | ||
Updated•12 years ago
|
Whiteboard: [Australis:M?] → [Australis:M7]
Assignee | ||
Comment 5•12 years ago
|
||
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
Assignee | ||
Comment 7•11 years ago
|
||
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.
Assignee | ||
Comment 8•11 years ago
|
||
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.
Assignee | ||
Comment 9•11 years ago
|
||
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•11 years ago
|
||
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… ;)
Assignee | ||
Updated•11 years ago
|
Whiteboard: [Australis:M7] → [Australis:M7][User Research Build+]
Comment 11•11 years ago
|
||
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.
Assignee | ||
Comment 12•11 years ago
|
||
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.
Assignee | ||
Comment 13•11 years ago
|
||
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.
Whiteboard: [Australis:M7][User Research Build+] → [Australis:M7]
Assignee | ||
Comment 14•11 years ago
|
||
Removing the items from M7 that do not block landing on m-c.
Whiteboard: [Australis:M7] → [Australis:M?]
Updated•11 years ago
|
Whiteboard: [Australis:M?] → [Australis:M?][Australis:P2]
Updated•11 years ago
|
Status: NEW → ASSIGNED
Comment 16•11 years ago
|
||
(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.)
Assignee: bwinton → nobody
Status: ASSIGNED → NEW
Assignee | ||
Comment 17•11 years ago
|
||
(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•11 years ago
|
||
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!
Attachment #760914 -
Attachment is obsolete: true
Attachment #784372 -
Flags: feedback?(mconley)
Attachment #784372 -
Flags: feedback?(jaws)
Assignee | ||
Comment 19•11 years ago
|
||
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.
Updated•11 years ago
|
Attachment #784372 -
Flags: feedback?(jaws)
Comment 20•11 years ago
|
||
(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.
Assignee | ||
Comment 21•11 years ago
|
||
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?
Attachment #784372 -
Flags: feedback?(mconley)
Comment 22•11 years ago
|
||
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...
Assignee | ||
Comment 23•11 years ago
|
||
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
Assignee | ||
Comment 24•11 years ago
|
||
It's clear from these that we're spending an enormous time painting, both when entering and when exiting.
Updated•11 years ago
|
Assignee | ||
Comment 25•11 years ago
|
||
Gonna be looking at this stuff again once I get the BrowserUITelemetry stuff wrapped.
Assignee: nobody → mconley
Assignee | ||
Updated•11 years ago
|
Whiteboard: [Australis:M?][Australis:P2] → [Australis:M?][Australis:P2][investigate-fix-on-aurora]
Assignee | ||
Comment 26•11 years ago
|
||
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...
Whiteboard: [Australis:M?][Australis:P2][investigate-fix-on-aurora] → [Australis:M?][Australis:P2]
Updated•11 years ago
|
Summary: [meta] Make entering and existing customization mode feel smooth → [meta] Make entering and exiting customization mode feel smooth
Assignee | ||
Comment 27•11 years ago
|
||
Here's a link to a recent reflow profile for the Enter transition on Windows 7:
http://tests.themasta.com/cleopatra/?report=2f8bce629c9fa5756ebd4278e48f8e6ec2218227
Assignee | ||
Comment 28•11 years ago
|
||
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.
Assignee | ||
Updated•11 years ago
|
Whiteboard: [Australis:M?][Australis:P2] → [Australis:M?][Australis:P-]
Updated•11 years ago
|
Whiteboard: [Australis:M?][Australis:P-] → [Australis:P-]
Assignee | ||
Updated•11 years ago
|
Depends on: australis-cart
Updated•6 years ago
|
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•