Closed Bug 69346 Opened 24 years ago Closed 24 years ago

Save-unders for transient windows in X11 don't work properly

Categories

(Core :: Web Painting, defect)

Product:
Core
Component:
Web Painting
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9

People

(Reporter: roc, Assigned: roc)

References

Details

Attachments

(1 file)

View manager changes to fix this bug
5.29 KB, patch
Details | Diff | Splinter Review 
In the old view manager we explicitly repainted underlying views when a view
with a transient pop-up window (e.g. menu or combobox) went away. This is, of
course, expensive and dumb. So I got rid of that in the new view manager.

Unfortunately, experiments seem to show that in the X/GTK toolkit, it doesn't
work right. Save-unders work OK most of the time, we never get a repaint event,
and menus are noticeably more responsive. But if we invalidate one of the
underlying widgets while the transient is showing, then that part of the
save-under buffer is not restored, *and* we don't get an expose event for that
area. The result is that those bits are not updated and we see garbage
consisting of whatever the transient window was showing in that area.

You can reproduce this reliably by turning on the new view manager, going to the
Preferences dialog and the Appearance | Colors pane, then clicking on the
buttons that pop up the color picker. When you dismiss the color picker some of
the widgets (ones that are updated because of focus change or something else
that happens when you activate the color picker) are not repainted.

Presumably we're supposed to get an expose event for at least the invalid areas
when the transient goes away. We do in Windows.
I can't reproduce this here.  When the color picker goes away I don't see any
damage.  If I open menus on top of loading pages where there will be drawing
things look fine.  I can't see any damage.  I know that I have your view manager
on too since I'm getting all of those XXX messages.

Thinking about this I don't see how it could happen anyway.  Assuming that save
unders are turned on for my transients ( I'm not sure if that's a WM thing or
not ) paints would still be painted underneath the transient.  If save-under is
on then you will not get an expose.  That's why you have save-under on in the
first place :)

When an area is damaged under a transient, we have to get a paint event for that 
area sooner or later. (How else can the X server know what to draw there when 
the transient is no longer showing?) Can you describe what is supposed to 
happen in this case?
It all depends on whether or not save unders for that transient is set.

In the case where the transient window does have save unders turned on it's like
having a backing store turned on temporarily for the window underneath it.  The
X server stores the information about what's underneath the window and will
automatically fill it in when the window is removed.

From the standpoint of someone like you and me writing an application it's like
the window was never obscured.  You just draw like normal and you don't get
expose events since you don't have to fill in those areas by hand.
OK, but we only draw in response to paint events received by the view manager. 
So if the X server doesn't generate expose events for this case, what is 
supposed to trigger the paint event received by the view manager?
The point is that it's not supposed to be damaged at all so you don't need to
repaint it.
OK, I'm obviously not making myself clear. The problem occurs when we, i.e. 
Mozilla code, invalidate a widget because it needs to be repainted because its 
old pixels are out of date. For example, we removed focus from a XUL control, so 
we need it to be repainted. But it's currently covered by a transient. So what 
happens?
Oh, now I get it.  In that case the paint should take place like the transient
wasn't even there.
Aaaaah. OK, I found the problem. When that repaint comes in, the view manager is 
painting everything that covers the area, including the floating views! I have a 
patch, but it depends on the patch for bug 69146 which has yet to be checked in. 
Adding the dependency.
Assignee: blizzard → roc+moz
Depends on: 69146
My bug. PS, this didn't show up on Windows because Windows issues a new repaint 
after the transient has closed.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9
The patch makes sure we only paint floaters if we're painting the floating
widget. So when we paint behind a transient, we paint the contents of the window
behind the transient and none of the transient's own content. The bug is gone.
May I prevail upon Kevin and Marc to give this their blessing? :-)
How often are widgets not visible?  If it's decently frequent, it might be 
worth testing |visible| before getting the view and checking for floating.

"Performance: every little bit counts."

Marc, can you please sr this? I will address Shaver's comment when I fix bug 
70446. Thanks.
sr=attinasi
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Marking verified per last comments
Status: RESOLVED → VERIFIED
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: