GNOME Bugzilla – Bug 426246
"Spurious" expose events during asynchronous GtkWindow resizing
Last modified: 2016-05-19 06:36:22 UTC
When hacking on the new tooltips code, I discovered that expose events are emitted for a GtkWindow when the allocation of the GtkWindow is not the same as the current size of that window. These events are emitted in between the actual resize of the GtkWindow and the size allocation of the window. (For the record, the old tooltips code does not have a problem with this, since it is using the values from the requisition to draw the background in the expose event. The new code uses the allocation, and drawing artifacts appeared). I found two approaches for fixing this problem, and will attach two patches. None of these two patches is supposed to be ready-to-commit. A. Discard any expose event when the window is waiting on a configure event (this spans the time between the window resize and the size allocation). This was actually working right in 1.2.10, but probably broke somewhere in the 2.x process. The comments in gtk_window_move_resize() actually mention that expose events should be discarded in between window resize and reception of the new configure event. B. For "override redirect" windows, directly update the allocation and call size_allocate in gtk_window_move_resize(). This will avoid waiting for the configure event to arrive and gtk_window_move_resized() to be called for the second time. The code does not change for any other window type.
Created attachment 85802 [details] [review] Approach A: discard expose events
Created attachment 85803 [details] [review] Approach B: Directly size allocate for override redirect windows
Comment on attachment 85802 [details] [review] Approach A: discard expose events > >Index: gtkwidget.c >=================================================================== >--- gtkwidget.c (revision 17566) >+++ gtkwidget.c (working copy) >@@ -4119,6 +4119,17 @@ gtk_widget_event_internal (GtkWidget *wi > signal_num = CLIENT_EVENT; > break; > case GDK_EXPOSE: >+ /* Discard exposes if the window is waiting on a configure >+ * event. (See gtk_window_move_resize() for more details.) >+ */ >+ if (!event->any.send_event >+ && GTK_IS_WINDOW (widget) >+ && GTK_WINDOW (widget)->configure_request_count) >+ { >+ gtk_widget_unref (widget); >+ return TRUE; >+ } >+ > signal_num = EXPOSE_EVENT; > break; hm, not sure i really like this window specific hackery in gtkwidget.c. also, some code wrg XCopyArea and no-expose events depends on receiving all expose notifications and evaluates expose->count. it's conceivable that such code could be silently broken by discarding "random" expose events. i.e. maybe discarding expose events at all is something that should be avoid to not break existing application logic (subtly even).
(In reply to comment #0) > B. For "override redirect" windows, directly update the allocation and call > size_allocate in gtk_window_move_resize(). This will avoid waiting for the > configure event to arrive and gtk_window_move_resized() to be called for the > second time. The code does not change for any other window type. the whole allocaiton resizing logic is non trivial of course, partly due to the X model which makes this an asyncronous process that can be arbitrarily intercepted and affected by windoiw managers. so not all effects of changes can be perfectly foreseen for all scenarios. still i think appling the logic described in B for *all* window types would be the best thing to do, because for >99% of the cases, we'll get the new requested allocation anyway (for all window types, also window manager managed windows). thus it doesn't make much sense to leave the intermediate code (the between move-resize and configure-event handling of an expose event) operate on the assumption that the old allocaiton is still valid after we *know* we requested it to change. so we can just as well re-allocate syncronously with move_resize, and if in a later configure-event we indeed discover an unexpected window size we can still re-allocate to fix up matters (remember that this is expected to occour comparatively rarely).
If we go with approach B for managed windows, then the result on a change to window contents that makes them get bigger is going to be: - The contents of the window get bigger and go outside the window bounds - The window itself gets larger - The remainder of the contents are drawn A WM/CM can be smart enough to prevent the user from seeing this since it gets the resize request before it gets it gets the damage requests, but even that case there is significant inefficiency involved since we are drawing things in two pieces instead of one, so we have twice the setup costs. Approach B) for only override-redirect windows makes sense to me; there are a couple of small issues with the patch: - No need to assign directly to widget->allocation - The gtk_widget_queue_resize (widget) / _gtk_container_queue_resize (container) calls shouldn't be there wen are just going ahead directly and doing the allocation. That presumably leaves the same problem for managed windows; approach A) would be a reasonable thing to do to fix that, if a little more work to implement. gdk_window_freeze_updates() is similar to what is needed, with a few caveats: - You'd have to be able to do it for a whole window heirarchy - You'd have to get the case of windows being reparented into or out of that hierarchy right (corner case, but broken is broke...) This implies you might want a "and-descendents freeze count" instead of recursing and freezing all the descendants P.S. - I'm a little puzzled by you seeing drawing artifacts from this, however. ... unless you call gtk_widget_set_redraw_on_allocate(widget, FALSE) explicitly somewhere, shouldn't everything get invalidated again when the new size comes in?
(In reply to comment #5) > Approach B) for only override-redirect windows makes sense to me; there > are a couple of small issues with the patch: > > - No need to assign directly to widget->allocation > - The gtk_widget_queue_resize (widget) / > _gtk_container_queue_resize (container) calls shouldn't be there wen > are just going ahead directly and doing the allocation. I made these changes and can confirm the patch is still functioning properly. > P.S. - I'm a little puzzled by you seeing drawing artifacts from this, however. > ... unless you call gtk_widget_set_redraw_on_allocate(widget, FALSE) > explicitly somewhere, shouldn't everything get invalidated again when > the new size comes in? With the changes above applied the drawing artifacts came back. Since the requisitions and allocations are correct now, the artifacts should be caused by something else. For a reason I don't know yet most exposes the tooltip windows receive don't cover the full area but only (x+2,y+2) w-2 x h-2.
(In reply to comment #5) > P.S. - I'm a little puzzled by you seeing drawing artifacts from this, however. > ... unless you call gtk_widget_set_redraw_on_allocate(widget, FALSE) > explicitly somewhere, shouldn't everything get invalidated again when > the new size comes in? Little update on the matter; the weird expose I was seeing in comment #6 is triggered by a label which is being hidden and invalidates its area. It doesn't look like everything gets invalidated when the new size comes in.
Putting on the 2.12 freeze milestone, because having this in before 2.12.0 is a must for tooltips.
Created attachment 91174 [details] [review] A for managed windows, B for override redirect Is this a step in the right direction? For managed windows I added a function like gdk_window_freeze_updates() which will freeze updates for a toplevel GdkWindow and all its descendants. I could verify that this approach does successfully remove a mismatched allocation/expose for the toplevel window. I don't exactly know how the properly test the corner case you described.
Owen, can you comment on this ?
Looks quite good to me: - I'd be a more comfortable with the compatibility implications of the GdkWindowObject change if you added the field to the end of the structure. (You can't externally derive from GdkWindowObject, so that should be sufficent) - In gdk_window_freeze_toplevel_updates g_return_if_fail (private->window_type == GDK_WINDOW_TOPLEVEL); Is I think too restrictive... != GDK_WINDOW_CHILD I think is better. - New public functions need docs - Comments explaining why we special-case POPUP and why we freeze updates for other windows would be good. That's all I really see. Testing on Win32/OS X before doing a release with this patch would be good.
Oh, yeah, and I wouldn't worry about testing the corner case .. it was part of an argument for doing it this way rather than walking the tree and recursively freezing all children. Hmm, there *is* a small bug in the unfreeze logic: If the toplevel is frozen *and* toplevel-frozen And: The children are not frozen Then when the toplevel is toplevel-unfrozen an update won't be scheduled for the children. Might be good to just always force scheduling of the update idle after a toplevel-unfreeze.
Created attachment 95458 [details] [review] updated patch Updated patch. Marked the new API as libgtk_only for now, we can move this to public in 2.14. Tested on OSX and works fine there too, don't have a gtk+ build for Windows here ... The only thing I didn't really have a clue about are the comments in gtkwindow.c why we are doing things this way ...
Committed r18802.
Bratsche, can you do the windows testing here please?