[PATCH weston 3/6] libweston-desktop/xwayland: window type XWAYLAND cannot have a parent
ppaalanen at gmail.com
Thu Nov 24 14:14:32 UTC 2016
On Thu, 24 Nov 2016 15:04:08 +0100
Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:
> On 24/11/2016 13:59, Pekka Paalanen wrote:
> > On Thu, 24 Nov 2016 12:29:26 +0000
> > Daniel Stone <daniel at fooishbar.org> wrote:
> >> Hi,
> >> On 24 November 2016 at 11:40, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> >>> Add an assert to ensure that a window of type XWAYLAND is never
> >>> attempted with a parent.
> >>> This essentially adding documentation.
> >>> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> >>> ---
> >>> libweston-desktop/xwayland.c | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>> diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
> >>> index bd68bc6..0c4ff2b 100644
> >>> --- a/libweston-desktop/xwayland.c
> >>> +++ b/libweston-desktop/xwayland.c
> >>> @@ -74,6 +74,7 @@ weston_desktop_xwayland_surface_change_state(struct weston_desktop_xwayland_surf
> >>> bool to_add = (parent == NULL && state != XWAYLAND);
> >>> assert(state != NONE);
> >>> + assert(!parent || state != XWAYLAND);
> >> From looking at the callers, it seems like it should be (!parent ||
> >> state == TRANSIENT). I'm a huge fan of adding documentation via
> >> asserts though!
> > Hi,
> > hm, Quentin said type XWAYLAND windows (mostly just override-redirect)
> > must not have a parent, and that's what I wanted to document since
> > XWAYLAND is the one with all the special handling.
> That is exactly why I don’t like asserts as docs for such cases.
> This is all internal API, and it is supposed to be used that way:
> - TOPLEVEL, MAXIMIZED and FULLSCREEN are for toplevel windows, that the
> shell will know about
> - XWAYLAND is for special parentless windows (mostly O-R, but currently
> we have some false positive matches)
> - TRANSIENT is for grabless windows that the shell will never know
> about, and all of them will have a parent *and a position relative to
> this parent*
That would have made a pretty nice doxygen doc. ;-)
I didn't realize TRANSIENT windows are never "added". I would have
assumed to have spotted it in the code... or did I misunderstand
> If a window has a parent and a relative positione, it must be linked to
> this parent. This way, any created view of the parent will get children
> views created too. Theoretically, an XWAYLAND window could have O-R
> children, if they do not have an absolute position, but they would have
> to be TRANSIENT.
> Both your asserts are correct, but they do not “document” the same
> thing. :-)
In that case, let's go with the more strict assert. Asserts can always
be relaxed later, making them more strict might not be easy.
> Maybe it was a mistake to reuse as much code as possible here, since
> XWAYLAND is really special.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 801 bytes
Desc: OpenPGP digital signature
More information about the wayland-devel