GNOME Bugzilla – Bug 759738
wayland: use a subsurface for GDK_WINDOW_TEMP if attached to a toplevel
Last modified: 2016-01-08 09:36:25 UTC
On X11, some application (e.g. gucharmap) use GTK_WINDOW_POPUP to display some additional information (like the zoom window, see bug 759229 for example). On Wayland, this will be translated by default as a regular surface which will take receive focus and cannot be placed programatically by the application. Best way to get on Wayland a behaviour similar to overide redirect on X11 is to use subsurface, but gtk+ doesn't make it particularly easy to achieve this (there is no gtk+ API to request a subsurface because this is a Wayalnd concept and threrefore belongs to the gdk backend). Currently, easiest way is to set the window type to "tooltip" as this will be translated by default as a subsurface on Wayland (if the parent is set), but that's a hack and it's a clear misuse of the intended type. Another possibility is to subclass gtkwindow to override the realize method and set the gdk window type to GDK_WINDOW_SUBSURFACE, but that means that the application needs to test the underlying windowing system (there is no GDK_WINDOW_SUBSURFACE outside of Wayland) and it also adds a lot of complexity for a simple and quite common use case. Ideally, the gdk wayland backend should use a subsurface when possible, ie with GDK_WINDOW_TEMP with a parent (transient_for) set and not when an xdg-popup is to be used (e.g. menus). That would make the fix for Wayland a one liner in the applications, just have to set the transient_for to the toplevel and gdk would automatically prefer a subsurface that can be positioned at will by the application, just like on X11. Patch to follow.
Created attachment 317735 [details] [review] wayland: prefer subsurface when possible Quite a few applications use GTK_WINDOW_POPUP to create various temporary windows and place then on screen. That works fine on X11 but on Wayland there is no global coordinate system for regular surfaces. If the application is using a gdk temp window and set a parent with gtk_window_transient_for(), the gdk wayland backend has all it needs to create a subsurface that can be placed at will by the application.
Could this logic be extended so that in case of GTK_WINDOW_POPUP I always get a window that can be positioned? - If there is a grab use a popup. - If there is no grab use a subsurface. - If there is no parent print a warning that you should set a parent (at least on wayland) I currently set the typehints to get this working under wayland, but with the above all those changes wouldn't be needed.
I reckon we should keep the xdg-popup separate. While an xdg-popup can be mapped without a parent, they cannot be positioned alone as there is no global positioning coordinates on Wayland, so you still need a parent somehow (gdk wayland works around this by trying to guess one if none was specified, based on the input seat grab and if it fails to find one, it will create a surface). So I am tempted to think if you set a parent, you don't need an xdg-popup for the general use case. And this case is covered with the above patch anyway, i.e. a temp gdk window (i.e. a GTK_WINDOW_POPUP) with a parent would be translated as a subsurface.
Created attachment 317756 [details] [review] Updated patch This updated patch drops the window hint test in should_map_as_subsurface() and adds a warning if the parent is not set or not mapped.
Created attachment 317772 [details] [review] [PATCH v3] wayland: prefer subsurface when possible Updated patch v3: should do pretty much what's described in comment 2, we still keep window hint test for compatibility with existing applications though.
Review of attachment 317772 [details] [review]: hmm, placement doesn't seem to work right here. Under X, the popup ends up at 0,0. Under Wayland, it ends up a little further down, but not centered on the main window. ::: gtk/gtkwindow.c @@ +3235,3 @@ + * subsurface-based window #GDK_WINDOW_SUBSURFACE which can be + * positionned at will relatively to the #GTK_WINDOW_TOPLEVEL surface. + * Only one n here: positioned
popup positioning is broken on X11 since https://git.gnome.org/browse/gtk+/commit/?id=2438a06d54636e5074c29bd696e3e81e90288b8d
(In reply to Christoph Reiter (lazka) from comment #8) > popup positioning is broken on X11 since > https://git.gnome.org/browse/gtk+/commit/ > ?id=2438a06d54636e5074c29bd696e3e81e90288b8d is it? I thought it was the other way around, as a git bisect pointed out to the commit that was reverted here.
(In reply to Matthias Clasen from comment #7) > hmm, placement doesn't seem to work right here. Under X, the popup ends up > at 0,0. Under Wayland, it ends up a little further down, but not centered on > the main window. Weird, it works fine on X11 here, just current gtk+ master with the patch applied. The popup being slightly misplaced is more likely an issue in the test program itself, as the size is growing after all widgets (especially the header bar) is added.
(In reply to Olivier Fourdan from comment #9) > (In reply to Christoph Reiter (lazka) from comment #8) > > popup positioning is broken on X11 since > > https://git.gnome.org/browse/gtk+/commit/ > > ?id=2438a06d54636e5074c29bd696e3e81e90288b8d > > is it? I thought it was the other way around, as a git bisect pointed out to > the commit that was reverted here. See bug 759990 and bug 759990 . Current master should work again.
Created attachment 318216 [details] [review] [PATCH 4] wayland: prefer subsurface when possible v4: fix spelling rework test code to use pointer location
Works as advertised here and allows me to get rid of gdk type hints under wayland.
Created attachment 318231 [details] [review] popup grab test I tried to come up with a test for popup positioning with a grab. Under X11 this works, but under wayland (weston) the cursor just shows up for a short time and I don't get any events from outside the window. If I click outside, the window disappears as expected but the window state is still visible/mapped as far as gtk is concerned and I get no event. The inspector also doesn't show the grab cursor so that might be a different problem. Maybe someone can point out my error.
Created attachment 318246 [details] [review] popup grab test (modified) It looks like the program itself behaves as expected, ie the grab works in both X11 and Wayland and the application behaves equally well in both windowing systems. I checked that in this case of a grab, gdkwindow-wayland rightfully choses xdg-popup whereas without the grab, it selects a subsurface (i.e. good to see it behaves as intended with this patch, as far as I can see). It's just the cursor pointer that doesn't show as expected in Wayland, as you say in comment 14, it changes for a short time then revert back to the default cursor. I don't think this is related to this particular bug/patch here so unless I am wrong, we shouldn't deny this bug 759738 for that reason, as this is badly needed for many apps that use popup windows in X11. Anyhow, attaching a version that works for me, in Wayland, you'd need to set the cursor explicitly, apparently...
Created attachment 318247 [details] [review] [PATCH v5] wayland: prefer subsurface when possible v5: don't need to set the window gravity of the toplevel to "static" in the test code, that was a leftover of some other experiment I made before...
Meanwhile, I looked into this grab/cursor issue and the cursor is set back to default by the pointer_handle_enter() event handler, triggered by the popup being mapped right under the pointer... I'll file another bug for this depending on what I find digging there.
(In reply to Olivier Fourdan from comment #15) > Created attachment 318246 [details] [review] [review] > popup grab test (modified) > > It looks like the program itself behaves as expected, ie the grab works in > both X11 and Wayland and the application behaves equally well in both > windowing systems. > > I checked that in this case of a grab, gdkwindow-wayland rightfully choses > xdg-popup whereas without the grab, it selects a subsurface (i.e. good to > see it behaves as intended with this patch, as far as I can see). > > It's just the cursor pointer that doesn't show as expected in Wayland, as > you say in comment 14, it changes for a short time then revert back to the > default cursor. Thanks for having a look. > I don't think this is related to this particular bug/patch here so unless I > am wrong, we shouldn't deny this bug 759738 for that reason, as this is > badly needed for many apps that use popup windows in X11. Sure. Feel free to ignore this patch for this bug. > Anyhow, attaching a version that works for me, in Wayland, you'd need to set > the cursor explicitly, apparently...
(In reply to Christoph Reiter (lazka) from comment #18) > [...] > Sure. Feel free to ignore this patch for this bug. I filed bug 760213 for this and posted a possible fix there.
Review of attachment 318247 [details] [review]: Looks alright
Review of attachment 318247 [details] [review]: attachment 318247 [details] [review] pushed as commit 28f011e wayland: prefer subsurface when possible