Note: There are a few cases of duplicates in user autocompletion which are being worked on.

closing a tab reflows the entire tab bar during the animation, making the animation crappy

RESOLVED FIXED in mozilla19

Status

()

Core
Layout
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gal, Assigned: mattwoodrow)

Tracking

(Blocks: 1 bug, {perf})

unspecified
mozilla19
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [snappy:p1])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
STR: Enable paint flashing, open a few tabs and close the rightmost one. As the animation happens we repaint the entire tab bar as we do the "shrink tab" animation.
I doubt that front-end tabbrowser code can do anything about this...
Keywords: perf
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [snappy]
(Reporter)

Comment 2

5 years ago
Diagnose it. You are doing something that causes a reflow. Probably changing the size and it's not absolute positioned. Once we know what causes it we can see if we can avoid triggering it or teach the platform to not redraw since the sizes aren't affected.
(In reply to Andreas Gal :gal from comment #2)
> Diagnose it. You are doing something that causes a reflow. Probably changing
> the size and it's not absolute positioned. Once we know what causes it we
> can see if we can avoid triggering it or teach the platform to not redraw
> since the sizes aren't affected.

I noticed this back in the Firefox 4 days. Off the top of my head: The fundamental problem is that we're using flexbox for our CSS transitions. If the animations were only on CSS transforms and nothing else, we could layerize the tab strip animations and avoid the repaint.
Blocks: 593680

Updated

5 years ago
Duplicate of this bug: 750495
(In reply to Andreas Gal :gal from comment #2)
> Once we know what causes it we
> can see if we can avoid triggering it or teach the platform to not redraw
> since the sizes aren't affected.

Pretty much any "we paint stuff that hasn't changed" bug should depend on DLBI. DLBI is very close to landing and even if it doesn't fix all those bugs, it makes them much easier to fix.
(In reply to Patrick Walton (:pcwalton) from comment #3)
> I noticed this back in the Firefox 4 days. Off the top of my head: The
> fundamental problem is that we're using flexbox for our CSS transitions. If
> the animations were only on CSS transforms and nothing else, we could
> layerize the tab strip animations and avoid the repaint.

The current setup is not an oversight, it's the behavior we want. It makes a difference as soon as you close a tab that isn't the last one or as soon as you have enough tabs to reduce their width.
Depends on: 539356
(In reply to Dão Gottwald [:dao] from comment #6)
> (In reply to Patrick Walton (:pcwalton) from comment #3)
> > I noticed this back in the Firefox 4 days. Off the top of my head: The
> > fundamental problem is that we're using flexbox for our CSS transitions. If
> > the animations were only on CSS transforms and nothing else, we could
> > layerize the tab strip animations and avoid the repaint.
> 
> The current setup is not an oversight, it's the behavior we want. It makes a
> difference as soon as you close a tab that isn't the last one or as soon as
> you have enough tabs to reduce their width.

Those could be done with CSS transforms as well, by making the middle part of each tab a separate piece that resizes via a width transform.
A scaleX transform, I mean.
CSS transforms can't get us the current behavior.

Updated

5 years ago
Whiteboard: [snappy] → [snappy:p1]

Updated

5 years ago
Depends on: 753559
(Assignee)

Comment 10

5 years ago
Created attachment 671727 [details] [diff] [review]
Remove calls to Redraw from nsSprocketLayout

DLBI should be able to handle these changes itself.

This reduces the invalidation considerably when closing a tab. I now only see changes to the last two tabs.
Attachment #671727 - Flags: review?(roc)
Component: Tabbed Browser → Layout
Product: Firefox → Core
Comment on attachment 671727 [details] [diff] [review]
Remove calls to Redraw from nsSprocketLayout

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

It looks OK to me, but who knows really? Let's do it! :-)
Attachment #671727 - Flags: review?(roc) → review+

Updated

5 years ago
Assignee: nobody → matt.woodrow
(Assignee)

Comment 12

5 years ago
Created attachment 671735 [details]
Screen recording of paint flashing during tab closing

Sorry about the format, quicktime isn't much help here.
(Assignee)

Comment 13

5 years ago
Invalidation log that matches the video: http://pastebin.mozilla.org/1868182

Updated

5 years ago
No longer depends on: 753559
(Assignee)

Comment 14

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/000af356f525
Can this change be backported to Firefox 18? (after enough testing, I mean)
(Assignee)

Comment 16

5 years ago
I wasn't planning on requesting approval, since we don't really know what regressions this might cause.

However, if it's a significant performance improvement, then it might be worth considering.

We should probably try get input from the Snappy team in that case.

Comment 17

5 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #16)
> I wasn't planning on requesting approval, since we don't really know what
> regressions this might cause.
> 
> However, if it's a significant performance improvement, then it might be
> worth considering.
> 
> We should probably try get input from the Snappy team in that case.

I'm waiting for this to land to see what the impact is.

Matt, can we do something similar to fix flicker in overflow animation(ie when you open and close tabs in tab overflow situation)?
https://hg.mozilla.org/mozilla-central/rev/000af356f525
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19

Updated

5 years ago
Depends on: 803556
You need to log in before you can comment on or make changes to this bug.