Closed Bug 562138 Opened 14 years ago Closed 13 years ago

Ugly white flash before (colored) opaque popup window (e.g. <select> dropdown) appears (GL: flash of previous contents)

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set
minor

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- .x+

People

(Reporter: jruderman, Assigned: ehsan.akhgari)

References

Details

(Keywords: testcase, Whiteboard: [softblocker][has patch])

Attachments

(2 files, 2 obsolete files)

Attached file testcase
If you open and close the dropdown in the testcase five times, you'll see a white flash at least once or twice.

Bug 301160 might be related, but it's harder to reproduce.
This affects all opaque popup windows. Another example is the all-tabs panel. This is because nsCocoaWindow::Show for popup windows makes the window visible before it makes the contentView visible. Making NSWindows visible is synchronous but for NSViews it's not.
But we can't make the NSView visible before making the window visible because that would repaint it synchronously, and showing popup windows happens during reflow where painting isn't allowed.
Transparent windows don't have this problem because you can't see a transparent flash.

We can probably make all popup NSWindows transparent and only their NSView opaque.
Summary: Ugly white flash before (colored) <select> dropdown appears → Ugly white flash before (colored) opaque popup window (e.g. <select> dropdown) appears
(In reply to comment #1)
> But we can't make the NSView visible before making the window visible because
> that would repaint it synchronously, and showing popup windows happens during
> reflow where painting isn't allowed.

But that's something we can change: we just need to tell nsView that it shouldn't call mWidget->Show while refresh is disabled (e.g. during reflow).

---

This bug affects GL layers in a slightly different way: Instead of a white flash, what you briefly see are the popup's previous contents. This is the cause of bug 609694.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Summary: Ugly white flash before (colored) opaque popup window (e.g. <select> dropdown) appears → Ugly white flash before (colored) opaque popup window (e.g. <select> dropdown) appears (GL: flash of previous contents)
Blocks: 609694
This patch makes visibility changes to popups work the same way as size and position changes - synchronous if refresh is enabled, deferred otherwise. Now visibility changes are allowed to cause synchronous painting, just like size changes always were.
Attachment #503487 - Flags: review?(roc)
This makes a popup's content NSView visible before showing the NSWindow. This causes it to be drawn during the first NSWindow paint (synchronously, inside orderFront:).
A small workaround is necessary to make IsVisible return true during window showing.
Attachment #503488 - Flags: review?(joshmoz)
blocking2.0: --- → ?
blocking2.0: ? → final+
Whiteboard: [softblocker]
Attachment #503488 - Flags: review?(joshmoz) → review+
Yes please!
http://hg.mozilla.org/mozilla-central/rev/44c42d8b8d08
http://hg.mozilla.org/mozilla-central/rev/f948c56cb86f
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
Depends on: 627824
Looks like the patch to nsView.cpp caused bug 628861
Blocks: 629004
No longer blocks: 629004
Depends on: 629004
This also seems to have caused or worsened bug 627996, which should be dealt with before this relands
Assignee: mstange → ehsan
(In reply to comment #1)
> We can probably make all popup NSWindows transparent and only their NSView
> opaque.

This patch implements this idea.  It seems to work great for me.  This is my first time writing cocoa code (or objective-C code for that matter!), so handle this patch with the appropriate level of care.  :-)
Attachment #503487 - Attachment is obsolete: true
Attachment #503488 - Attachment is obsolete: true
Attachment #514584 - Flags: review?(joshmoz)
Whiteboard: [softblocker] → [softblocker][needs review]
Status: REOPENED → ASSIGNED
Whiteboard: [softblocker][needs review] → [softblocker][has patch][needs review josh]
** PRODUCT DRIVERS PLEASE NOTE **

This bug is one of 7 automatically changed from blocking2.0:final+ to blocking2.0:.x during the endgame of Firefox 4 for the following reasons:

 - it was marked as a soft blocking issue without a requirement for beta coverage
blocking2.0: final+ → .x+
(In reply to comment #10)
> Created attachment 514584 [details] [diff] [review]
> Make NSWindow's transparent
> 
> (In reply to comment #1)
> > We can probably make all popup NSWindows transparent and only their NSView
> > opaque.
> 
> This patch implements this idea.  It seems to work great for me.

Indeed, it works for <select> popups!
Unfortunately it doesn't work for other types of popup windows (created by nsMenuPopupFrame), since those have their window transparency overwritten by a call to SetTransparencyMode before the window is shown.

Making this approach work correctly with SetTransparencyMode is a bit more work, which is the reason that I shied away from it originally. I wouldn't recommend going that route since it's pretty workaround-heavy. Debugging the regressions of the original patch would probably be more advisable.

But we could leave the opaque XUL popup issue to another bug and just land your patch to fix <select>.
(In reply to comment #12)
> (In reply to comment #10)
> > Created attachment 514584 [details] [diff] [review]
> > Make NSWindow's transparent
> > 
> > (In reply to comment #1)
> > > We can probably make all popup NSWindows transparent and only their NSView
> > > opaque.
> > 
> > This patch implements this idea.  It seems to work great for me.
> 
> Indeed, it works for <select> popups!
> Unfortunately it doesn't work for other types of popup windows (created by
> nsMenuPopupFrame), since those have their window transparency overwritten by a
> call to SetTransparencyMode before the window is shown.
> 
> Making this approach work correctly with SetTransparencyMode is a bit more
> work, which is the reason that I shied away from it originally. I wouldn't
> recommend going that route since it's pretty workaround-heavy. Debugging the
> regressions of the original patch would probably be more advisable.
> 
> But we could leave the opaque XUL popup issue to another bug and just land your
> patch to fix <select>.

Do we have instances of this bug showing for things other than <select> popups?
(In reply to comment #13)
> Do we have instances of this bug showing for things other than <select> popups?

I see it with the awesomebar dropdown, for example. It's not really visible as a white flash because it happens so quickly and the contrast to the real dropdown contents is fairly low, but you can still feel that there's something wrong when it appears.
(In reply to comment #13)
> > But we could leave the opaque XUL popup issue to another bug and just land your
> > patch to fix <select>.

So, should we do this?  :-)
josh: ping?
Comment on attachment 514584 [details] [diff] [review]
Make NSWindow's transparent

I'm fine with trying this but please 1) file another bug on the problem mstange described in comment 12 and comment 14 and 2) keep an eye on perf numbers when this lands, just in case something we don't expect goes wrong.

Thanks for the patch!
Attachment #514584 - Flags: review?(joshmoz) → review+
Whiteboard: [softblocker][has patch][needs review josh] → [softblocker][has patch]
(In reply to comment #17)
> I'm fine with trying this but please 1) file another bug on the problem mstange
> described in comment 12 and comment 14 and 2) keep an eye on perf numbers when
> this lands, just in case something we don't expect goes wrong.

Filed bug 648711.
Pushed http://hg.mozilla.org/mozilla-central/rev/ed6f4884f03d
Status: ASSIGNED → RESOLVED
Closed: 14 years ago13 years ago
Resolution: --- → FIXED
Depends on: 677279
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: