GNOME Bugzilla – Bug 108926
Bad tearing during resize
Last modified: 2004-12-22 21:47:04 UTC
The frame tears badly during resize as the soon-to-be-attached screenshot shows. What happens is this: - in meta_frame_sync_to_window, the frame window is resized. Then in meta_ui_repaint_frame() the corresponding foreign GdkWindow has process_updates called on it. But since the window was never invalidated, nothing happens. Instead since the window has a bg=None, an expose is sent and the window is redrawn. Since the bg=None, this doesn't flicker, but until the expose arrives, there is bad tearing. - adding a gdk_window_invalidate(window, NULL) makes most of the window actually be redrawn. However, since the window was resized with X{Move,Resize}Window, Gdk doesn't know the new coordinates before a configure event arrives. This means gdk will not invalidate the newly uncovered area of the window, so this doesn't actually solve anything. - calling gdk_window_move_resize() will not help since the window is not a child window. Gdk always waits for configure events for windows that aren't child windows. - as long as gdk has the wrong idea about what the window dimensions are, this tearing will go on, because gdk clips any drawing to what it thinks is the visible area. I don't know what the solution is. Maybe add a "gdk_window_i_AM_the_window_manager_dammit()" that would make resizing of gdk windows update the cache immediately. Another solution would be to bypass gdk entirely in metacity. A non-solution is to make the frame window a regular gdk toplevel, because resizing such windows will also not update the cached dimensions. I am attaching a patch that makes resizing prettier by making metacity wait for the configure event, but this is a bad hack, and the extra roundtrip here should not be necessary.
Created attachment 15158 [details] bad tearing
Created attachment 15159 [details] [review] patch that makes metacity wait for a configure event
hey owen, can we add a function called that? ;-)
Hmmm ... can you create your "toplevels" as child window? GTK+ assumes child windows resize immediately. They certainly shouldn't get all the extra properties, focus windows, etc, that GTK+ puts on toplevels.
they're created with XCreateWindow(), then wrapped with gdk_window_foreign_new()... I guess we could try creating them with gdk_window_new instead, but I don't know what side effects it might have.
How about gdk_window_foreign_set_is_toplevel ()? something like that?
Created attachment 19252 [details] [review] patch
This patch seems to work, but I haven't tested it very thoroughly. There are some events that GDK doesn't provide for the event mask, so I'm setting it twice, both in ui.c and in frame.c
The patch looks good. In one spot you seem to have reimplemented this: GdkDisplay * gdk_x11_lookup_xdisplay (Display *xdisplay) (from gdkx.h) Also, when creating the GdkWindow for the frame maybe you should just set the event mask to 0; otherwise people might think that event mask matters when in reality we're going to immediately replace it. I wonder if there's some reason I did the "if (move and resize) MoveResize() else if (move) Move() else if (resize) Resize()" thing. Probably not. Conceivably it changes configure events somehow? Anyway I'll apply this patch and try using it for a while and if it doesn't break let's put it in.
I don't think you can set the event mask for the GdkWindow to 0, because GDK depends on knowing it in some places. So I did it this to let GDK know as much as possible about the real event mask. But a comment is probably in order. I'll attach a new patch.
Created attachment 19262 [details] [review] new patch
This works for me, do you want to commit? I have it applied to my tree too if you want me to commit. Thanks
It's easier for me if you commit as I have some other experiments in my tree currently.
Committed. I removed the change of max resizes per second (20->200) since it seemed unrelated; if you think that's a good idea in general maybe we can discuss separately. I wanted to put a cap on CPU usage from opaque resize so people wouldn't whine about it on timeshare systems.
Right, the 20->200 was just a leftover from something else.
This patch breaks metacity on AMD64, or at least exhibits another problem more clearly. i.e. Now with 2.6.1, resizing gnome-terminal windows (or rxvt) makes it inoperant, or at least metacity thinks we request a 0x0 change.
How do you know it was this patch? (If you debugged the problem, can you tell us what it was? ;-))
Problem is not entirely debugged, unfortunately :( Reverting to metacity just before this patch was checked in (thanks CVS) so no problem but applying this patch breaks load of width_inc and height_inc by metacity.. It is not clear yet why metacity doesn't load these values correctly on AMD64 while xprop does..
CVS dichotomy. ;-) I can only speak about effects of the bug, not the cause since it's yet to be found: - Metacity 2.4.1 (the one from MDK 9.0/AMD64) works correctly wrt. windows resizing. - Metacity 2.6.1 makes gnome-terminal (and rxvt, aka. resizable windows under constraints) not resizing correctly. - It may be that patch triggered a latent bug. - SizeHints look totally bogus. It may be that due to some gdk use, those values are nuked somehow. - I do think of a 32-bit/64-bit arg mismatch either as a vararg or as an integral constant passed as the >= 7th argument. However, I looked at longish metacity functions, they seem to be called correctly. Reading gcc warnings only lead me to another 64-bit bug in delete.c but that doesn't solve this problem. :-(
Created attachment 20272 [details] [review] 64-bit fixes
There was a mismatch between the 32-bit values returned through XGetAsyncData(), and 64-bit (long) that were expected from xPropSizeHints et al. You can optimize it further for 32-bit with proper #if SIZEOF_LONG == 8/#endif, if you check for them in configury scripts of course. Oh, I think X defines LONG64 too.
I think the problem you're patching is bug #114035, which isn't the same as this original bug report (the reopen was not right I think, as the change just happened to expose #114035, it didn't introduce it). Let's further track this issue on bug #114035