Last Comment Bug 768400 - Re-opening a transparent panel sometimes renders outdated content for a split second
: Re-opening a transparent panel sometimes renders outdated content for a split...
Status: RESOLVED FIXED
: testcase
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: mozilla16
Assigned To: Jim Mathies [:jimm]
:
: Jim Mathies [:jimm]
Mentors:
Depends on:
Blocks: 773360
  Show dependency treegraph
 
Reported: 2012-06-26 04:39 PDT by Dão Gottwald [:dao]
Modified: 2012-07-12 11:16 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (873 bytes, application/vnd.mozilla.xul+xml)
2012-06-26 05:36 PDT, Dão Gottwald [:dao]
no flags Details
patch (2.73 KB, patch)
2012-07-11 05:01 PDT, Jim Mathies [:jimm]
roc: review+
Details | Diff | Splinter Review

Description Dão Gottwald [:dao] 2012-06-26 04:39:29 PDT
+++ This bug was initially created as a clone of Bug #684534 +++

We've worked around this at least twice, in bug 684534 and here: http://hg.mozilla.org/mozilla-central/annotate/74e503bfa575/browser/base/content/browser-tabPreviews.js#l134

I think this doesn't affect all panels, but it's not clear yet what's needed to trigger this.
Comment 1 Dão Gottwald [:dao] 2012-06-26 05:36:19 PDT
Created attachment 636670 [details]
testcase
Comment 2 Neil Deakin 2012-06-26 06:01:01 PDT
Don't see any issue on my windows 7 machine with this testcase.
Comment 3 Dão Gottwald [:dao] 2012-06-26 06:13:20 PDT
Do you see the images in the panel?
Comment 4 Neil Deakin 2012-06-26 06:23:29 PDT
Yes, I see the panel opening and closing, alternating between the sun and clouds images. There is no flickering of the previous image when opening.
Comment 5 Dão Gottwald [:dao] 2012-06-26 06:33:35 PDT
Is this on a fast desktop machine? Maybe it depends on the image decoding time or something.

Also, it doesn't flicker every time. With this test case it seems to happen with something like a 50% chance over here.
Comment 6 Timothy Nikkel (:tnikkel) 2012-06-26 15:54:17 PDT
I can reproduce in a Windows XP VM.
Comment 7 Neil Deakin 2012-06-26 17:45:51 PDT
Timothy, do you want to investigate this bug?
Comment 8 Timothy Nikkel (:tnikkel) 2012-06-26 23:27:26 PDT
Sure, I'll take a look.
Comment 9 Timothy Nikkel (:tnikkel) 2012-07-10 23:37:11 PDT
So I think the problem is that transparent windows are drawn differently from opaque ones. Opaque ones we use InvalidateRect, go back to the event loop and get a paint event then we paint. The panel will be blank until this happens. With transparent windows we keep a drawing context in memory around, and I guess it keeps the contents of what was in the panel before it was hidden. We use InvalidateRect to give us a paint event and then once we get back to the event loop and process that resulting paint event we update the contents of that memory drawing context, but until then we Windows uses the old contents to draw to the screen. Does that sound reasonable?

Perhaps we should just draw transparent black to the memory DC when we hide a transparent window so that it is invisible the next time we show it and then shows the new contents when we actually draw them?
Comment 10 Dão Gottwald [:dao] 2012-07-11 00:51:10 PDT
(In reply to Timothy Nikkel (:tn) from comment #9)
> Does that sound reasonable?

At least, it very much matches what I see.

> Perhaps we should just draw transparent black to the memory DC when we hide
> a transparent window so that it is invisible the next time we show it and
> then shows the new contents when we actually draw them?

Sounds good from my front-end perspective.
Comment 11 Jim Mathies [:jimm] 2012-07-11 05:01:07 PDT
Created attachment 641008 [details] [diff] [review]
patch

This patch seems to fix the problem. Pushed to try w/reporting.
Comment 12 Jim Mathies [:jimm] 2012-07-11 13:53:12 PDT
Comment on attachment 641008 [details] [diff] [review]
patch

Clear transparent windows before we hide them to eliminate old content ghosting when they are shown.

This passed try fine.
Comment 13 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-07-11 19:00:38 PDT
Comment on attachment 641008 [details] [diff] [review]
patch

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

r+ with those two lines removed

::: widget/windows/nsWindow.cpp
@@ +7462,5 @@
> +void nsWindow::ClearTranslucentWindow()
> +{
> +  if (mTransparentSurface) {
> +    nsRefPtr<gfxContext> thebesContext = new gfxContext(mTransparentSurface);
> +    thebesContext->ResetClip();

There can't be a clip set on thebesContext yet, so this seems pointless.

@@ +7465,5 @@
> +    nsRefPtr<gfxContext> thebesContext = new gfxContext(mTransparentSurface);
> +    thebesContext->ResetClip();
> +    thebesContext->SetOperator(gfxContext::OPERATOR_CLEAR);
> +    thebesContext->Paint();
> +    thebesContext->SetOperator(gfxContext::OPERATOR_OVER);

This SetOperator is unnecessary since the context is not used again.
Comment 15 Ed Morley [:emorley] 2012-07-12 09:35:47 PDT
https://hg.mozilla.org/mozilla-central/rev/10c3a37563ab

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