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

[Pekka Paalanen]

On Sun,  6 Aug 2017 20:03:47 +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 in the ancestor chain found by xwl_window_from_window.
> 
> Signed-off-by: Roman Gilg <subdiff at gmail.com>
> ---
>  hw/xwayland/xwayland-input.c |  2 +-
>  hw/xwayland/xwayland.c       | 27 ++++++++++++++++++---------
>  hw/xwayland/xwayland.h       |  2 +-
>  3 files changed, 20 insertions(+), 11 deletions(-)
> 

Hi,

how does this work when windows are reparented? Does reparenting force
them to go through the unrealize/realize steps again?

Assuming the theory is sound, how about you kept the old names and
semantics of xwl_window_get() and xwl_window_from_window() but just
used the stored private in xwl_window_from_window()? You would need to
have a xwl_window_find_from_window() for the single use in
xwl_realize_window(), but I think that would allow you to put the test

	if (win != xwl_window->window)

in xwl_window_get() rather than open-coding it in the users. I believe
keeping the semantics would be less surprising to the reader,
especially if they know the old code.


Thanks,
pq

> diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
> index 92e530d..d0e7fc4 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_get(grab->window));
>      }
>  
>      ActivateKeyboardGrab(device, grab, time, passive);
> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
> index cb929ca..b7cec4c 100644
> --- a/hw/xwayland/xwayland.c
> +++ b/hw/xwayland/xwayland.c
> @@ -105,7 +105,7 @@ static DevPrivateKeyRec xwl_window_private_key;
>  static DevPrivateKeyRec xwl_screen_private_key;
>  static DevPrivateKeyRec xwl_pixmap_private_key;
>  
> -static struct xwl_window *
> +struct xwl_window *
>  xwl_window_get(WindowPtr window)
>  {
>      return dixLookupPrivate(&window->devPrivates, &xwl_window_private_key);
> @@ -196,7 +196,7 @@ xwl_property_callback(CallbackListPtr *pcbl, void *closure,
>          return;
>  
>      xwl_window = xwl_window_get(rec->win);
> -    if (!xwl_window)
> +    if (!xwl_window || rec->win != xwl_window->window)
>          return;
>  
>      xwl_screen = xwl_screen_get(screen);
> @@ -234,7 +234,7 @@ xwl_close_screen(ScreenPtr screen)
>      return screen->CloseScreen(screen);
>  }
>  
> -struct xwl_window *
> +static struct xwl_window *
>  xwl_window_from_window(WindowPtr window)
>  {
>      struct xwl_window *xwl_window;
> @@ -274,7 +274,7 @@ xwl_cursor_warped_to(DeviceIntPtr device,
>      if (!xwl_seat)
>          xwl_seat = xwl_screen_get_default_seat(xwl_screen);
>  
> -    xwl_window = xwl_window_from_window(window);
> +    xwl_window = xwl_window_get(window);
>      if (!xwl_window && xwl_seat->focus_window) {
>          focus = xwl_seat->focus_window->window;
>  
> @@ -317,7 +317,7 @@ xwl_cursor_confined_to(DeviceIntPtr device,
>          return;
>      }
>  
> -    xwl_window = xwl_window_from_window(window);
> +    xwl_window = xwl_window_get(window);
>      if (!xwl_window && xwl_seat->focus_window) {
>          /* Allow confining on InputOnly windows, but only if the geometry
>           * is the same than the focus window.
> @@ -433,7 +433,7 @@ xwl_realize_window(WindowPtr window)
>      struct xwl_screen *xwl_screen;
>      struct xwl_window *xwl_window;
>      struct wl_region *region;
> -    Bool ret;
> +    Bool ret, create_xwl_window;
>  
>      xwl_screen = xwl_screen_get(screen);
>  
> @@ -450,13 +450,20 @@ xwl_realize_window(WindowPtr window)
>          RegionNull(&window->borderClip);
>      }
>  
> +    create_xwl_window = TRUE;
> +
>      if (xwl_screen->rootless) {
>          if (window->redirectDraw != RedirectDrawManual)
> -            return ret;
> +            create_xwl_window = FALSE;
>      }
>      else {
>          if (window->parent)
> -            return ret;
> +            create_xwl_window = FALSE;
> +    }
> +
> +    if (!create_xwl_window) {
> +        dixSetPrivate(&window->devPrivates, &xwl_window_private_key, xwl_window_from_window(window));
> +        return ret;
>      }
>  
>      xwl_window = calloc(1, sizeof *xwl_window);
> @@ -561,8 +568,10 @@ xwl_unrealize_window(WindowPtr window)
>      screen->UnrealizeWindow = xwl_unrealize_window;
>  
>      xwl_window = xwl_window_get(window);
> -    if (!xwl_window)
> +    if (!xwl_window || xwl_window->window != window) {
> +        dixSetPrivate(&window->devPrivates, &xwl_window_private_key, NULL);
>          return ret;
> +    }
>  
>      wl_surface_destroy(xwl_window->surface);
>      if (RegionNotEmpty(DamageRegion(xwl_window->damage)))
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index 6d3edf3..9e9092d 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -309,7 +309,7 @@ RRModePtr xwayland_cvt(int HDisplay, int VDisplay,
>  void xwl_pixmap_set_private(PixmapPtr pixmap, struct xwl_pixmap *xwl_pixmap);
>  struct xwl_pixmap *xwl_pixmap_get(PixmapPtr pixmap);
>  
> -struct xwl_window *xwl_window_from_window(WindowPtr window);
> +struct xwl_window *xwl_window_get(WindowPtr window);
>  
>  Bool xwl_shm_create_screen_resources(ScreenPtr screen);
>  PixmapPtr xwl_shm_create_pixmap(ScreenPtr screen, int width, int height,

-------------- 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/20170810/58f9a3e4/attachment.sig>