GNOME Bugzilla – Bug 769937
wayland: Port backend to use xdg-shell unstable v6
Last modified: 2016-08-25 04:45:47 UTC
Port the xdg-shell usage to recently released unstable v6 version. This also updates the gtk-shell protocol to extend the configure event instead of adding arbitrary values given the state range previously assigned to GNOME. This also implements move_to_rect properly. This needs mutter patches in bug 769936 to function properly.
Created attachment 333358 [details] [review] wayland: Fix indentation
Created attachment 333359 [details] [review] wayland: Get tiled state from gtk_shell instead of xdg_shell Use our the 'tiled' entry from our new 'state' enum sent via xdg_surface.configure.
Created attachment 333360 [details] [review] wayland: Remove gdk_wayland_display_get_xdg_shell() Don't expose the xdg_shell struct as it is not yet a stable type that will stay the same.
Created attachment 333361 [details] [review] wayland: Only sync surface regions once per commit Only set input, opaque and window geometry regions once per commit. They are double buffered anyway, so the last one would only take effect either way; this way reading protocol logs are much more pleasent.
Created attachment 333362 [details] [review] wayland: Only update the window title if it actually updated This makes the protocol log less spammy.
Created attachment 333363 [details] [review] wayland: Move window geometry calculation to helper We'll use it from more places later.
Created attachment 333364 [details] [review] wayland: Port to xdg_shell unstable v6
Created attachment 333365 [details] [review] wayland: Implement move_to_rect Translate move_to_rect parameter into xdg_positioner requests, and use the generated xdg_positioner to create the popup.
Review of attachment 333358 [details] [review]: sure
Review of attachment 333359 [details] [review]: ok
Review of attachment 333360 [details] [review]: I don't think we want to remove a symbol that has been around since 3.8. Can we just document it as 'unstable, don't use' ?
Review of attachment 333361 [details] [review]: ok
Review of attachment 333362 [details] [review]: ::: gdk/wayland/gdkwindow-wayland.c @@ +2357,3 @@ + if (impl->title && title && strcmp (title, impl->title) == 0) + return; Could use g_strcmp0 here, but no big deal
Review of attachment 333363 [details] [review]: sure
(In reply to Matthias Clasen from comment #11) > Review of attachment 333360 [details] [review] [review]: > > I don't think we want to remove a symbol that has been around since 3.8. Can > we just document it as 'unstable, don't use' ? That'd mean defining struct xdg_surface ourself and always return NULL. Is that good enough?
Review of attachment 333364 [details] [review]: ok. We should land this at the same time as the mutter side
Review of attachment 333365 [details] [review]: ::: gdk/wayland/gdkwindow-wayland.c @@ +2367,3 @@ + gdk_window_thaw_updates (window); + else + impl->initial_configure_received = FALSE; Is this related to the rest of the patch or an independent bug fix ?
Review of attachment 333364 [details] [review]: do we stop working with any current (non-shell) compositors by bumping the xdg-shell version ?
(In reply to Matthias Clasen from comment #19) > Review of attachment 333364 [details] [review] [review]: > > do we stop working with any current (non-shell) compositors by bumping the > xdg-shell version ? We wont work on weston 1.11, but will work with 1.12 which will support unstable v6. I'd guess we will stop working on most compositors really, but that's a fundamental issue with relying on xdg-shell before it was declared stable. Eventually we have to break the world, and (hopefully only once) again for the next backward incompatible xdg-shell version. The alternative is to add a "best-effort" wl_shell fallback, but that'll work a lot worse than falling back to X11.
Review of attachment 333365 [details] [review]: ::: gdk/wayland/gdkwindow-wayland.c @@ +2367,3 @@ + gdk_window_thaw_updates (window); + else + impl->initial_configure_received = FALSE; Looks like this leaked over from the previous patch.
(In reply to Jonas Ådahl from comment #16) > (In reply to Matthias Clasen from comment #11) > > Review of attachment 333360 [details] [review] [review] [review]: > > > > I don't think we want to remove a symbol that has been around since 3.8. Can > > we just document it as 'unstable, don't use' ? > > That'd mean defining struct xdg_surface ourself and always return NULL. Is > that good enough? Hmm, I don't think thats really useful. The function previously was never returning NULL, so callers will not handle that. Anyway, debians codesearch finds no callers, so we can probably get away with dropping this, after all.
We should add a release note for that
(In reply to Jonas Ådahl from comment #20) > (In reply to Matthias Clasen from comment #19) > > Review of attachment 333364 [details] [review] [review] [review]: > > > > do we stop working with any current (non-shell) compositors by bumping the > > xdg-shell version ? > > We wont work on weston 1.11, but will work with 1.12 which will support > unstable v6. I'd guess we will stop working on most compositors really, but > that's a fundamental issue with relying on xdg-shell before it was declared > stable. Eventually we have to break the world, and (hopefully only once) > again for the next backward incompatible xdg-shell version. We should release-note this as well. I'll take care of that once this lands.
For xdg-shell v6 we can also consider the patches I posted in bug 764413 for min/max sizes.
(In reply to Olivier Fourdan from comment #25) > For xdg-shell v6 we can also consider the patches I posted in bug 764413 for > min/max sizes. Yes, makes sense. Could you rebase them on these ones?
Oddly, attachment 333361 [details] [review] doesn;t seem to apply cleanly on top of git master here (can't find unset_transient_for_exported() in master)
(In reply to Olivier Fourdan from comment #27) > Oddly, attachment 333361 [details] [review] [review] doesn;t seem to apply cleanly on > top of git master here (can't find unset_transient_for_exported() in master) Ah found out, bugzilla search in attachments ftw \o/ It needs attachment 333170 [details] [review] from bug 769788
Attachment 333358 [details] pushed as 80dd756 - wayland: Fix indentation Attachment 333359 [details] pushed as e53d381 - wayland: Get tiled state from gtk_shell instead of xdg_shell Attachment 333360 [details] pushed as d2385be - wayland: Remove gdk_wayland_display_get_xdg_shell() Attachment 333361 [details] pushed as 8270699 - wayland: Only sync surface regions once per commit Attachment 333362 [details] pushed as d2a80cd - wayland: Only update the window title if it actually updated Attachment 333363 [details] pushed as 643f033 - wayland: Move window geometry calculation to helper Attachment 333364 [details] pushed as ceada4a - wayland: Port to xdg_shell unstable v6 Attachment 333365 [details] pushed as b7964cf - wayland: Implement move_to_rect