628949 - Tab bar turns black bar temporarily when maximizing window

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Luke Iliffe (Harlequin99)]

When maximizing minefield the empty part of the tab bar turns black during the transition to the maximized state then disappears. This occurs both with hardware acceleration turned on or off. Recent regression in the 2011-01-25 nightly confirmed by another Windows 7 user on mozillazine; http://forums.mozillazine.org/viewtopic.php?p=10351061#p10351061

Hourly regression range:
works	2011-01-24-1295915186-20110124162626-742bc2628690	http://hg.mozilla.org/mozilla-central/rev/742bc2628690
broken	2011-01-24-1295920388-20110124175308-9d9dbd3c1a6d	http://hg.mozilla.org/mozilla-central/rev/9d9dbd3c1a6d

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=742bc2628690&tochange=9d9dbd3c1a6d

Comment 1

[Luke Iliffe (Harlequin99)]

I forgot to add the simple STR:

1. Open new profile, ensure the window has a normal windowstate.
2. Maximise window

Expected:
Empty tab bar doesn't flash solid black.

Actual:
Empty part of tab bar flashes black.

Note in a new profile this happens very quickly, it is easier to see in my normal profile for some reason. It is easier to see if you only have 1 or 2 tabs open rather than a full or overflowing tab bar. 

Not H/A related but I include my graphics information in case it is not universally reproducible.

Adapter Description: Mobile Intel(R) 4 Series Express Chipset Family
Vendor ID: 8086
Device ID: 2a42
Adapter RAM: Unknown
Adapter Drivers: igdumdx32 igd10umd32
Driver Version: 8.15.10.2226
Driver Date: 10-15-2010
Direct2D Enabled: true
DirectWrite Enabled: true (6.1.7600.20830)
WebGL Renderer: (WebGL unavailable)
GPU Accelerated Windows: 1:/1 Direct3D 10

Mozilla/5.0 (Windows NT 6.1; rv:2.0b11pre) Gecko/20110126 Firefox/4.0b11pre

Comment 2

[[not reading bugmail]]

This reminds me of an old bug that we seen before.

Comment 3

[Luke Iliffe (Harlequin99)]

Attachment: Brief video illustrating the problem.

Attaching a brief video illustrating the problem. This was originally posted on mozillazine http://forums.mozillazine.org/viewtopic.php?p=10355021#p10355021

Comment 4

[Asa Dotzler [:asa]]

Attachment: black bar below aero glass area

I believe what's happening here is the aero glass area is quickly transitioning from the restored window to the maximized window position but Firefox's XUL navigation toolbar is slower to make that transition. So, there's a short period of time where there is no Firefox chrome below the bottom edge of the aero glass drawn area. 

I wonder if we could extend the aero glass down further behind the navigation toolbar so that when we maximized, that flash was of glass rather than empty (black).  Alternatively, if we had some way to color it toolbar color rather than black, that would help the flashing as well.

Comment 5

[Asa Dotzler [:asa]]

This is a pretty ugly experience in a fairly common user interaction (maxmizing a window) so I'm nominating as a Firefox 4 blocker. I believe that bug 624197 is the same issue. (I can see it just hitting alt twice to open and close the menubar when in maximized state.)

Jim, do you have any ideas here? Would putting a tiny (imperceptible) border radius on the nav toolbar's top outside corners force glass behind the nav bar and fix this?  Are there downsides to having glass extend behind our Chrome that would make that a poor solution (hack) to kill the ugly black flashing?

Comment 6

[Jim Mathies [:jimm]]

The glass margin looks right, but for some reason the content isn't repainting quickly enough to fill in the content area. I wonder if that margin is a carry over from normal mode or if it's been updated after the reflow but the paint hasn't happened yet.

Comment 7

[Jim Mathies [:jimm]]

(In reply to comment #5)
> Jim, do you have any ideas here? Would putting a tiny (imperceptible) border
> radius on the nav toolbar's top outside corners force glass behind the nav bar
> and fix this?  Are there downsides to having glass extend behind our Chrome
> that would make that a poor solution (hack) to kill the ugly black flashing?

I believe it already extends down to content. Changing this effects the position of the haze in the window border.

Comment 8

[Asa Dotzler [:asa]]

OK. I was just guessing because while I can repeat the black flash experience with alt key hit twice in maximized mode, I cannot in restored window mode.

Comment 9

[Jim Mathies [:jimm]]

(In reply to comment #7)
> (In reply to comment #5)
> > Jim, do you have any ideas here? Would putting a tiny (imperceptible) border
> > radius on the nav toolbar's top outside corners force glass behind the nav bar
> > and fix this?  Are there downsides to having glass extend behind our Chrome
> > that would make that a poor solution (hack) to kill the ugly black flashing?
> 
> I believe it already extends down to content. Changing this effects the
> position of the haze in the window border.

Actually, I should note this totally depends on how you have your chrome configured. On my system I have tabs on top, the nav bar and the favorites bar, and I get a glass margin of 112px.


(In reply to comment #8)
> OK. I was just guessing because while I can repeat the black flash experience
> with alt key hit twice in maximized mode, I cannot in restored window mode.

Yeah I can totally reproduce that even with gdi rendering.

Comment 10

[Jim Mathies [:jimm]]

Looks like we reflow, set the margin, then repaint. The delay between the margin set and the paint gives us the black area.

Comment 11

[Jim Mathies [:jimm]]

(In reply to comment #10)
> Looks like we reflow, set the margin, then repaint. The delay between the
> margin set and the paint gives us the black area.

..and it only happens in maximized mode. Interesting.

Comment 12

[Jim Mathies [:jimm]]

So I think your idea of rounding the nav bar edges or some other similar fix would address this. Whatever we can do to push the margin down so it's below the top of the nav bar after hitting alt.

Comment 13

[Asa Dotzler [:asa]]

Felipe and I talked about this in another bug. A radius that's small enough to not be visible would be ideal. I wonder if that's got any kind of perf cost that would make it an unsuitable bandaid?

Comment 14

[Dão Gottwald [:dao]]

Extending the glass area surely isn't free. Doesn't sound like the right tradeoff to me.

Comment 15

[Asa Dotzler [:asa]]

Dão, while I'll take your word that it's not free, it's what we already do for restored windows.

Comment 16

[Timothy Nikkel (:tnikkel)]

(In reply to comment #10)
> Looks like we reflow, set the margin, then repaint. The delay between the
> margin set and the paint gives us the black area.

When/where do we set the margins?

Comment 17

[Jim Mathies [:jimm]]

(In reply to comment #16)
> (In reply to comment #10)
> > Looks like we reflow, set the margin, then repaint. The delay between the
> > margin set and the paint gives us the black area.
> 
> When/where do we set the margins?

In widget's UpdateTransparentRegion.

Comment 18

[Timothy Nikkel (:tnikkel)]

Can we just make the UpdateTransparentRegion call in nsLayoutUtils::PaintFrame after the PaintRoot call there?

Comment 19

[Jim Mathies [:jimm]]

Attachment: layout utils patch

This seemed to cut down on the number of occurrences, but it didn't fix this 100% of the time.

Comment 20

[Timothy Nikkel (:tnikkel)]

Comment on attachment 510253 [details] [diff] [review]
layout utils patch

You should guard the new UpdateTransparentRegion call with !willFlushRetainedLayers && widget && !(aFlags & PAINT_DOCUMENT_RELATIVE) like it was before it was moved. I don't think that will make the remaining problem go away though.

Comment 21

[Johnathan Nightingale [:johnath]]

Not a blocker - would take an extremely safe patch. Is this that?

Comment 22

[Jim Mathies [:jimm]]

(In reply to comment #20)
> Comment on attachment 510253 [details] [diff] [review]
> layout utils patch
> 
> You should guard the new UpdateTransparentRegion call with
> !willFlushRetainedLayers && widget && !(aFlags & PAINT_DOCUMENT_RELATIVE) like
> it was before it was moved. I don't think that will make the remaining problem
> go away though.

Let me clean this up, maybe we can still get this approved for 4.0.

Comment 23

[Jim Mathies [:jimm]]

Attachment: call UpdateTransparentRegion after PaintRoot v.1

This significantly improves the about:addons case.

STR -
1) enable fx button
2) open about:addons in a tab
3) hit the alt key repeatedly

Without this patch you always see a black band in glass when the menu bar is shown. With the patch this anomaly doesn't occur. The maximized case isn't addressed 100% of the time, but it improves.

I'll fire off a try build for people to test with.

Comment 25

[Jim Mathies [:jimm]]

Comment on attachment 512806 [details] [diff] [review]
call UpdateTransparentRegion after PaintRoot v.1

Try server likes it.

Comment 26

[Timothy Nikkel (:tnikkel)]

Comment on attachment 512806 [details] [diff] [review]
call UpdateTransparentRegion after PaintRoot v.1

This looks good to me, but I'd like roc to look at it to make sure setting the transparent region after painting is ok.

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 512806 [details] [diff] [review]
call UpdateTransparentRegion after PaintRoot v.1

Great idea!

Comment 28

[Mike Beltzner [:beltzner, not reading bugmail]]

Nasty bug, big patch - my vote is to approve it, but would like other drivers to weigh in.

Comment 29

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 512806 [details] [diff] [review]
call UpdateTransparentRegion after PaintRoot v.1

Not really a big patch IMHO. It's mostly just moving code around.

Comment 30

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/792bb2cb1cb1