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

add a --max-size option to limit the maximum window size #263

Closed
totaam opened this issue Feb 18, 2013 · 20 comments
Closed

add a --max-size option to limit the maximum window size #263

totaam opened this issue Feb 18, 2013 · 20 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Feb 18, 2013

On a 2560x1600 monitor, having maximized windows is generally not a good thing: latency goes way up and the refresh rate collapses, basically the application becomes unusable. (and as big monitors become cheaper, this problem will only worsen - LAN bandwidth has remain pretty static over the last 10 years)

This flag should be either a "server default with client override" or a "server constraint that client can restrict further if desired" (both solutions are acceptable).
We just need to be careful that:

  • the new constraints fit with the other constraints given by the client application
  • that the client OS/window-manager doesn't overrule this somehow
@totaam
Copy link
Collaborator Author

totaam commented Feb 21, 2013

Oh well, I was hoping that it would be a simple matter of setting some properties using gtk.Window.set-geometry-hints, but unfortunately that does not affect the maximize button... so we'll need something else for that (and probably also deal with fullscreen properly as per #264)
On X11 clients, I was hoping we could just remove maximize from _NET_WM_ALLOWED_ACTIONS, but this is managed by the client window manager - so we shouldn't touch it I think.

Here's a trivial hard-coded patch which limits resizing to 800x600:

--- src/xpra/server_source.py	(revision 2745)
+++ src/xpra/server_source.py	(working copy)
@@ -640,6 +640,15 @@
                     v = getattr(hints, attr)
                     if v is not None:
                         hints_metadata[metakey] = v
+            MAXW = 800
+            MAXH = 600
+            if True:
+                ms = hints_metadata.get("maximum-size")
+                if ms:
+                    mw, mh = ms
+                else:
+                    mw, mh = 0, 0
+                hints_metadata["maximum-size"] = max(mw, MAXW), max(mh, MAXH)
             return {"size-constraints": hints_metadata}
         elif propname == "class-instance":
             c_i = window.get_property("class-instance")

It looks like (preventing window maximisation/minimisation in x window system) it is not possible (at least with x11, and therefore probably with gtk too) to prevent maximization... We can only detect it and resize back down to our maximum size (which will flicker briefly). When doing this, we will have to be careful to not send the maximized dimensions to the server with each resize event (these sizes also need to be clamped).

So, if we go with native code instead, we find:

  • CreateWindowEx on MS Windows, it looks like we can achieve what we want... I don't think this can be injected into GTK easily - so a huge amount of work to port the whole client or at least the window code :(
  • Looks like OSX is going to be even worse, since there is no maximize button there (only a "fit to content" button - supposedly)

So I think we should go with the easy option for now, and revisit later.

@totaam
Copy link
Collaborator Author

totaam commented Feb 25, 2013

Well, that's a problem...
I was making progress on Linux, then when I tried on win32 I found that gtk/win32 does not honour window sizes:

import gtk
x=gtk.Window()
x.set_geometry_hints(max_width=600, max_height=400)
x.show()
gtk.main()

You can resize it to any size you like, though interestingly the "maximize" button is greyed out.

@totaam
Copy link
Collaborator Author

totaam commented Feb 25, 2013

2013-02-25 18:23:17: antoine uploaded file max-size.patch (9.3 KiB)

adds --max-size option (but does not work on win32..)

@totaam
Copy link
Collaborator Author

totaam commented Feb 25, 2013

According to gdkevents-win32.c, MINMAXINFO.ptMaxTrackSize should be set to the maximum dimensions when gdk receives the WM_GETMINMAXINFO event.
But the example from comment:2 seems to prove that it does not?

@totaam
Copy link
Collaborator Author

totaam commented Feb 27, 2013

@totaam
Copy link
Collaborator Author

totaam commented Mar 17, 2013

Since I've had no reply on the mailing list, I've filed a bug on gnome's bugzilla

@totaam
Copy link
Collaborator Author

totaam commented Mar 22, 2013

Still no answer there - I think we should look at using a different toolkit for the client code, qt or even native versions.

Re-scheduling.

@totaam
Copy link
Collaborator Author

totaam commented May 17, 2013

See also #338

@totaam
Copy link
Collaborator Author

totaam commented May 19, 2014

GTK2 is dead, I'm adding this to #90 instead.

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2014

Looks like the bug got fixed in gtk+ 2.24.23: GDK_HINT_MAX_SIZE ignored on Win32

Now if we could only get a new win32 build of gtk + pygtk... I have found some newer documentation on how to do it here: GTK+ for Windows, and it doesn't look too bad... So we could get a newer GTK build, with newer libpng, etc.. (see #544) without needing to wait for GTK3 (#90)

See for example:

@totaam
Copy link
Collaborator Author

totaam commented Sep 15, 2014

2014-09-15 17:38:00: totaam commented


FWIW: the GTK3 builds don't honour it either :(

I've added a simple test script which can be used from the command line natively on win32: r7633.

@totaam
Copy link
Collaborator Author

totaam commented Sep 16, 2014

2014-09-16 12:54:41: totaam uploaded file gtk3-get-hwnd.patch (7.8 KiB)

patch for accessing the hwnd of a gi gdk window

@totaam
Copy link
Collaborator Author

totaam commented Sep 16, 2014

2014-09-16 14:15:12: totaam commented


  • managed to get a GTK workaround working in r7635 (using monkey patching via pywin32)
  • added max-size support in r7637 (+ fix in r7771)
  • we still honour fullscreen as of r7638 - but maybe this should be an extra switch?

Still TODO for later:

  • GTK3 Cython workaround code because of the lack of gdk_win32_window_get_handle in the gi bindings: the patch attached to this ticket does this (I think - tested only on Linux with a different method call), but it is impossible do build it without building all the dependencies from source on win32 #678 (and I've tried very hard... it's a mess)
  • version test for GTK2 so we don't override fixed versions of GTK (low priority, assuming there are any - I haven't seen one yet)

afarr: please check that:

  • the max-size command line client switch works, in particular with win32 clients
  • that it doesn't prevent fullscreen, that the size restrictions still apply after an un-fullscreen, etc..
  • that applications can also set their won max-size (like this one: [/browser/xpra/trunk/src/tests/xpra/test_apps/test_window_maxsize.py]) and that we honour that too (in fact both, whichever is the smallest)
    etc

@totaam
Copy link
Collaborator Author

totaam commented Oct 2, 2014

2014-10-02 04:34:05: totaam commented


Bug reported in #263 which can be triggered using [/browser/xpra/trunk/src/tests/xpra/test_apps/test_window_maxsize.py] (not sure how I missed that since this is exactly the code that the test was designed to exercise..):

2014-10-01 13:00:08,288 with hints={'min_width': 357, 'min_height': 82}
Traceback (most recent call last):
  File "xpra\client\client_window_base.pyc", line 237, in set_size_constraints
TypeError: apply_geometry_hints() takes exactly 2 arguments (1 given)

is fixed in r7858.

@totaam
Copy link
Collaborator Author

totaam commented Oct 3, 2014

2014-10-03 00:29:21: afarr commented


Testing with 0.156.0 r7865 windows client (windows 8.1) vs. 0.15.0 r7865 server (fedora 20) ... --max-size= works nicely indeed.

Testing with [/browser/xpra/trunk/src/tests/xpra/test_apps/test_window_maxsize.py] behaves as expected, with the windows able to be stretched to new size limits if they are larger than previous, and with the window shrinking to fit new constraints if the new are smaller than the current size (with appropriate notifications server-side).

Testing by stretching xterms and browser windows also shows it to be working very well, with Uh-oh messages server side when the re-sizing limits are hit.

  • Note, when a browser window (in this case google-chrome) is expanded to the limit size, and then moved around the workarea, the Uh-oh messages roll across server-side console in a non-stop wave (not sure whether that logging will just be removed, or if there's liable to be some event triggered in this situation that could potentially cause an issue if a user decides to dance a max-sized window around the display for an hour to the time of a long string of youtube videos...).

  • Note also, while the google-chrome video fullscreen, then Esc key un-fullscreen set of user actions no longer throws that Traceback, it does still change the "outer frame" of the window, making it impossible to re-size to anything but fullscreen, except by killing the browser (red-X). {I suspect it is a separate issue, will open a new ticket and include a before & after screenshot.}

  • Final note - the disabled maximize button ... it would be nice if that could be "controlled" to the same max-size as the connection CLI setting... but I suspect that's a whole new can of worms.

@totaam
Copy link
Collaborator Author

totaam commented Oct 3, 2014

2014-10-03 17:24:48: totaam commented


with Uh-oh messages server side when the re-sizing limits are hit
(..)
Note, when a browser window (in this case google-chrome) is expanded to the limit size, and then moved around the workarea ...
[[BR]]
Well spotted. That's because the max-size we allow the user to set may not be a valid dimension for the window (ie: an xterm requires: width_inc=6, height_inc=13, min_height=17, base_width=19, min_width=25, base_height=4), so we may have to round them down. Then I found out that the win32 api controls the window dimensions with the borders included, so these need substracting. Both done in r7870.
[[BR]]

(..) google-chrome video fullscreen, then Esc key un-fullscreen ..
I suspect it is a separate issue, will open a new ticket and include a before & after screenshot.
[[BR]]
Please do.
[[BR]]

Final note - the disabled maximize button ... it would be nice if that could be "controlled" to the same max-size as the connection CLI setting... but I suspect that's a whole new can of worms.
[[BR]]
AFAIK, we have zero control over that unfortunately. Feel free to file an enhancement request with Microsoft, Apple, GTK, etc.. ;)

@totaam
Copy link
Collaborator Author

totaam commented Oct 3, 2014

2014-10-03 22:16:57: afarr commented


  • Testing with 0.15.0 r7865 osx 10.9 client vs. 0.15.0 r7865 fedora 20 server.. --max-size= works as expected with browser windows, with test_window_maxsize, and with xterms.
  • Note, xterm seems to have a 5 pixel buffer on the second value that reduces its max-size on OSX, but chrome doesn't seem to have the same reduction in place... setting max-size=1573x1309 (arbitrarily) and checking windows I have the following output:
window[1].size=(1573, 1304)
window[2].size=(499, 316)
window[3].size=(1573, 1309)
window[8].size=(500, 200)

... where the 499x316 is a normal sized xterm, the 500x200 is a test_window_maxsize window, the 1573x1309 is a chrome window stretched to its limit, and the 1573x1304 is another xterm stretched to its limit. (I'm not sure this is anything to worry about, just thought I'd mention it.)

@totaam
Copy link
Collaborator Author

totaam commented Nov 22, 2014

Note, xterm seems to have a 5 pixel buffer on the second value that reduces its max-size on OSX

Not sure what that means.. but since things seem to work ok, I'm not going to worry about it!

Closing.

@totaam totaam closed this as completed Nov 22, 2014
@totaam
Copy link
Collaborator Author

totaam commented May 18, 2015

2015-05-18 17:20:05: antoine commented


Started seeing this during testing on win32:

Traceback (most recent call last):
  File "_ctypes/callbacks.c", line 314, in 'calling callback function'
  File "xpra\platform\win32\window_hooks.pyc", line 109, in _wndproc
  File "xpra\platform\win32\window_hooks.pyc", line 74, in on_getminmaxinfo
TypeError: must be a ctypes type

The debug that was added in r9136 caused a name clash and broke the getminmaxinfo callback.
Fixed in r9444 (backport in 9445).

@totaam
Copy link
Collaborator Author

totaam commented Jun 27, 2015

Note: this hook is no longer necessary with GTK3: as of r9737, we skip it with py3k.

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