After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 766860 - tiled (snapped, half-maximized) windows in Wayland aren't GDK_WINDOW_STATE_TILED
tiled (snapped, half-maximized) windows in Wayland aren't GDK_WINDOW_STATE_TILED
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
3.20.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: WaylandRelated
 
 
Reported: 2016-05-25 09:19 UTC by Simon McVittie
Modified: 2016-06-08 13:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for GTK+ (2.25 KB, patch)
2016-05-25 13:09 UTC, Olivier Fourdan
committed Details | Review
Patch for mutter (1.50 KB, patch)
2016-05-25 13:09 UTC, Olivier Fourdan
committed Details | Review

Description Simon McVittie 2016-05-25 09:19:50 UTC
Steps to reproduce:

* GNOME Shell 3.20 in Wayland mode

* Snap an X11 GTK window to one half of the screen (for example
  `GDK_BACKEND=x11 gedit`)

* Snap a Wayland GTK window to the other half (for example gnome-clocks)

Expected result:

* Each app's top-level window has
  (gdk_window_get_state (gtk_widget_get_window (top_level)) & TILED)

* In particular, each top-level window has rectangular corners

Actual result:

* The X11 window behaves as expected

* The Wayland window is not flagged as TILED, and has rounded corners
  (symptoms similar to Bug #762067)

This appears to be because enum xdg_surface_state in the xdg_shell protocol used by Mutter and GTK to communicate the MAXIMIZED state doesn't have a flag corresponding to being TILED.

This has a particularly annoying effect when using a gnome-terminal with the patches from Bug #760944: because the gnome-terminal can't tell it has been tiled, it doesn't disable the geometry-based resizing, leading to symptoms similar to Bug #755947. The same would be true for any other terminal that implemented resizing in steps of 1 character cell.
Comment 1 Simon McVittie 2016-05-25 09:20:14 UTC
I tried prototyping a workaround in GDK by assuming that any window that is the full width or height of the work area should be considered TILED, but gdk_screen_get_monitor_workarea() is the same as gdk_screen_get_monitor_geometry() on Wayland, so that workaround fails: a tiled window's height is smaller than the apparent work area (it differs by the height of the Shell top bar).
Comment 2 Olivier Fourdan 2016-05-25 11:33:17 UTC
Unfortunately, tiled is not a state known to xdg-shell in Wayland (unlike maximized or fullscreen).

Actually, this is not even the case in X11 either, it's merely (mis)using partial maximization as designed in EWMH (and that's not even complete in gtk, it doesn't implement horizontal tiling for example, see bug 766860)

So, best (imho) would be to try to get tiling in xdg-shell, so we don't have to implement workarounds in the toolkit (like it was done for X11).
Comment 3 Olivier Fourdan 2016-05-25 11:34:29 UTC
(In reply to Olivier Fourdan from comment #2)
> [...] it doesn't implement horizontal tiling for example, see bug 766860

Sorry, I meant bug 742525
Comment 4 Olivier Fourdan 2016-05-25 11:54:16 UTC
Well, thinking of it, we could add these additional states tile vertical and horizontal in the GNOME extended state (range  0x1000 - 0x1FFF) of xdg-shell states so that mutter and gtk+ can share this state using the existing xdg-shell protocol.
Comment 5 Simon McVittie 2016-05-25 12:38:33 UTC
(In reply to Olivier Fourdan from comment #4)
> Well, thinking of it, we could add these additional states tile vertical and
> horizontal in the GNOME extended state (range  0x1000 - 0x1FFF) of xdg-shell
> states so that mutter and gtk+ can share this state using the existing
> xdg-shell protocol.

Where is the canonical reference for what states in that range mean? Mutter? Gtk? I can look into implementing this, but I'd like to be sure that I'm sending the patch to the right place to reserve a state ID.

Adding a single state ID corresponding exactly to GDK_WINDOW_STATE_TILED is probably the best starting point, unless people particularly want to extend GdkWindowState to have more members. The meaning I'm looking for is something like

"""
GNOME_SURFACE_STATE_TILED: the surface is occupying a fixed rectangular space on the screen which may be directly adjacent to other surfaces, for example occupying exactly half the screen or managed in the style of a tiling window manager. The client should not attempt to adjust its size, even if it would normally resize in multiple-pixel increments like a terminal. It might also draw window decorations differently, for example avoiding rounded corners.
"""
Comment 6 Olivier Fourdan 2016-05-25 12:58:53 UTC
I have PoC ready, will upload the patches shortly.

https://cgit.freedesktop.org/wayland/wayland-protocols/tree/unstable/xdg-shell/xdg-shell-unstable-v5.xml#n321
Comment 7 Olivier Fourdan 2016-05-25 13:09:21 UTC
Created attachment 328508 [details] [review]
Patch for GTK+
Comment 8 Olivier Fourdan 2016-05-25 13:09:42 UTC
Created attachment 328512 [details] [review]
Patch for mutter
Comment 9 Olivier Fourdan 2016-05-25 13:13:15 UTC
Can be easily tested using GDK_DEBUG=events and tiling a wayland native gtk+ window (e.g. gtk3-demo).

You will also notice that gtk+ won't use rounded corners with CSD when the window is tiled, which is the expected behaviour.
Comment 10 Simon McVittie 2016-05-25 20:36:13 UTC
(In reply to Olivier Fourdan from comment #6)
> https://cgit.freedesktop.org/wayland/wayland-protocols/tree/unstable/xdg-
> shell/xdg-shell-unstable-v5.xml#n321

I realise that's what defines the range of states available for GNOME's use; I was wondering which GNOME component is the authority on what specific states in that range mean.

The patches look ideal, I'll try them with the patched gnome-terminal.
Comment 11 Simon McVittie 2016-05-26 09:23:45 UTC
(In reply to Simon McVittie from comment #10)
> The patches look ideal, I'll try them with the patched gnome-terminal.

They work fine for me with non-terminal apps, but it looks like I need a few more iterations on the terminal patches to make those do the right thing.
Comment 12 Olivier Fourdan 2016-05-26 09:30:54 UTC
Can you elaborate a bit, what is it exactly that you need or gnome-terminal?

Those patches are a PoC (a very simple fix, that's what I like), but Mike Blumenkrantz mentioned on irc #wayland yesterday that Jonas and himself have been aorking working on xdg-shell for tiling policies for some time, so if you have specific requirements, that might be a good time to post them.

Having an agreed, approved and shared standard for tiling in xdg-shell would be the way to go, but might take a lot longer.
Comment 13 Matthias Clasen 2016-05-26 12:54:59 UTC
Personally, I think the 'reserved ranges' patch to the protocol is not the way to go. At least not with explictly reserved ranges for individual desktops. If anything, there can be a 'private use' area, with the understanding that you need to know your compositor to interpret the states in there.
Comment 14 Olivier Fourdan 2016-05-26 13:01:56 UTC
fwiw, the 'reserved ranges' is not a patch to the protocol, it's already there (comment 6), we merely use an existing mechanism for extending the states.
Comment 15 Matthias Clasen 2016-05-26 13:02:46 UTC
ah. bad protocol already in place :-(
Comment 16 Jonas Ådahl 2016-05-26 14:59:36 UTC
(In reply to Matthias Clasen from comment #15)
> ah. bad protocol already in place :-(

FWIW, the "reserved range" thing is meant for DE's to manage their own set of states. We don't make any public promises, and can for example use our second half of the range for experimental stuff, and the first half for things we want to continue use but still don't want to "upstream". All in all, there are no promises, and its simply our own playground that is designed to interfere with the "real" values or other DE's values.
Comment 17 Jonas Ådahl 2016-05-26 15:00:23 UTC
(In reply to Jonas Ådahl from comment #16)
> (In reply to Matthias Clasen from comment #15)
> > ah. bad protocol already in place :-(
> 
> FWIW, the "reserved range" thing is meant for DE's to manage their own set
> of states. We don't make any public promises, and can for example use our
> second half of the range for experimental stuff, and the first half for
> things we want to continue use but still don't want to "upstream". All in
> all, there are no promises, and its simply our own playground that is
> designed to interfere with the "real" values or other DE's values.

.. to *not* interfere of course ..
Comment 18 Jonas Ådahl 2016-05-26 15:04:44 UTC
(In reply to Olivier Fourdan from comment #12)
> Can you elaborate a bit, what is it exactly that you need or gnome-terminal?
> 
> Those patches are a PoC (a very simple fix, that's what I like), but Mike
> Blumenkrantz mentioned on irc #wayland yesterday that Jonas and himself have
> been aorking working on xdg-shell for tiling policies for some time, so if
> you have specific requirements, that might be a good time to post them.

Those changes are not directly related to tiling, they are more about negotiating the "state" of which a surface is drawn. In the wip we have so far introduced a "no drop shadow" state. All these states are supposed to be optional, and guaranteed to be respected when supported.

I think it might make sense to have tiling state part of the "window state" enum, and and probably tiling edges. For example, I suspect a "no_shadow" mode would mean we still have rounded corners, but a "tiling" mode, we'd have sharp corners.

> 
> Having an agreed, approved and shared standard for tiling in xdg-shell would
> be the way to go, but might take a lot longer.
Comment 19 Olivier Fourdan 2016-05-26 15:17:48 UTC
(In reply to Jonas Ådahl from comment #18)
> Those changes are not directly related to tiling, they are more about
> negotiating the "state" of which a surface is drawn. In the wip we have so
> far introduced a "no drop shadow" state. All these states are supposed to be
> optional, and guaranteed to be respected when supported.

I fail to see "no drop shadow"  as a state to be honest, but that's another topic.
 
> I think it might make sense to have tiling state part of the "window state"
> enum, and and probably tiling edges. For example, I suspect a "no_shadow"
> mode would mean we still have rounded corners, but a "tiling" mode, we'd
> have sharp corners.

I reckon possible tiling state(s) discussions should be driven by developers of tiling WM/CM, because as far as I am concerned, I reckon "tiled" in itself is sufficient, anything more precise (thus as edge tiling) might end up being restrictive, we could have tiling WM/CMs who tile windows in random places on screen, not just screen edges. But then again, we should discuss that on wayland-devel ML for a broader audience.
Comment 20 Jonas Ådahl 2016-05-26 15:23:52 UTC
(In reply to Olivier Fourdan from comment #19)
> (In reply to Jonas Ådahl from comment #18)
> > Those changes are not directly related to tiling, they are more about
> > negotiating the "state" of which a surface is drawn. In the wip we have so
> > far introduced a "no drop shadow" state. All these states are supposed to be
> > optional, and guaranteed to be respected when supported.
> 
> I fail to see "no drop shadow"  as a state to be honest, but that's another
> topic.
>

State, mode, .. the major difference from window states is that its negotiated, not just notified.

> > I think it might make sense to have tiling state part of the "window state"
> > enum, and and probably tiling edges. For example, I suspect a "no_shadow"
> > mode would mean we still have rounded corners, but a "tiling" mode, we'd
> > have sharp corners.
> 
> I reckon possible tiling state(s) discussions should be driven by developers
> of tiling WM/CM, because as far as I am concerned, I reckon "tiled" in
> itself is sufficient, anything more precise (thus as edge tiling) might end
> up being restrictive, we could have tiling WM/CMs who tile windows in random
> places on screen, not just screen edges. But then again, we should discuss
> that on wayland-devel ML for a broader audience.

Shouldn't we just mark the left, top and bottom edges tiled, and the right edge untiled, meaning drop shadow should only be drawn on the right edge, if we tile on the left half of the screen, in gnome shell? I.e. by pressing Ctrl-Super-Left (or whatever the default is if I changed it).
Comment 21 Olivier Fourdan 2016-05-26 15:30:22 UTC
(In reply to Jonas Ådahl from comment #20)
> Shouldn't we just mark the left, top and bottom edges tiled, and the right
> edge untiled, meaning drop shadow should only be drawn on the right edge, if
> we tile on the left half of the screen, in gnome shell? I.e. by pressing
> Ctrl-Super-Left (or whatever the default is if I changed it).

Oh I see what you mean, I thought you were speaking of screen edges whereas it's window edges you're talking about.

But then, think of the current behaviour in gtk+ (like for example with the two patches applied), when a window is tiled, it won't use round corners, not even on the side which is not tiled, so that when you have two windows tiled next to each other, the borders align correctly.
Comment 22 Jonas Ådahl 2016-05-26 15:33:14 UTC
(In reply to Olivier Fourdan from comment #21)
> (In reply to Jonas Ådahl from comment #20)
> > Shouldn't we just mark the left, top and bottom edges tiled, and the right
> > edge untiled, meaning drop shadow should only be drawn on the right edge, if
> > we tile on the left half of the screen, in gnome shell? I.e. by pressing
> > Ctrl-Super-Left (or whatever the default is if I changed it).
> 
> Oh I see what you mean, I thought you were speaking of screen edges whereas
> it's window edges you're talking about.
> 
> But then, think of the current behaviour in gtk+ (like for example with the
> two patches applied), when a window is tiled, it won't use round corners,
> not even on the side which is not tiled, so that when you have two windows
> tiled next to each other, the borders align correctly.

If the right edge is completely "free" (not tiled against anything), should it still have drop shadow?
Comment 23 Simon McVittie 2016-05-26 15:53:21 UTC
(In reply to Olivier Fourdan from comment #12)
> Can you elaborate a bit, what is it exactly that you need [for]
> gnome-terminal?

Apart from the decision whether GTK's client-side decorations should have rounded or sharp corners (which is internal to GTK), gnome-terminal wants to be able to decide between two modes:

* fit exactly into a space we've been given, and fill that space even if
  it means leaving up to (1 character cell - 1 pixel) unused around the
  edges
  (MAXIMIZED, TILED or FULLSCREEN)

* behave as a normal "floating" window; constrain resizes to
  character-cell-sized steps because that's all that makes sense for
  a terminal
  (none of those flags)

I would guess that tiling window managers (XMonad, Awesome, etc.) would probably want to put every window in the TILED mode too.

GTK's API for this, GDK_WINDOW_STATE_TILED, is (quoting the docs)

"""
the window is in a tiled state
"""

and it isn't 100% clear which specific aspect(s) of tiling were meant by that, but given that the API exists, it seems sensible for it to mean *something* in Wayland.
Comment 24 Matthias Clasen 2016-05-27 01:59:07 UTC
(In reply to Simon McVittie from comment #23)

> 
> GTK's API for this, GDK_WINDOW_STATE_TILED, is (quoting the docs)
> 
> """
> the window is in a tiled state
> """
> 
> and it isn't 100% clear which specific aspect(s) of tiling were meant by
> that, but given that the API exists, it seems sensible for it to mean
> *something* in Wayland.

When this was implemented, I didn't have much to work with (just the horizontal/vertical maximized state).

I think it would make sense to keep this somewhat vague overall 'tiled' state, and add more information where we can (e.g as more GNOME-private states in the Wayland case).

Information that would be useful to get:

- Which edges should get a drop shadow ?
- Which edges should be resizable ?
Comment 25 Matthias Clasen 2016-05-27 02:00:31 UTC
Review of attachment 328508 [details] [review]:

Given that this mechanism exists, lets use it.
Comment 26 Matthias Clasen 2016-05-27 02:02:20 UTC
Review of attachment 328512 [details] [review]:

::: src/wayland/meta-wayland-xdg-shell.c
@@ +407,3 @@
+      s = wl_array_add (states, sizeof *s);
+      *s = XDG_SURFACE_STATE_GNOME_TILED;
+    }

A very obvious enhancement here would be to have separate states for tiled-left and tiled-right, but we may want to discuss the best way to encode that; e.g. by enumerating non-tiled edges
Comment 27 Simon McVittie 2016-06-06 06:51:51 UTC
(In reply to Matthias Clasen from comment #25)
> Review of attachment 328508 [details] [review]:
> 
> Given that this mechanism exists, lets use it.

This mechanism doesn't exist until Attachment #328512 [details] is (reviewed and) merged.

(In reply to Matthias Clasen from comment #26)
> Review of attachment 328512 [details] [review]:
> 
> ::: src/wayland/meta-wayland-xdg-shell.c
> @@ +407,3 @@
> +      s = wl_array_add (states, sizeof *s);
> +      *s = XDG_SURFACE_STATE_GNOME_TILED;
> +    }
> 
> A very obvious enhancement here would be to have separate states for
> tiled-left and tiled-right, but we may want to discuss the best way to
> encode that; e.g. by enumerating non-tiled edges

Having a simple flag for TILED exactly matches the GDK API here, and is sufficient to preserve X11 functionality under Wayland (rectangular window decorations when tiled, and reporting the tiled status to application code). I think there's a risk of letting the perfect be the enemy of the good: can we land the simple version that is sufficient to implement the current GDK API, then leave the bug open (or clone another bug) for enhancing it to describe more elaborate encodings if desired?
Comment 28 Olivier Fourdan 2016-06-06 09:35:29 UTC
+1, let's keep things simple.

https://lists.freedesktop.org/archives/wayland-devel/2016-June/029305.html
Comment 29 Simon McVittie 2016-06-06 10:14:52 UTC
(In reply to Olivier Fourdan from comment #28)
> +1, let's keep things simple.
> 
> https://lists.freedesktop.org/archives/wayland-devel/2016-June/029305.html

Feel free to copy my wording from Comment #5 if you want something a bit longer/more explanatory for the protocol text.
Comment 30 Olivier Fourdan 2016-06-06 11:22:41 UTC
Comment #5 might be a tad GNOME (gtk+/gdk/mutter) centric though, xdg-shell is meant to be used by a much wider range on implementations.
Comment 31 Olivier Fourdan 2016-06-06 11:43:48 UTC
Oh sorry, my bad, thought you were mentioning comment 5 as the justification for a single "tiled" state, whereas you meant the description of the state... Let's see first if other people agree on a single value for the window state, and then if there is a need for a better explanation if the state.
Comment 32 Matthias Clasen 2016-06-06 12:38:42 UTC
For me, the single tiled state was a stopgap when I introduced it for X11. It is insufficient even for gnome-shells simplistic tiling. Knowing left or right would let us do better theming for window borders and shadows in the tiled case.
Comment 33 Olivier Fourdan 2016-06-06 13:19:05 UTC
(In reply to Matthias Clasen from comment #32)
> For me, the single tiled state was a stopgap when I introduced it for X11.
> It is insufficient even for gnome-shells simplistic tiling. Knowing left or
> right would let us do better theming for window borders and shadows in the
> tiled case.

Could this be part of the additional "draw states" that Mike has proposed?

https://lists.freedesktop.org/archives/wayland-devel/2016-May/029106.html
Comment 34 Matthias Clasen 2016-06-06 13:21:24 UTC
(In reply to Olivier Fourdan from comment #33)
> (In reply to Matthias Clasen from comment #32)
> > For me, the single tiled state was a stopgap when I introduced it for X11.
> > It is insufficient even for gnome-shells simplistic tiling. Knowing left or
> > right would let us do better theming for window borders and shadows in the
> > tiled case.
> 
> Could this be part of the additional "draw states" that Mike has proposed?
> 
> https://lists.freedesktop.org/archives/wayland-devel/2016-May/029106.html

That sounds like quite a reversal from semantic states back to purely visual mwm-hints styles. Is that going to fly ?
Comment 35 Jonas Ådahl 2016-06-06 13:25:21 UTC
(In reply to Olivier Fourdan from comment #33)
> (In reply to Matthias Clasen from comment #32)
> > For me, the single tiled state was a stopgap when I introduced it for X11.
> > It is insufficient even for gnome-shells simplistic tiling. Knowing left or
> > right would let us do better theming for window borders and shadows in the
> > tiled case.
> 
> Could this be part of the additional "draw states" that Mike has proposed?
> 
> https://lists.freedesktop.org/archives/wayland-devel/2016-May/029106.html

That is more about negotiating responsibility of drawing parts of a window. The partial tiling stuff should just be together with the regular window state, just as in your first patch. It'll probably only be GNOME who sets anything but tile-all-sides though.
Comment 36 Olivier Fourdan 2016-06-06 14:07:43 UTC
(In reply to Jonas Ådahl from comment #35)
> > https://lists.freedesktop.org/archives/wayland-devel/2016-May/029106.html
> 
> That is more about negotiating responsibility of drawing parts of a window.
> The partial tiling stuff should just be together with the regular window
> state, just as in your first patch. It'll probably only be GNOME who sets
> anything but tile-all-sides though.

OK, could you reply the patch on the wayland-devel ML and weigh in favour of the initial approach then, my initial patch with different states hasn't been very successful, hence my new patch for a single state instead.
Comment 37 Jonas Ådahl 2016-06-07 01:15:30 UTC
Review of attachment 328512 [details] [review]:

I think this is fine for now. For v6 we might need to add our own enum and configure if we get rid of the private state ranges, but lets deal with that then.
Comment 38 Olivier Fourdan 2016-06-08 12:56:15 UTC
Comment on attachment 328508 [details] [review]
Patch for GTK+

attachment 328508 [details] [review] pushed in gtk+ as commit 8540718 - wayland: add extended state for tiled
Comment 39 Olivier Fourdan 2016-06-08 13:02:32 UTC
Comment on attachment 328512 [details] [review]
Patch for mutter

attachment 328512 [details] [review] pushed in mutter as commit e7430a4 - wayland: add extended state for tiled
Comment 40 Olivier Fourdan 2016-06-08 13:05:10 UTC
Both patches have been pushed, closing the bug for now. 

Once we reach a consensus for xdg-shell v6 wrt. tiling, we can adopt the standard and drop those sepecific extensions.