[PATCH v2 2/2] xdg-shell: Fold several client-side decoration requests into a generic request

Giulio Camuffo giuliocamuffo at gmail.com
Fri Dec 30 15:11:50 UTC 2016


2016-12-30 1:59 GMT+01:00 Adam Goode <agoode at google.com>:
> On Mon, Dec 19, 2016 at 10:15 PM, Jonas Ådahl <jadahl at gmail.com> wrote:
>>
>> On Mon, Dec 19, 2016 at 12:55:24PM -0500, Adam Goode wrote:
>> > Compositors currently receive "move", "resize", and "show_window_menu"
>> > requests from clients whose decorations have been interacted with.
>> > Clients currently implement key- and mouse-binding policies to
>> > determine which one of these requests to send to the compositor in
>> > response to user action.
>> >
>> > Desktop environments traditionally have let users bind a large number of
>> > potential actions to various interactions with decorations: not just a
>> > handful like move and resize.
>> >
>> > This change to xdg-shell makes the protocol more generic, allowing more
>> > surface management policy to be implemented in the compositor directly.
>> > This centralizes more policy in compositors, reducing policy duplication
>> > from client toolkits and allowing for more standard surface management
>> > behavior across them.
>>
>> Hi,
>>
>> I think this is an interesting proposal, but I don't agree it's the
>> right way to do it. Especially, I don't think we should replace
>> move/resize with it, and I think it should be a separate extension.
>> Reasons for this ar 1) triggering a move is not limited window
>> decorations and I don't think its reasonable to define each such area
>> separately, 2) a client is expected to show feedback (i.e. change
>> cursor sprite) when hovering over area that would trigger a resize, this
>> will not be possible since a client has no idea what will happen when
>> clicked anymore, 3) using move/resie requests are proven to work well
>> for what they are meant to do, 4) we're in a state where any major
>> changes will set us back considerably when it comes to getting xdg-shell
>> in state where we can move forward with a backward compatible base.
>>
>> Given this, I wouldn't mind this added as an extension. Whether this
>> extension will eventually be added into xdg-shell "proper" can be
>> discussed at a later point when we have some experience "in the wild",
>> but that is not where we are now. I'd happily review such an extension,
>> but I oppose bumping xdg-shell and replacing move/resize as is proposed
>> here.
>>
>>
>> Jonas
>>
>
> Hi Jonas,
>
> I've been looking into how to do this properly. I've reached a bit of an
> impasse. Specifically, I am thinking about the problems of roundtrips.
>
> Currently, move, resize, and show_window_menu take a serial that the
> compositor remembers. In looking at weston and mutter, this is done through
> implicit grabs, which (appear) to save the last single-button click that
> happened. There is a race condition, because if the client stalls, a new
> implicit grab may occur and wipe out the old one before the move/resize/etc
> request is sent and processed. It mostly works because roundtrips are fast
> enough and clients rarely stall. I think the compositor would just ignore
> the event if the serial doesn't match the latest grab. Lost events lead to a
> bad experience.

It is not a race condition, but rather it prevents one. If the serial
is old it means that the user pressed the mouse somewhere else in the
meanwhile, and at that point you don't want to see the window move
when you're now navigating the shell's menu, for instance, you indeed
want to ignore the move/resize request.

Cheers,
Giulio

>
> We could increase the number of implicit grabs that compositors remember,
> but that would lead to more desynchronization, as client requests come in
> that say "that old event from a while ago was actually a move". It is way
> more complicated to handle than the current single-slot implicit grab that
> compositors use, and seems counter-waylandic to rely on all these roundtrips
> and introduce these race conditions.
>
> My original plans were to continue this reasoning and press on with the
> generic "interact" request. In implementing, I would probably need to look
> into storing at least a few grabs, to let compositors recognize
> double-clicks and such. It was looking a bit sketchy already, but then I
> noticed that axis events don't provide serials, and it would be nice to let
> "interact" allow users to implement other classic bindings, such as "spin
> mousewheel on titlebar to shade". Looking into how axis events are delivered
> to clients really made me start to think if this was the correct way at all.
>
> A complex way to solve the race condition would be for clients to use a
> decoration mask that would be committed to the server along with surface
> updates. Then the compositor would know at all times exactly what
> interactions are on what decorations. This would easily allow compositors to
> even bind axis events without changing the protocol to add serials. Using
> wl_region seemed pretty close to what I wanted, but is fundamentally
> rectangular and seems expensive to update. It would be difficult to use
> wl_region to mask out Chrome's non-rectangular tabs, for example (see
> https://medium.com/google-design/redesigning-chrome-desktop-769aeb5ab987 for
> some illustrations).
>
> So what should I do next? Possible options:
>  1. Stop worrying about these roundtrips and lost events and other issues
> and press forward with "interact".
>  2. Come up with a design for non-rectangular regions and design a
> decoration mask protocol.
>  3. Something simpler I am missing.
>
> I really do want to get this working one way or another, and I'm really
> enjoying learning Wayland. Thanks for your feedback so far!
>
>
>
> Adam
>
>
>
>
>>
>> >
>> > Signed-off-by: Adam Goode <agoode at google.com>
>> > ---
>> >  COPYING                                      |   1 +
>> >  unstable/xdg-shell/xdg-shell-unstable-v7.xml | 137
>> > +++++++++++----------------
>> >  2 files changed, 55 insertions(+), 83 deletions(-)
>> >
>> > diff --git a/COPYING b/COPYING
>> > index 8ab3291..73cebf9 100644
>> > --- a/COPYING
>> > +++ b/COPYING
>> > @@ -6,6 +6,7 @@ Copyright © 2014      Jonas Ådahl
>> >  Copyright © 2014      Jason Ekstrand
>> >  Copyright © 2014-2015 Collabora, Ltd.
>> >  Copyright © 2015      Red Hat Inc.
>> > +Copyright © 2016      Google Inc.
>> >
>> >  Permission is hereby granted, free of charge, to any person obtaining a
>> >  copy of this software and associated documentation files (the
>> > "Software"),
>> > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v7.xml
>> > b/unstable/xdg-shell/xdg-shell-unstable-v7.xml
>> > index de5d21c..5b99eec 100644
>> > --- a/unstable/xdg-shell/xdg-shell-unstable-v7.xml
>> > +++ b/unstable/xdg-shell/xdg-shell-unstable-v7.xml
>> > @@ -580,101 +580,72 @@
>> >        <arg name="app_id" type="string"/>
>> >      </request>
>> >
>> > -    <request name="show_window_menu">
>> > -      <description summary="show the window menu">
>> > -     Clients implementing client-side decorations might want to show
>> > -     a context menu when right-clicking on the decorations, giving the
>> > -     user a menu that they can use to maximize or minimize the window.
>> > -
>> > -     This request asks the compositor to pop up such a window menu at
>> > -     the given position, relative to the local surface coordinates of
>> > -     the parent surface. There are no guarantees as to what menu items
>> > -     the window menu contains.
>> > -
>> > -     This request must be used in response to some sort of user action
>> > -     like a button press, key press, or touch down event.
>> > -      </description>
>> > -      <arg name="seat" type="object" interface="wl_seat" summary="the
>> > wl_seat of the user event"/>
>> > -      <arg name="serial" type="uint" summary="the serial of the user
>> > event"/>
>> > -      <arg name="x" type="int" summary="the x position to pop up the
>> > window menu at"/>
>> > -      <arg name="y" type="int" summary="the y position to pop up the
>> > window menu at"/>
>> > -    </request>
>> > -
>> > -    <request name="move">
>> > -      <description summary="start an interactive move">
>> > -     Start an interactive, user-driven move of the surface.
>> > -
>> > -     This request must be used in response to some sort of user action
>> > -     like a button press, key press, or touch down event. The passed
>> > -     serial is used to determine the type of interactive move (touch,
>> > -     pointer, etc).
>> > -
>> > -     The server may ignore move requests depending on the state of
>> > -     the surface (e.g. fullscreen or maximized), or if the passed
>> > serial
>> > -     is no longer valid.
>> > -
>> > -     If triggered, the surface will lose the focus of the device
>> > -     (wl_pointer, wl_touch, etc) used for the move. It is up to the
>> > -     compositor to visually indicate that the move is taking place,
>> > such as
>> > -     updating a pointer cursor, during the move. There is no guarantee
>> > -     that the device focus will return when the move is completed.
>> > -      </description>
>> > -      <arg name="seat" type="object" interface="wl_seat" summary="the
>> > wl_seat of the user event"/>
>> > -      <arg name="serial" type="uint" summary="the serial of the user
>> > event"/>
>> > -    </request>
>> > -
>> > -    <enum name="resize_edge">
>> > -      <description summary="edge values for resizing">
>> > -     These values are used to indicate which edge of a surface
>> > -     is being dragged in a resize operation.
>> > +    <enum name="decoration_part">
>> > +      <description summary="decoration values for interacting">
>> > +     These values are used to indicate which part of a surface
>> > +     is being interacted with in an interact_with_decoration operation.
>> >        </description>
>> >        <entry name="none" value="0"/>
>> > -      <entry name="top" value="1"/>
>> > -      <entry name="bottom" value="2"/>
>> > -      <entry name="left" value="4"/>
>> > -      <entry name="top_left" value="5"/>
>> > -      <entry name="bottom_left" value="6"/>
>> > -      <entry name="right" value="8"/>
>> > -      <entry name="top_right" value="9"/>
>> > -      <entry name="bottom_right" value="10"/>
>> > +      <entry name="edge_top" value="1"/>
>> > +      <entry name="edge_bottom" value="2"/>
>> > +      <entry name="edge_left" value="4"/>
>> > +      <entry name="edge_top_left" value="5"/>
>> > +      <entry name="edge_bottom_left" value="6"/>
>> > +      <entry name="edge_right" value="8"/>
>> > +      <entry name="edge_top_right" value="9"/>
>> > +      <entry name="edge_bottom_right" value="10"/>
>> > +      <entry name="titlebar" value="17"/>
>> >      </enum>
>> >
>> > -    <request name="resize">
>> > -      <description summary="start an interactive resize">
>> > -     Start a user-driven, interactive resize of the surface.
>> > +    <request name="interact_with_decoration">
>> > +      <description summary="respond to a client-side decoration
>> > interaction">
>> > +     Start a user-driven interaction on a client-side
>> > +     decoration. Clients should send this when a user interacts
>> > +     with the bare titlebar or edge of a client-side decoration.
>> >
>> >       This request must be used in response to some sort of user action
>> >       like a button press, key press, or touch down event. The passed
>> > -     serial is used to determine the type of interactive resize (touch,
>> > +     serial is used to determine the type of interactive action (touch,
>> >       pointer, etc).
>> >
>> > -     The server may ignore resize requests depending on the state of
>> > -     the surface (e.g. fullscreen or maximized).
>> > -
>> > -     If triggered, the client will receive configure events with the
>> > -     "resize" state enum value and the expected sizes. See the "resize"
>> > -     enum value for more details about what is required. The client
>> > -     must also acknowledge configure events using "ack_configure".
>> > After
>> > -     the resize is completed, the client will receive another
>> > "configure"
>> > -     event without the resize state.
>> > -
>> > -     If triggered, the surface also will lose the focus of the device
>> > -     (wl_pointer, wl_touch, etc) used for the resize. It is up to the
>> > -     compositor to visually indicate that the resize is taking place,
>> > -     such as updating a pointer cursor, during the resize. There is no
>> > -     guarantee that the device focus will return when the resize is
>> > -     completed.
>> > -
>> > -     The edges parameter specifies how the surface should be resized,
>> > -     and is one of the values of the resize_edge enum. The compositor
>> > -     may use this information to update the surface position for
>> > -     example when dragging the top left corner. The compositor may also
>> > -     use this information to adapt its behavior, e.g. choose an
>> > -     appropriate cursor image.
>> > +     Compositors will react to this request in different ways
>> > +     depending on their own policy, but it is likely that drag
>> > +     interactions on a titlebar will result in a surface move and
>> > +     drag interactions on edges will result in a surface
>> > +     resize. Other actions are possible, depending on the
>> > +     compositor, but may include raise, lower, maximize, minimize,
>> > +     shade, or show a menu.
>> > +
>> > +     The server may ignore grab_decoration requests depending on
>> > +     the state of the surface (e.g. fullscreen or maximized), or if
>> > +     the passed serial is no longer valid.
>> > +
>> > +     Depending on the nature of the interaction, the client may
>> > +     receive configure events with the "resize" state enum value
>> > +     and the expected sizes. See the "resize" enum value for more
>> > +     details about what is required. The client must also
>> > +     acknowledge configure events using "ack_configure". After the
>> > +     resize is completed, the client will receive another
>> > +     "configure" event without the resize state.
>> > +
>> > +     During some interactions, the surface will lose the focus of
>> > +     the device (wl_pointer, wl_touch, etc) used for the
>> > +     interaction. It is up to the compositor to visually indicate
>> > +     that the interaction is taking place, such as updating a
>> > +     pointer cursor. There is no guarantee that the device focus
>> > +     will return when the interaction is completed.
>> > +
>> > +     The "decoration_part" parameter specifies what part of the
>> > client-side
>> > +     decoration has been interacted with, and is one of the values
>> > +     of the decoration_part enum. The compositor may use this
>> > +     information to update the surface position, for example when
>> > +     dragging the top left corner. The compositor may also use this
>> > +     information to adapt its behavior, e.g. choose an appropriate
>> > +     cursor image.
>> >        </description>
>> >        <arg name="seat" type="object" interface="wl_seat" summary="the
>> > wl_seat of the user event"/>
>> >        <arg name="serial" type="uint" summary="the serial of the user
>> > event"/>
>> > -      <arg name="edges" type="uint" summary="which edge or corner is
>> > being dragged"/>
>> > +      <arg name="decoration_part" type="uint" summary="which part of
>> > the decoration is being interacted with"/>
>> >      </request>
>> >
>> >      <enum name="state">
>> > --
>> > 2.8.0.rc3.226.g39d4020
>> >
>> > _______________________________________________
>> > wayland-devel mailing list
>> > wayland-devel at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
>
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>


More information about the wayland-devel mailing list