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

[Adam Jackson]

On Tue, 2017-08-22 at 15:38 +0200, Roman Gilg 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 

remote: I: patch #172927 updated using rev 
82df2ce38c560915f8c6574052bd56215b649072.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   1089d5d518..82df2ce38c  master -> master

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

[Pekka Paalanen]

On Tue, 22 Aug 2017 15:38:26 +0200
Roman Gilg  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 
> ---
>  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(>devPrivates, _window_private_key);
>  }
>  
> +static struct xwl_window *
> +xwl_window_of_self(WindowPtr window)
> +{
> +struct xwl_window *xwl_window = dixLookupPrivate(>devPrivates, 
> _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 


Thanks,
pq


pgpMLLZCOJ8P8.pgp
Description: OpenPGP digital signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

[Roman Gilg]

I've just sent another version with different naming and the additional
function for comparing the private field's xwl_window with its creator as
reply to your mail. Take a look how you like it. One other difference: I
realized that the xwl_window_find(..) function is unneeded, since when
putting the xwl_window always in the private field, we can just on
Realization take the one of its parent window.

On Mon, Aug 14, 2017 at 11:33 AM, Pekka Paalanen 
 wrote:
>
> How does your Present and sub-surface work change the relationships
> between Windows and xwl_windows? Does the child Window getting a
> sub-surface have its own xwl_window? Are there cases for fetching from
> the child Window (or from its descendant) both its own xwl_window and
> the respective toplevel Window's xwl_window?
>

Currently I always use the xwl_window of the ancestor like it's right now.
That means that in one ancestor chain originating from the top xwl_window
only one of the windows can use the Present extension at one point in time.
But I assume that's sufficient for most use cases. Introducing another
struct for allowing flips also for windows without owning an xwl_window
could be an improvement in the future though.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

[Roman Gilg]

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 
---
 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(>devPrivates, _window_private_key);
 }
 
+static struct xwl_window *
+xwl_window_of_self(WindowPtr window)
+{
+struct xwl_window *xwl_window = dixLookupPrivate(>devPrivates, 
_window_private_key);
+
+if (xwl_window && xwl_window->window == window)
+return xwl_window;
+else
+return  NULL;
+}
+
 struct xwl_screen *
 xwl_screen_get(ScreenPtr screen)
 {
@@ -195,7 +206,7 @@ xwl_property_callback(CallbackListPtr *pcbl, void *closure,
 if (rec->win->drawable.pScreen != screen)
 return;
 
-xwl_window = xwl_window_get(rec->win);
+xwl_window = xwl_window_of_self(rec->win);
 if (!xwl_window)
 return;
 
@@ -234,22 +245,6 @@ xwl_close_screen(ScreenPtr screen)
 return screen->CloseScreen(screen);
 }
 
-struct xwl_window *
-xwl_window_from_window(WindowPtr window)
-{
-struct xwl_window *xwl_window;
-
-while (window) {
-xwl_window = xwl_window_get(window);
-if (xwl_window)
-return xwl_window;
-
-window = window->parent;
-}
-
-return NULL;
-}
-
 static struct xwl_seat *
 xwl_screen_get_default_seat(struct xwl_screen *xwl_screen)
 {
@@ -274,7 +269,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_of_top(window);
 if (!xwl_window && xwl_seat->focus_window) {
 focus = xwl_seat->focus_window->window;
 
@@ -317,7 +312,7 @@ xwl_cursor_confined_to(DeviceIntPtr device,
 return;
 }
 
-xwl_window = xwl_window_from_window(window);
+xwl_window = xwl_window_of_top(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,6 +428,7 @@ xwl_realize_window(WindowPtr window)
 struct xwl_screen *xwl_screen;
 struct xwl_window *xwl_window;
 struct wl_region *region;
+Bool create_xwl_window = TRUE;
 Bool ret;
 
 xwl_screen = xwl_screen_get(screen);
@@ -452,11 +448,17 @@ xwl_realize_window(WindowPtr window)
 
 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) {
+if (window->parent)
+dixSetPrivate(>devPrivates, _window_private_key, 
xwl_window_of_top(window->parent));
+return ret;
 }
 
 xwl_window = calloc(1, sizeof *xwl_window);
@@ -560,9 +562,11 @@ xwl_unrealize_window(WindowPtr window)
 xwl_screen->UnrealizeWindow = screen->UnrealizeWindow;
 screen->UnrealizeWindow = xwl_unrealize_window;
 
-xwl_window = xwl_window_get(window);
-if (!xwl_window)
+xwl_window = xwl_window_of_self(window);
+if (!xwl_window) {
+dixSetPrivate(>devPrivates, _window_private_key, NULL);
 return ret;
+}
 
 wl_surface_destroy(xwl_window->surface);
 if 

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

[Pekka Paalanen]

On Sat, 12 Aug 2017 03:45:39 +0200
Roman Gilg  wrote:

> On Thu, Aug 10, 2017 at 2:49 PM, Pekka Paalanen  wrote:

> > 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.
> >  
> 
> It's a good idea to not open-code the condition for the users. I would
> prefer though to risk the short-term confusion for the users, because
> otherwise in the long term the naming is ambiguous:
> 
> A Window has exactly one xwl_window associated (or none): The one in its
> private field. Calling xwl_window_get() therefore is the natural way to
> receive this one xwl_window. The function naming becomes really confusing
> when we call xwl_window_from_window() inside xwl_window_get() to receive
> the private field and afterwards do the test or when we call
> xwl_window_from_window() in xwl_window_find_from_window() on the Window's
> parent Window.

Hi Roman,

are you seeing xwl_window_get() as "get the private" while I think of
it as "get the owned xwl_window"? In that case, and as it has not been
documented which one it really meant before, I think we should just not
use the name xwl_window_get() anymore at all to get rid of the
ambiguity. This is the most important point in my comments and even
that is not too important.

> How do you like this approach:
> * xwl_window_get(WindowPtr) returns the private field
> * New function xwl_window_own(WindowPtr win) calls xwl_window_get and
> forwards the xwl_window if (win == xwl_window->window), otherwise NULL.
> Maybe there is a better name than xwl_window_own though for this function?
> * xwl_window_from_window() is renamed to xwl_window_find() and as it's now
> will only be called in xwl_realize_window.
> 
> But if you prefer the other naming, it's also fine with me.

Yes, that sounds better. Since the semantics of xwl_window_get() change
due to setting the window private for more windows than the one
actually owning the xwl_window, it requires touching all the current
callers. That's why I previously suggested to keep its semantics intact.

Would it be nice to rename xwl_window_get() to
xwl_window_from_priv() to denote this change in semantics and
ensure that no callers go unnoticed? Or to_xwl_window()?

xwl_window_own() would be the replacement for the old xwl_window_get(),
or maybe call it... xwl_window_from_owner()? Returns NULL if the Window
doesn't own any xwl_window.

How does your Present and sub-surface work change the relationships
between Windows and xwl_windows? Does the child Window getting a
sub-surface have its own xwl_window? Are there cases for fetching from
the child Window (or from its descendant) both its own xwl_window and
the respective toplevel Window's xwl_window?

If the above doesn't feel particularly fitting, then let's go with your
suggestion.


Thanks,
pq


pgpTf0C51NLEN.pgp
Description: OpenPGP digital signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

[Roman Gilg]

On Thu, Aug 10, 2017 at 2:49 PM, Pekka Paalanen  wrote:
>
> how does this work when windows are reparented? Does reparenting force
> them to go through the unrealize/realize steps again?
>

I assumed this because it is described this way for the XLib
function XReparentWindow:
https://linux.die.net/man/3/xreparentwindow

Thanks Adam for confirming this.


> 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.
>

It's a good idea to not open-code the condition for the users. I would
prefer though to risk the short-term confusion for the users, because
otherwise in the long term the naming is ambiguous:

A Window has exactly one xwl_window associated (or none): The one in its
private field. Calling xwl_window_get() therefore is the natural way to
receive this one xwl_window. The function naming becomes really confusing
when we call xwl_window_from_window() inside xwl_window_get() to receive
the private field and afterwards do the test or when we call
xwl_window_from_window() in xwl_window_find_from_window() on the Window's
parent Window.

How do you like this approach:
* xwl_window_get(WindowPtr) returns the private field
* New function xwl_window_own(WindowPtr win) calls xwl_window_get and
forwards the xwl_window if (win == xwl_window->window), otherwise NULL.
Maybe there is a better name than xwl_window_own though for this function?
* xwl_window_from_window() is renamed to xwl_window_find() and as it's now
will only be called in xwl_realize_window.

But if you prefer the other naming, it's also fine with me.

Cheers,
Roman
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

[Pekka Paalanen]

On Sun,  6 Aug 2017 20:03:47 +0200
Roman Gilg  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 
> ---
>  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(>devPrivates, _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(>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(>devPrivates, _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 = 

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

[Roman Gilg]

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 
---
 hw/xwayland/xwayland-input.c |  2 +-
 hw/xwayland/xwayland.c   | 27 ++-
 hw/xwayland/xwayland.h   |  2 +-
 3 files changed, 20 insertions(+), 11 deletions(-)

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(>devPrivates, _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(>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(>devPrivates, _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(>devPrivates, _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,
-- 
2.7.4

___