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

[Roman Gilg]

On Thu, Aug 10, 2017 at 2:49 PM, Pekka Paalanen <ppaalanen at gmail.com> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.x.org/archives/xorg-devel/attachments/20170812/2ca2d07c/attachment-0001.html>