After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 629923 - Consider always calling unmap() when unsetting MAPPED flag
Consider always calling unmap() when unsetting MAPPED flag
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.91.x
Other All
: Normal normal
: 3.0
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2010-09-17 14:34 UTC by Havoc Pennington
Modified: 2010-12-20 18:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Verify GtkWidget invariants if G_ENABLE_DEBUG is defined. (13.19 KB, patch)
2010-09-29 02:09 UTC, Havoc Pennington
none Details | Review
Add invariant that a child is unmapped if parent is unmapped (4.15 KB, patch)
2010-09-29 02:09 UTC, Havoc Pennington
none Details | Review
Always emit unmap when a widget is unmapped (1.79 KB, patch)
2010-09-29 02:09 UTC, Havoc Pennington
none Details | Review

Description Havoc Pennington 2010-09-17 14:34:00 UTC
bug 319607 has some discussion that we deliberately skip the unmap() vfunc when unrealizing. This was apparently a performance optimization (link to prior discussion welcome). However, it makes it pretty annoying to e.g. have a timeout that's added only when a widget is mapped, or do anything else only when mapped.
For example animations should stop if you gtk_widget_hide() a widget.

Creating this bug to track discussion.
Comment 1 Havoc Pennington 2010-09-17 15:31:09 UTC
The performance concern was apparently sending Unmap X requests to all subwindows; in GTK 3 there are no server-side subwindows (other than an occasional GL widget or X embed) so this doesn't seem like an issue. 
(I find it hard to imagine it was much of an issue anyway unless there were syncs or at least flushes involved, and those should be possible to eliminate)
Comment 2 Christian Persch 2010-09-17 18:57:49 UTC
I think the previous discussion was bug 613302.
Comment 3 Havoc Pennington 2010-09-29 02:09:26 UTC
Created attachment 171320 [details] [review]
Verify GtkWidget invariants if G_ENABLE_DEBUG is defined.

These checks are a bit expensive so require --enable-debug=yes.
gtk_widget_verify_invariants() checks invariants mentioned
in docs/widget_system.txt in particular, and can verify
others in the future.

Some of the invariants in docs/widget_system.txt don't
in fact hold right now, so those are #if 0'd in this
patch pending someone fixing either the docs or the code.
Comment 4 Havoc Pennington 2010-09-29 02:09:30 UTC
Created attachment 171321 [details] [review]
Add invariant that a child is unmapped if parent is unmapped

Requires fixes to GtkContainer and GtkWindow to unmap their
children, rather than just withdrawing or hiding the container
window.

Requires fix to GtkHandleBox to chain up to GtkContainer unmap.

Historically we avoided these unmaps for efficiency reasons,
but these days it's a bigger problem that there's no way
for child widgets to know that one of their ancestors has
become unmapped.
Comment 5 Havoc Pennington 2010-09-29 02:09:33 UTC
Created attachment 171322 [details] [review]
Always emit unmap when a widget is unmapped

Previously, for performance reasons we would sometimes
skip invoking the unmap signal (and associated vfunc)
in favor of simply unrealizing. However, widgets then
had no way to clean stuff up when they were hidden
(but still inside a parent which was shown).

This patch also removes _gtk_tooltip_hide() which
was done in both unmap and unrealize in gtkwidget.c,
now can only be in unmap.

There are probably lots of things cleaned up in
unrealize that would now be better to move to unmap.
Comment 6 Havoc Pennington 2010-12-10 21:11:31 UTC
I'm not sure where you are on release freezes, but would it make sense to land these patches?
Comment 7 Matthias Clasen 2010-12-13 15:05:40 UTC
Yes, I think we should.
Comment 8 Claudio Saavedra 2010-12-13 16:37:37 UTC
hm, when I proposed this you didn't like it. Good to see Havoc succeeded :)

Anyway, just wanted to point out that the docs introduced in https://bugzilla.gnome.org/show_bug.cgi?id=613302#c11 can be removed if this lands, I think.