[PATCH] xwayland: Avoid repeatedly looping through window ancestor chain

[Pekka Paalanen]

On Tue, 22 Aug 2017 15:38:26 +0200
Roman Gilg <subdiff at gmail.com> wrote:

> Calling xwl_window_from_window means looping through the window ancestor
> chain whenever it is called on a child window or on an automatically
> redirected window.
> 
> Since these properties and the potential ancestor's xwl_window are constant
> between window realization and unrealization, we can omit the looping by
> always putting the respective xwl_window in the Window's private field on
> its realization. If the Window doesn't feature an xwl_window on its own,
> it's the xwl_window of its first ancestor with one.
> 
> Signed-off-by: Roman Gilg <subdiff at gmail.com>
> ---
>  hw/xwayland/xwayland-input.c |  2 +-
>  hw/xwayland/xwayland.c       | 54 ++++++++++++++++++++++++--------------------
>  hw/xwayland/xwayland.h       |  2 +-
>  3 files changed, 31 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
> index 92e530d..5a905c7 100644
> --- a/hw/xwayland/xwayland-input.c
> +++ b/hw/xwayland/xwayland-input.c
> @@ -1068,7 +1068,7 @@ xwl_keyboard_activate_grab(DeviceIntPtr device, GrabPtr grab, TimeStamp time, Bo
>          if (xwl_seat == NULL)
>              xwl_seat = find_matching_seat(device);
>          if (xwl_seat)
> -            set_grab(xwl_seat, xwl_window_from_window(grab->window));
> +            set_grab(xwl_seat, xwl_window_of_top(grab->window));
>      }
>  
>      ActivateKeyboardGrab(device, grab, time, passive);
> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
> index cb929ca..79deead 100644
> --- a/hw/xwayland/xwayland.c
> +++ b/hw/xwayland/xwayland.c
> @@ -105,12 +105,23 @@ static DevPrivateKeyRec xwl_window_private_key;
>  static DevPrivateKeyRec xwl_screen_private_key;
>  static DevPrivateKeyRec xwl_pixmap_private_key;
>  
> -static struct xwl_window *
> -xwl_window_get(WindowPtr window)
> +struct xwl_window *
> +xwl_window_of_top(WindowPtr window)
>  {
>      return dixLookupPrivate(&window->devPrivates, &xwl_window_private_key);
>  }
>  
> +static struct xwl_window *
> +xwl_window_of_self(WindowPtr window)
> +{
> +    struct xwl_window *xwl_window = dixLookupPrivate(&window->devPrivates, &xwl_window_private_key);
> +
> +    if (xwl_window && xwl_window->window == window)
> +        return xwl_window;
> +    else
> +        return  NULL;
> +}

Hi Roman,

xwl_window_of_top() vs. xwl_window_of_self(), ok, when seeing both, it
gives a hint about their meaning.

I'm not sure what the convention is in xserver, would this warrant a
comment somewhere explaining what exactly is in each Window's
wl_window_private_key private as not all Windows that have it set
actually own it.

Another thing is maybe the long lines would need splitting.

But anyway, the patch looks good to me, so:
Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.x.org/archives/xorg-devel/attachments/20170823/555d0cb1/attachment.sig>