GNOME Bugzilla – Bug 783669
Allow resizing tiled windows
Last modified: 2017-10-04 14:49:50 UTC
See patch below for the explanation, and bug 645153 for context.
Created attachment 353585 [details] [review] window: Allow resizing tiled windows With the introduction of client-side decorated windows, GTK+ has to mimic the window manager's behavior when a window is client decorated. As per bug 645153, Mutter is receiving the ability to resize tiled windows in tandem with their tile match windows. Thus, GTK+'s assumption becomes wrong and breaks the new behavior of Mutter. Fix this by allowing resizing tiled windows.
Review of attachment 353585 [details] [review]: Not sure this is really what we want though ? This will just put invisible borders at the edge where they will overlap the other tiled window ?
From my testing these invisible borders are currently already there. They just can't be used to resize the window. This is what I did: 1. Tile a window to the left, one to the right. 2. Focus the one on the right. 3. Click about 4 px left of the left border of the right window. 4. Nothing happens. 5. Click about 10 px left of the left broder of the right window. 6. The left window gets the focus.
yes. This is due to 'tiled' not being a first-class state in gtk. That needs to be fixed to make progress here.
I honestly never had the intention to get this patch merged. It's just a reference patch for the curious souls that wish to test the new work. I apologize to waste Matthias energy and time. I'll take care of GTK's side once I get Mutter figured out.
no need to apologize! very much looking forward to these improvements
The main problem I can see is application being able to remember their "state" when relaunched. If you launch 2 nautilus windows side-by-side, and tile them both, close them, and reopen them, they'll have the size but not the properties of the window(s) you just closed. This is already a problem now, with half/half tiling, but it's going to be worse with smaller and bigger windows.
The issue Bastien raised, is indeed very very annoying, the window "looks" tiled, but it's not and being that big makes it hard to resize, I hope Georges can figure out something to fix that as well. IIRC Mutter has the concet or left and right tile, maybe gtk+ should follow? That would help from a theme pov as well.
Created attachment 357933 [details] [review] gdk: introduce edge constraint states These states will be consumed by GtkWindow in order to have better edge management on tiling situations. Their values are supplied by the compositor, and will be send through and X11 Atom or a Wayland protocol extension.
Created attachment 357934 [details] [review] window: handle tiled edges separately GTK windows don't have their tiling states really hooked into the client-side decoration code, and the only effect it has is disabling the resizing edges. With the introduction of per-edge tiling information, we are backed by much more precise data on how the window manager wants the app to behave. This patch, then, fixes GtkWindow to take into account per-edge tiling information. For compatibility purposes, the previous tiled field was kept, and thing will just continue working if no edge information is supplied.
Created attachment 357935 [details] [review] wayland: consider edge constraints in surface configuration Now that GTK windows have the ability to properly handle per-edge tiling constraints, this patch extends GTK's internal Wayland protocol to have a proper enum with the relevant edge data. Once this approach is validated, we can think of upstreaming this work as an official Wayland protocol extension.
Created attachment 357936 [details] [review] x11: Add support for _GTK_EDGE_CONSTRAINTS atom Following the previous patch, where edge constraints support was added to the Wayland backend, this patch introduces the necessary code to handle the _GTK_EDGE_CONSTRAINTS atom from X11 backend. A small refactoring of the GdkWindowX11 code was necessary in order to cleanly implement this feature.
Created attachment 357937 [details] [review] window: Add individual CSS classes based on edge constraints The last touch on this patch series is making GtkWindow able to selectively adjust various UI details based on the different tiled edges. The main driver here is that we don't want to show shadows on edges that are constrained. This patch adds the necessary code to do that, while still maintaining compatibility with the old ways.
Review of attachment 357933 [details] [review]: The documentation should perhaps say something about backend support for these states.
Review of attachment 357934 [details] [review]: ::: gtk/gtkwindow.c @@ +6866,3 @@ + + g_message ("has edge information! (n: %d, e: %d, s: %d, w: %s)", + resize_n, resize_e, resize_s, resize_w); debug spew here, remove or turn into a proper debug-only output use GTK_NOTE or somesuch @@ +6985,2 @@ + x = (resize_e || resize_w) ? window_border.left + handle_h : 0; + w = (resize_e || resize_w) ? width - 2 * handle_h : width + window_border.left + window_border.right; not sure I understand why the width is the same regardless if one or both edges are resizable
Review of attachment 357933 [details] [review]: I think for gtk4, I would prefer to remove the tiled state, to avoid overlapping information.
Review of attachment 357936 [details] [review]: ::: gdk/x11/gdkdisplay-x11.c @@ +205,3 @@ + + + if (constraints == 0) Hmm, contraints == 0 means either that the wm doesn't support _GTK_EDGE_CONSTRAINTS or that the window is unconstrained, right ? Should this be an explicit check for whether _GTK_EDGE_CONSTRAINTS is supported ? @@ +233,3 @@ + if (old_state & GDK_WINDOW_STATE_TOP_TILED) + { + if ((constraints & 1 << 0) == 0) Might be nice to define an enum or defines for these bits here, rather than open-coding the shifts @@ +1045,3 @@ + + if (xevent->xproperty.atom == gdk_x11_get_xatom_by_name_for_display (display, "_GTK_EDGE_CONSTRAINTS")) + gdk_check_edge_constraints_changed (window); Looks like an indentation mishap here
Review of attachment 357937 [details] [review]: ::: gtk/gtkwindow.c @@ +7553,3 @@ else + { + guint constraints = priv->edge_constraints; might be nice to use the same name as above: either edge_constraints or constraints, in both places. @@ +7884,3 @@ + GDK_WINDOW_STATE_RIGHT_TILED | + GDK_WINDOW_STATE_BOTTOM_TILED | + GDK_WINDOW_STATE_LEFT_TILED)) missing the resizable flags here ?
(In reply to Matthias Clasen from comment #18) > Review of attachment 357937 [details] [review] [review]: > @@ +7884,3 @@ > + GDK_WINDOW_STATE_RIGHT_TILED | > + GDK_WINDOW_STATE_BOTTOM_TILED | > + GDK_WINDOW_STATE_LEFT_TILED)) > > missing the resizable flags here ? I don't think so. The "resizable" flags only affect the CSD input windows for resizing handles, while the "tiled" flags affect the CSS classes. Perhaps I'm missing something?
Created attachment 358542 [details] [review] window: handle tiled edges separately - Properly documented the math. - Improved math to be more clear and concise. - Reworked the corner resizing windows.
Created attachment 358543 [details] [review] wayland: consider edge constraints in surface configuration - Updated protocol modifications to match Mutter's latest version.
Created attachment 358544 [details] [review] x11: Add support for _GTK_EDGE_CONSTRAINTS atom - Left-shift input to reuse GDK enum. - Fixed indentation (damn tabs!)
Created attachment 358545 [details] [review] window: Add individual CSS classes based on edge constraints - Updated to apply against the latest changes. - Updated variable name.
(In reply to Matthias Clasen from comment #17) > Review of attachment 357936 [details] [review] [review]: > > ::: gdk/x11/gdkdisplay-x11.c > @@ +205,3 @@ > + > + > + if (constraints == 0) > > Hmm, contraints == 0 means either that the wm doesn't support > _GTK_EDGE_CONSTRAINTS or that the window is unconstrained, right ? Should > this be an explicit check for whether _GTK_EDGE_CONSTRAINTS is supported ? > for X11, there is _NET_SUPPORTED for Wayland, check the right protocol / protocol version ?
Review of attachment 358542 [details] [review]: thanks for the updates. looks good to me now
Review of attachment 358543 [details] [review]: does this still work with the v1 protocol ? not sure how compatibility works with extra listeners
Review of attachment 358544 [details] [review]: I don't see the GdkWindowX11 refactoring here ?
Review of attachment 358545 [details] [review]: ok
(In reply to Matthias Clasen from comment #26) > Review of attachment 358543 [details] [review] [review]: > > does this still work with the v1 protocol ? not sure how compatibility works > with extra listeners If the server implements v1, it simply won't send 'configure_edges' nor the GTK_SURFACE1_STATE_TILED_* flags. From IRC: "in Mutter, there are checks for the surface1 version. it only sends the edge constraints when the client supports it"
Created attachment 358596 [details] [review] gdk: introduce edge constraint states * Updated comment above the enum
Created attachment 358597 [details] [review] window: handle tiled edges separately * Updated
Created attachment 358598 [details] [review] wayland: consider edge constraints in surface configuration * Updated
Created attachment 358599 [details] [review] x11: Add support for _GTK_EDGE_CONSTRAINTS atom * Check if WM supports _GTK_EDGE_CONSTRAINTS
Created attachment 358600 [details] [review] window: Add individual CSS classes based on edge constraints * Add more selectors
Reapplying previous reviews (modulo X11 and Wayland)
Created attachment 360887 [details] [review] gdk: introduce edge constraint states - Updated doc comments with the correct GTK version
Created attachment 360888 [details] [review] window: Add individual CSS classes based on edge constraints - Finally fix all the black corner issues.
Review of attachment 358598 [details] [review]: Now that I look at this again I wonder why this wasn't done as flags, but too late. Looks ok, anyway.
Review of attachment 358599 [details] [review]: ok
Review of attachment 360887 [details] [review]: ok
Review of attachment 358597 [details] [review]: ::: gtk/gtkwindow.c @@ +7901,3 @@ + (state & GDK_WINDOW_STATE_BOTTOM_RESIZABLE) | + (state & GDK_WINDOW_STATE_LEFT_TILED) | + (state & GDK_WINDOW_STATE_LEFT_RESIZABLE); I think this patch may need some reordering ? Aren't these flags only introduced in a later patch ?
Review of attachment 360888 [details] [review]: ok
Thanks for the reviews! Attachment 358597 [details] pushed as d73c49e - window: handle tiled edges separately Attachment 358598 [details] pushed as 3bae80d - wayland: consider edge constraints in surface configuration Attachment 358599 [details] pushed as c415bef - x11: Add support for _GTK_EDGE_CONSTRAINTS atom Attachment 360887 [details] pushed as f1a3bc2 - gdk: introduce edge constraint states Attachment 360888 [details] pushed as 79bf5b8 - window: Add individual CSS classes based on edge constraints
Whoops! commit 77401118c21402e6a8438efb5659facecbb43413 (HEAD -> gtk-3-22) Author: Daniel Boles <dboles@src.gnome.org> Date: Wed Oct 4 15:42:18 2017 +0100 Adwaita: Fix typo .backgrounf => .background diff --git a/gtk/theme/Adwaita/_common.scss b/gtk/theme/Adwaita/_common.scss index 274c0ce531..c4fb80c232 100644 --- a/gtk/theme/Adwaita/_common.scss +++ b/gtk/theme/Adwaita/_common.scss @@ -1658,7 +1658,7 @@ headerbar { .background.tiled-bottom .titlebar, .background.tiled-left .titlebar, .background.maximized .titlebar, -.backgrounf.solid-csd .titlebar { +.background.solid-csd .titlebar {
Is someone planning to adapt the CSS changes, once that have settled (c.f. Bug 788516 for example), to HighContrast? This is the kind of thing where HighContrast's tendency to be neglected could be especially noticeable.