[PATCH xserver 2/3] present: Factor code for restoring screen pixmap out of present_unflip (v2)

[Michel Dänzer]

On 22.02.2016 18:16, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
> 
> The following fix will use the refactored function.
> 
> The logic in the refactored function is slightly simplified, exploiting
> the fact that this function is only ever called with a valid flip
> pixmap.
> 
> v2: Assert that flip_pixmap is non-NULL instead of testing and bailing
>     on NULL, preserve test for flip_window being non-NULL before calling
>     present_set_tree_pixmap for it (Keith Packard)
> 
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk> (v1)
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>

Keith, does this look good?

Or can I get a review from someone else?


> diff --git a/present/present.c b/present/present.c
> index d6ec895..63ad275 100644
> --- a/present/present.c
> +++ b/present/present.c
> @@ -392,6 +392,28 @@ present_set_tree_pixmap(WindowPtr window,
>  }
>  
>  static void
> +present_restore_screen_pixmap(ScreenPtr screen)
> +{
> +    present_screen_priv_ptr screen_priv = present_screen_priv(screen);
> +    PixmapPtr screen_pixmap = (*screen->GetScreenPixmap)(screen);
> +    PixmapPtr flip_pixmap = screen_priv->flip_pixmap;
> +    WindowPtr flip_window = screen_priv->flip_window;
> +
> +    assert (flip_pixmap);
> +
> +    /* Update the screen pixmap with the current flip pixmap contents
> +     * Only do this the first time for a particular unflip operation, or
> +     * we'll probably scribble over other windows
> +     */
> +    if (screen->GetWindowPixmap(screen->root) == flip_pixmap)
> +        present_copy_region(&screen_pixmap->drawable, flip_pixmap, NULL, 0, 0);
> +
> +    if (flip_window)
> +        present_set_tree_pixmap(flip_window, flip_pixmap, screen_pixmap);
> +    present_set_tree_pixmap(screen->root, NULL, screen_pixmap);
> +}
> +
> +static void
>  present_set_abort_flip(ScreenPtr screen)
>  {
>      present_screen_priv_ptr screen_priv = present_screen_priv(screen);
> @@ -416,26 +438,11 @@ static void
>  present_unflip(ScreenPtr screen)
>  {
>      present_screen_priv_ptr screen_priv = present_screen_priv(screen);
> -    PixmapPtr pixmap = (*screen->GetScreenPixmap)(screen);
>  
>      assert (!screen_priv->unflip_event_id);
>      assert (!screen_priv->flip_pending);
>  
> -    /* Update the screen pixmap with the current flip pixmap contents
> -     * Only do this the first time for a particular unflip operation, or
> -     * we'll probably scribble over other windows
> -     */
> -    if (screen->GetWindowPixmap(screen->root) == screen_priv->flip_pixmap) {
> -        present_copy_region(&pixmap->drawable, screen_priv->flip_pixmap,
> -                            NULL, 0, 0);
> -    }
> -
> -    if (screen_priv->flip_pixmap && screen_priv->flip_window)
> -        present_set_tree_pixmap(screen_priv->flip_window,
> -                                screen_priv->flip_pixmap,
> -                                pixmap);
> -
> -    present_set_tree_pixmap(screen->root, NULL, pixmap);
> +    present_restore_screen_pixmap(screen);
>  
>      screen_priv->unflip_event_id = ++present_event_id;
>      DebugPresent(("u %lld\n", screen_priv->unflip_event_id));
> 


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer