Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Window resize loop #532

Closed
totaam opened this issue Mar 12, 2014 · 28 comments
Closed

Window resize loop #532

totaam opened this issue Mar 12, 2014 · 28 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Mar 12, 2014

Issue migrated from trac ticket # 532

component: server | priority: major | resolution: invalid | keywords: loop

2014-03-12 08:44:36: mptei created the issue


The window of handbrake (ghb) sometimes creates an endless loop of size change events.
After that usually the xpra server must be hard killed.

2014-03-12 08:02:05,978 resize_corral_window() corral window (1030x768) does not match client window (1024x768), resizing it
2014-03-12 08:02:05,987 resize_corral_window() corral window (1024x768) does not match client window (1030x768), resizing it
2014-03-12 08:02:06,016 resize_corral_window() corral window (1030x768) does not match client window (1024x768), resizing it
2014-03-12 08:02:06,046 resize_corral_window() corral window (1024x768) does not match client window (1030x768), resizing it
...
@totaam
Copy link
Collaborator Author

totaam commented Mar 12, 2014

2014-03-12 08:45:10: mptei uploaded file resizeloop1.log.bz2 (111.0 KiB)

Server log with -d all

@totaam
Copy link
Collaborator Author

totaam commented Mar 12, 2014

2014-03-12 09:08:29: totaam uploaded file resize-via-idle_add.patch (0.9 KiB)

I seem to remember that we were resizing via idle_add to prevent races before

@totaam
Copy link
Collaborator Author

totaam commented Mar 12, 2014

2014-03-12 09:09:18: totaam changed owner from antoine to mptei

@totaam
Copy link
Collaborator Author

totaam commented Mar 12, 2014

2014-03-12 09:09:18: totaam commented


Thanks a lot for the details.

There is a at least one issue fixed in r5743, but there could well be more.
Can you run with that patch and see if you still get resize_corral_window storms?
And if you do, does resize-via-idle_add.patch help?

@totaam
Copy link
Collaborator Author

totaam commented Mar 12, 2014

2014-03-12 10:23:23: mptei commented


Thanks for the hints.

r5743 doesn't change anything.
resize-via-idle_add.patch makes the "does not match client window"-messages completely go away, but the behavior stays the same.

There is still an endless resizing. You see it when you grep for "window_resized_signaled" in resizeloop2.log

@totaam
Copy link
Collaborator Author

totaam commented Mar 12, 2014

2014-03-12 10:24:14: mptei uploaded file resizeloop2.log.bz2 (107.7 KiB)

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 07:32:47: totaam uploaded file resize-from-idle.patch (1.2 KiB)

do all the resize work from idle_add

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 07:34:22: totaam commented


Sorry, I don't have handbrake installed to test... Will try it in a VM eventually.

Does the [/attachment/ticket/532/resize-from-idle.patch] make any difference?
The fact that does not match client window had gone away was surprising.

If not, does taking out this line help:

    BaseWindowModel.do_xpra_configure_event(self, event)

What is your window manager client side? Does it matter?

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 07:56:59: mptei commented


The new patch also doesn't work.

The problem appeared every time when using the osx client, for a long time (since about at least 2874). I'm not sure, but I think it started when changing the window size. On osx I see a flicker of the window on the right side (due to still changing the size).

Using a linux client I can't remember to have that problem before. But this time it seems the window has a size that it happens regularly even on linux. I can't see a flicker. It just stops refreshing.

On client side this log appears multiple times (also without the patch):

2014-03-13 08:42:06,120 with hints={'max_height': 768, 'min_height': 747, 'base_width': 4294967295, 'max_width': 1024, 'min_width': 1030, 'base_height': 4294967295}
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/xpra/client/client_window_base.py", line 174, in set_metadata
    self.apply_geometry_hints(hints)
  File "/usr/lib64/python2.7/site-packages/xpra/client/gtk2/gtk2_window_base.py", line 160, in apply_geometry_hints
    self.set_geometry_hints(None, **hints)
OverflowError: signed integer is greater than maximum

Commenting out line 1074 of src/xpra/x11/gtk_x11/window.py also doesn't change anything.

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 08:14:16: totaam commented


OH!

r5758 fixes a long standing bug: we failed to re-sanitize window size hints when they changed.

It looks like handbrake sets some invalid values there (as can be seen in the log you posted):

  • the base_width and base_height should be unset (via the corresponding flag) rather than set to -1 (-1==4294967295 for unsigned 32-bit integers)
  • the min_width is greater than the max_width, no wonder things struggle to settle on a valid size! (the sanitize code should emit a warning)

Does this help?

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 08:23:34: mptei commented


Pure r5759 doesn't have the client error anymore, but the problem remains.

Testing with resize-from-idle.patch ...

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 08:28:28: totaam commented


Please try also:

Note: with or without those patches, the server should emit the invalid size warning

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 08:31:39: totaam commented


I suspect r5760 may well work because we will end up choosing a size which suits handbrake (1024x768). If so, this is just luck. We should not have to guess how to fix invalid size hints...

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 08:42:46: mptei commented


The patch on r5759 also didn't work.

Pure r5760 still has the problem.

I'm doing still the same steps in handbrake. Even on the same software it sometimes takes longer until it happens. On the client I see only that refreshing stops.

The client show this messages only:

2014-03-13 09:35:19,126 TrayBacking does not have any pixel data!
2014-03-13 09:35:19,127 TrayBacking does not have any pixel data!
2014-03-13 09:37:30,082 Connection lost

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 12:33:51: mptei commented


Don't know if this is important, but the toggle loop now looks like this. It toggles the width between 1024 and 1030. It goes so fast, that I think this runs on server side only. I mean the client is not part of that loop.

2014-03-13 09:29:08,348 WindowModel.do_xpra_configure_event(<X11Event {'delivered_to': <gtk.gdk.Window object at 0x390ae10 (GdkWindow at 0x366fc60)>, 'send_event': 0, 'height': 768, 'width': 1024, 'window': <gtk.gdk.Window object at 0x390ae10 (GdkWindow at 0x366fc60)>, 'y': 0, 'x': 0, 'serial': 17986L, 'border_width': 0, 'type': 22, 'display': <gtk.gdk.Display object at 0x28dcdc0 (GdkDisplayX11 at 0x2985210)>}>)
2014-03-13 09:29:08,348 invalidating named pixmap
2014-03-13 09:29:08,348 x_event_filter event=('xpra-configure-event', None)/ConfigureNotify took 0.3ms
2014-03-13 09:29:08,349 x_event_filter event=('xpra-property-notify-event', None)/PropertyNotify window=0x800023
2014-03-13 09:29:08,349 Property changed on 8388643: _NET_WM_OPAQUE_REGION
2014-03-13 09:29:08,350 x_event_filter event=('xpra-property-notify-event', None)/PropertyNotify took 0.4ms
2014-03-13 09:29:08,350 x_event_filter event=('xpra-configure-event', None)/ConfigureNotify window=0x800023
2014-03-13 09:29:08,351 WindowModel.do_xpra_configure_event(<X11Event {'delivered_to': <gtk.gdk.Window object at 0x390ae10 (GdkWindow at 0x366fc60)>, 'send_event': 0, 'height': 768, 'width': 1030, 'window': <gtk.gdk.Window object at 0x390ae10 (GdkWindow at 0x366fc60)>, 'y': 0, 'x': 0, 'serial': 17988L, 'border_width': 0, 'type': 22, 'display': <gtk.gdk.Display object at 0x28dcdc0 (GdkDisplayX11 at 0x2985210)>}>)
2014-03-13 09:29:08,351 invalidating named pixmap
2014-03-13 09:29:08,351 x_event_filter event=('xpra-configure-event', None)/ConfigureNotify took 1.6ms
2014-03-13 09:29:08,352 x_event_filter event=('xpra-property-notify-event', None)/PropertyNotify window=0x800023
2014-03-13 09:29:08,353 Property changed on 8388643: _NET_WM_OPAQUE_REGION
2014-03-13 09:29:08,353 x_event_filter event=('xpra-property-notify-event', None)/PropertyNotify took 0.4ms
2014-03-13 09:29:08,353 x_event_filter event=('xpra-configure-event', None)/ConfigureNotify window=0x800023
2014-03-13 09:29:08,354 WindowModel.do_xpra_configure_event(<X11Event {'delivered_to': <gtk.gdk.Window object at 0x390ae10 (GdkWindow at 0x366fc60)>, 'send_event': 0, 'height': 768, 'width': 1024, 'window': <gtk.gdk.Window object at 0x390ae10 (GdkWindow at 0x366fc60)>, 'y': 0, 'x': 0, 'serial': 17992L, 'border_width': 0, 'type': 22, 'display': <gtk.gdk.Display object at 0x28dcdc0 (GdkDisplayX11 at 0x2985210)>}>)

@totaam
Copy link
Collaborator Author

totaam commented Mar 13, 2014

2014-03-13 12:43:58: totaam commented


I have handbrake in a virtual machine and I can reproduce the bug.

This is all to do with the invalid min and max size we get..
With r5765 + r5766 + r5767, you can still get a bit of a resizing frenzy if you try to resize the handbrake window (we tell the window we want it a certain size, it then tells us it wants another), but it eventually settles down without getting the server into a spin.

Does this work for you? Can I close this ticket?

@totaam
Copy link
Collaborator Author

totaam commented Mar 14, 2014

2014-03-14 04:37:37: mptei commented


Sorry that doesn't work for me. It still runs that toggle loop.

I've tried handbrake on a plain X. There I can see that the window maintains a width of 1024. It's not possible to change that. Even on resizing it's not possible to get another width temporarily. In wireshark I see only configuration requests to the X server with that width.

It seems that handbrake wants a fixed width of 1024 but xpra still offers a width of 1030. Why is it so? Are these 6 Pixels for the border?

Thank you for your effort in this ticket. But can you please leave that ticket open and assigned to me? I would like to take a deeper look into the code. Perhaps I find a solution.

@totaam
Copy link
Collaborator Author

totaam commented Mar 14, 2014

2014-03-14 05:22:11: totaam uploaded file log-minmax-windowsize.patch (0.7 KiB)

shows what min and max size the application sends

@totaam
Copy link
Collaborator Author

totaam commented Mar 14, 2014

2014-03-14 05:37:53: totaam uploaded file handbrake-ugly-workaround.patch (0.8 KiB)

fix for handbrake only (sigh)

@totaam
Copy link
Collaborator Author

totaam commented Mar 14, 2014

2014-03-14 05:39:30: totaam commented


As explained in previous comments, handbrake sends invalid min and max window size attributes. You can apply [/attachment/ticket/532/log-minmax-windowsize.patch] and you will see for yourself:

got max size hint: (1024, 768)
got min size hint: (1030, 690)

The reference documentation for WM_SIZE_HINTS can be found here: wm-normal-hints.html

Xpra only tries to make sense of those nonsensical values (see r5759 and r5760) so that it can give something meaningful to the client (osx in your case). When the client resizes the window, we tell handbrake to resize, and since it does not expect it... bad things happen.

[/attachment/ticket/532/handbrake-ugly-workaround.patch] probably does what you want, but I am a bit reluctant to apply this because it is a very arbitrary way of sanitizing those nonsensical values, and in many ways inferior to the current code.

@totaam
Copy link
Collaborator Author

totaam commented Mar 14, 2014

2014-03-14 10:46:28: totaam commented


I couldn't just let this rot without at least a workaround for some of the issues I found with handbrake, so:

  • r5783 and r5784 improve OSX tray support. But handbrake still refuses to acknowledge any clicks on the tray area (even on Linux), probably because of this error: could not find gdk window for tray window 0x.....
  • r5785 now handles the humongous window icons that handbrake uses (had to bump the window property size limit)

Then some more window resizing improvements, trying to prevent doing round trips when we can avoid them:

But.. there are still problems I am working on.

This may be related to: #458, #252, #107

@totaam
Copy link
Collaborator Author

totaam commented Mar 14, 2014

2014-03-14 16:21:24: totaam uploaded file noresize.patch (0.7 KiB)

if the size hints don't make sense, use min=max

@totaam
Copy link
Collaborator Author

totaam commented Mar 14, 2014

2014-03-14 16:25:19: totaam commented


More refinements:

  • r5796 prevents client loops: if the client is told by the application to resize the window, those resizing events won't cause "window configure events" on the server if the application has resized since (prevents a race where each end thinks it has the right size: the server knows best until the client can prove it has up to date information by sending the latest counter)
  • r5801: ensure we don't send multiple ConfigureNotify messages to the application window, in particular: "no-change" followed by the actual change in dimensions...

Despite all this... I can still get handbrake to spin out of control.
The only way I found to stop this is to use [/attachment/ticket/532/noresize.patch] which uses a fixed sized window when the size hints don't make sense.
This is far from ideal, the choice of values is arbitrary, and those are just hints: if the application somehow is told to resize (via xdotool?)... it will spin out of control again.

@totaam
Copy link
Collaborator Author

totaam commented Mar 15, 2014

2014-03-15 08:18:52: totaam commented


The invalid window size hints from handbrake can be reproduced easily enough with just Xvfb, no need for xpra:

Xvfb -dpi 96 +extension Composite -screen 0 2048x1200x24+32 -nolisten tcp -noreset :20 &
export DISPLAY=:20
metacity &
ghb &
WID=`xdotool getactivewindow`
xprop -id $WID | grep "size:"
		program specified minimum size: 1030 by 690
		program specified maximum size: 1024 by 768
		program specified base size: -1 by -1
xdotool getwindowgeometry $WID | grep Geometry
  Geometry: 1030x768

I've tried tweaking the dpi and font values but simply cannot get valid hints (they are valid when running on a real display).

@totaam
Copy link
Collaborator Author

totaam commented Mar 16, 2014

2014-03-16 07:37:18: totaam changed status from new to closed

@totaam
Copy link
Collaborator Author

totaam commented Mar 16, 2014

2014-03-16 07:37:18: totaam changed resolution from ** to invalid

@totaam
Copy link
Collaborator Author

totaam commented Mar 16, 2014

2014-03-16 07:37:18: totaam commented


Well, I've spent far too much time on this, but this has led to some improvements in the area of window size handling, etc.. So not all wasted.
As of r5806, the bug is more difficult to hit, but it is still present: you can trigger it by resizing the handbrake application window, either using xdotool on the server-side, or you may also be able to trigger it by resizing the client-side window (depending on how your client-side window manager decides to honour window size hints - in some cases, not).

However, I am now pretty certain that this is a handbrake bug, I have even run xpra + handbrake under xtrace to verify that we aren't doing anything silly, after already verifying with xpra's full debug mode.

What we are doing is that we resize the parent window of the main handbrake window to match its size, but when we do that, handbrake resizes itself to the size we had before and this just repeats forever.

This problem has been reported before to handbrake: Program border flickering with handbrake-gtk 4394svnppa1. I have now added the example there, hopefully they will fix this.
They do not have bug tracker, only a forum (!?) and here is the feedback I got: This message has been submitted successfully, but it will need to be approved by a moderator before it is publicly viewable. You will be notified when your post has been approved.

So I am closing this bug as invalid. Since I was already deep in this area of the code, I've dealt with #212

@totaam totaam closed this as completed Mar 16, 2014
@totaam
Copy link
Collaborator Author

totaam commented Mar 18, 2014

2014-03-18 03:12:01: totaam commented


FYI: despite my very thorough bug report to handbrake (see link above), they don't seem to want to acknowledge the issue, blaming it on a window manager that is not even running..
So I have added handbrake to the new wiki page: [ApplicationQuirks].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant