Last Comment Bug 713572 - annoying black flicker on menus and dropdowns
: annoying black flicker on menus and dropdowns
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla12
Assigned To: Karl Tomlinson (ni?:karlt)
:
Mentors:
Depends on:
Blocks: 579374
  Show dependency treegraph
 
Reported: 2011-12-26 15:07 PST by Stefan
Modified: 2012-01-05 08:32 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stack of black paint (4.20 KB, text/plain)
2011-12-29 01:54 PST, Karl Tomlinson (ni?:karlt)
no flags Details
don't draw from BasicThebesLayerBuffers when transaction is incomplete (1.15 KB, patch)
2012-01-03 22:15 PST, Karl Tomlinson (ni?:karlt)
no flags Details | Diff | Review
don't draw from BasicThebesLayerBuffers when transaction is incomplete v2 (2.07 KB, patch)
2012-01-04 13:40 PST, Karl Tomlinson (ni?:karlt)
roc: review+
Details | Diff | Review

Description Stefan 2011-12-26 15:07:56 PST
User Agent:  

Steps to reproduce:

1. Focus on a tab.
2. Repeat the sequence "Menu-Key*, ESC" rapidly (e.g. 2/s)

Alternatively invoke the tab's menu by a right click.

*) http://en.wikipedia.org/wiki/Menu_key


Actual results:

2. Flicker: A black rectangle is painted before the context menu appears which.


Expected results:

2. No flicker.

last good nightly 2011-01-24-03-mozilla-central 80266029824b
first bad nightly 2011-01-25-03-mozilla-central 6a097e294828

The first bad revision is:
changeset: 61193:e874889e43d1
user: Ehsan Akhgari <ehsan@mozilla.com>
date: Fri Jan 21 16:45:23 2011 -0500
summary: Bug 579374 - Clear cached resources for GTK windows when we hide/destroy them; r=roc a=blocking-final+
Comment 1 Timothy Nikkel (:tnikkel) 2011-12-27 21:36:16 PST
Maybe we should be doing ClearCachedResources at the end of nsWindow::Show?
Comment 2 Timothy Nikkel (:tnikkel) 2011-12-27 21:37:32 PST
I wasn't able to reproduce.

Stefan, you seem to be proficient in doing your own builds? Could you try moving this http://mxr.mozilla.org/mozilla-central/source/widget/src/gtk2/nsWindow.cpp#1086 ClearCachedResources call to the end of nsWindow::Show?
Comment 3 Karl Tomlinson (ni?:karlt) 2011-12-28 01:14:22 PST
Thanks very much for the regression window.
Which window manager is in use?  Is compositing enabled?
Comment 4 Stefan 2011-12-28 02:55:59 PST
 (In reply to Timothy Nikkel (:tn) from comment #2)
> Stefan, you seem to be proficient in doing your own builds? Could you try
> moving this
> http://mxr.mozilla.org/mozilla-central/source/widget/src/gtk2/nsWindow.
> cpp#1086 ClearCachedResources call to the end of nsWindow::Show?

The flicker goes but then the tooltip border issue of Bug 579374 is back.

(In reply to Karl Tomlinson (:karlt) from comment #3)
> Which window manager is in use?

KDE 3.5

> Is compositing enabled?

Do you mean "Compositing" in xorg.conf? Then it is disabled.
Comment 5 Karl Tomlinson (ni?:karlt) 2011-12-28 20:09:32 PST
I think I can reproduce something similar with compositing disabled.
(I'm using kwin 4.7, but the window manager should not be involved if compositing is enabled.)

If I click on the location bar dropdown button quickly several times, then sometimes
I see a black rectangular flash across the full width of the dropdown, but covering only a fraction of the vertical distance (and presumably depends on timing with vrefresh).

Under kwin, the initial state of compositing is controlled via System Settings -> Desktop Effects -> Enable desktop effects at startup.  If supported, it can be toggled with Alt+Shift+F12, but, if "xdpyinfo | grep Composite" doesn't find Composite, then window compositing won't be available.
Comment 6 Stefan 2011-12-28 20:31:25 PST
According to

> "xdpyinfo | grep Composite"

the "Composite" extension actually IS active. Nontheless disabling it (explicit "Composite" "Disabled" in xorg.conf) does NOT mitigate the flicker here.
Comment 7 Karl Tomlinson (ni?:karlt) 2011-12-29 01:54:31 PST
Created attachment 584720 [details]
stack of black paint

We're painting the whole window black because we are drawing from the buffer for a layer with an empty valid region during an incomplete empty layer manager transaction.
Comment 8 Timothy Nikkel (:tnikkel) 2011-12-29 02:09:47 PST
I think that would be what we would expect if we got a paint event for a window that has had ClearCachedResources called on it. Why would we get such a paint though?
Comment 9 Karl Tomlinson (ni?:karlt) 2011-12-29 02:11:11 PST
(In reply to Timothy Nikkel (:tn) from comment #8)
It has been shown again.
Comment 10 Timothy Nikkel (:tnikkel) 2011-12-29 22:21:42 PST
Ah, ok. So it must be an invalidate after drawing the black and then it draws correctly? When does that happen?
Comment 11 Karl Tomlinson (ni?:karlt) 2011-12-30 01:11:14 PST
During EndEmptyTransaction, it looks like PaintBuffer is designed to do
nothing more than mark the empty tranaction incomplete.  This will ensure that
EndEmptyTransaction returns false and so a non-empty transaction will
immediately take place (and this draws correctly).

However the black paint during the empty transaction is happening directly on the
window (because double buffering is not necessary).  I don't know why
PaintThebes is still drawing the buffer (which is invalid and black) even
though it knows the transaction is incomplete.
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-12-30 01:18:06 PST
(In reply to Karl Tomlinson (:karlt) from comment #11)
> I don't know why
> PaintThebes is still drawing the buffer (which is invalid and black) even
> though it knows the transaction is incomplete.

No good reason. Please fix that!
Comment 13 Karl Tomlinson (ni?:karlt) 2012-01-03 22:15:04 PST
Created attachment 585656 [details] [diff] [review]
don't draw from BasicThebesLayerBuffers when transaction is incomplete

Looks like we still need to BasicShadowableThebesLayer::PaintBuffer() even
with a null aCallback in order to process PaintState::mDidSelfCopy.
Comment 14 Stefan 2012-01-03 23:34:19 PST
(In reply to Karl Tomlinson (:karlt) from comment #13)
> Created attachment 585656 [details] [diff] [review]
> don't draw from BasicThebesLayerBuffers when transaction is incomplete

Works. Thx.
Comment 15 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-01-04 00:42:08 PST
Comment on attachment 585656 [details] [diff] [review]
don't draw from BasicThebesLayerBuffers when transaction is incomplete

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

Better to add an IsTransactionIncomplete() getter to BasicLayerManager, and call it to do an early return before "if (!IsHidden())". I think that will be a bit more clear. Also more robust since we won't be relying on state.mContext being null if there's no invalid area.
Comment 16 Karl Tomlinson (ni?:karlt) 2012-01-04 01:26:01 PST
Comment on attachment 585656 [details] [diff] [review]
don't draw from BasicThebesLayerBuffers when transaction is incomplete

OK.  That would be safer wrt behaviour changes in the other methods.
Comment 17 Karl Tomlinson (ni?:karlt) 2012-01-04 13:40:04 PST
Created attachment 585871 [details] [diff] [review]
don't draw from BasicThebesLayerBuffers when transaction is incomplete v2
Comment 18 Karl Tomlinson (ni?:karlt) 2012-01-04 21:40:41 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/f1e771f3c2cd
Comment 19 Marco Bonardo [::mak] 2012-01-05 08:32:30 PST
https://hg.mozilla.org/mozilla-central/rev/f1e771f3c2cd

Note You need to log in before you can comment on or make changes to this bug.