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

RESOLVED FIXED in mozilla16

Status

()

Core
Widget: Win32
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dao, Assigned: jimm)

Tracking

({testcase})

Trunk
mozilla16
x86
Windows 7
testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
+++ 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.
(Reporter)

Comment 1

5 years ago
Created attachment 636670 [details]
testcase
(Reporter)

Updated

5 years ago
Keywords: testcase-wanted → testcase
Don't see any issue on my windows 7 machine with this testcase.
(Reporter)

Comment 3

5 years ago
Do you see the images in the panel?
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.
(Reporter)

Comment 5

5 years ago
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.
I can reproduce in a Windows XP VM.
Timothy, do you want to investigate this bug?
Sure, I'll take a look.
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?
Component: XUL → Widget: Win32
(Reporter)

Comment 10

5 years ago
(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.
Summary: Re-opening a panel sometimes renders outdated content for a split second → Re-opening a transparent panel sometimes renders outdated content for a split second
(Assignee)

Comment 11

5 years ago
Created attachment 641008 [details] [diff] [review]
patch

This patch seems to fix the problem. Pushed to try w/reporting.
Assignee: nobody → jmathies
(Assignee)

Comment 12

5 years ago
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.
Attachment #641008 - Flags: review?(roc)
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.
Attachment #641008 - Flags: review?(roc) → review+
(Assignee)

Comment 14

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/10c3a37563ab
https://hg.mozilla.org/mozilla-central/rev/10c3a37563ab
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
(Reporter)

Updated

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