768400 - Re-opening a transparent panel sometimes renders outdated content for a split second

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Dão Gottwald [:dao]]

+++ 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]]

Attachment: testcase

Comment 2

[Neil Deakin]

Don't see any issue on my windows 7 machine with this testcase.

Comment 3

[Dão Gottwald [:dao]]

Do you see the images in the panel?

Comment 4

[Neil Deakin]

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]]

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)]

I can reproduce in a Windows XP VM.

Comment 7

[Neil Deakin]

Timothy, do you want to investigate this bug?

Comment 8

[Timothy Nikkel (:tnikkel)]

Sure, I'll take a look.

Comment 9

[Timothy Nikkel (:tnikkel)]

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]]

(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]]

Attachment: patch

This patch seems to fix the problem. Pushed to try w/reporting.

Comment 12

[Jim Mathies [:jimm]]

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)]

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 14

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/10c3a37563ab

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/10c3a37563ab