[PATCH xserver 1/2] present: cancel flip on reparenting

[Roman Gilg]

I'm a bit unsure on that one. I thought there is no cleanup code
necessary in Present on a reparent because in theory the current
Present code alone allows clients to flip arbitrary many child windows
to a certain parent window as long as they have the same dimension as
the parent. Of course a client trying to do flips on multiple child
windows at the same time all with the same dimension as the parent can
be considered somewhat weird and should not exist in practice. So if
there is a "flipping" window being reparented to one with a child
window doing flips it could be done from Present's side without any
cleanup.

But in the Xwayland case the driver should disallow the reparented
child window doing flips, which then does a cleanup on the Present
side for this particular child window.

Some notes below:

On Thu, Sep 27, 2018 at 5:31 PM Olivier Fourdan <ofourdan at redhat.com> wrote:
> diff --git a/present/present_screen.c b/present/present_screen.c
> index c7e37c5fd..f3f2ddef9 100644
> --- a/present/present_screen.c
> +++ b/present/present_screen.c
> @@ -207,6 +207,21 @@ present_clip_notify(WindowPtr window, int dx, int dy)
>      wrap(screen_priv, screen, ClipNotify, present_clip_notify);
>  }
>
> +static void
> +present_reparent_window(WindowPtr pWin, WindowPtr pPriorParent)
> +{
> +    ScreenPtr screen = pWin->drawable.pScreen;
> +    present_screen_priv_ptr screen_priv = present_screen_priv(screen);
> +
> +    if (screen_priv->reparent_window)
> +        screen_priv->reparent_window(pWin);
> +
> +    unwrap(screen_priv, screen, ReparentWindow)
> +    if (screen->ReparentWindow)
> +        screen->ReparentWindow(pWin, pPriorParent);
> +    wrap(screen_priv, screen, ReparentWindow, present_reparent_window);
> +}
> +
>  static Bool
>  present_screen_register_priv_keys(void)
>  {
> @@ -232,6 +247,7 @@ present_screen_priv_init(ScreenPtr screen)
>      wrap(screen_priv, screen, DestroyWindow, present_destroy_window);
>      wrap(screen_priv, screen, ConfigNotify, present_config_notify);
>      wrap(screen_priv, screen, ClipNotify, present_clip_notify);
> +    wrap(screen_priv, screen, ReparentWindow, present_reparent_window);
>
>      dixSetPrivate(&screen->devPrivates, &present_screen_private_key, screen_priv);
>
> diff --git a/present/present_wnmd.c b/present/present_wnmd.c
> index 8f3836440..0c49a3507 100644
> --- a/present/present_wnmd.c
> +++ b/present/present_wnmd.c
> @@ -682,6 +682,22 @@ present_wnmd_flush(WindowPtr window)
>      (*screen_priv->wnmd_info->flush) (window);
>  }
>
> +static void
> +present_wnmd_reparent_window(WindowPtr pWin)
> +{
> +    present_window_priv_ptr parent_window_priv;
> +
> +    parent_window_priv = present_window_priv(pWin->parent);

The present priv struct of a parent does not carry the information of
a potentially presenting child. These are saved to the child's priv
struct directly. So currently one would need to iterate all children
and check if one of them is presenting.

> +    if (!parent_window_priv)
> +        return;
> +
> +    /* If the new parent window already has a child flipping, cancel the
> +     * flip on the reparented window
> +     */
> +    if (parent_window_priv->flip_pending || parent_window_priv->flip_active)

present_wnmd_cancel_flip checks these conditions already, so we do not
need to do this here as well.

> +        present_wnmd_cancel_flip(pWin);
> +}
> +
>  void
>  present_wnmd_init_mode_hooks(present_screen_priv_ptr screen_priv)
>  {
> @@ -700,4 +716,5 @@ present_wnmd_init_mode_hooks(present_screen_priv_ptr screen_priv)
>
>      screen_priv->abort_vblank       =   &present_wnmd_abort_vblank;
>      screen_priv->flip_destroy       =   &present_wnmd_flip_destroy;
> +    screen_priv->reparent_window    =   &present_wnmd_reparent_window;
>  }
> --
> 2.19.0
>