[PATCH] dri2: Only invalidate our tree upon Pixmap changes

[Chris Wilson]

On Sun, Oct 26, 2014 at 05:23:42PM -0700, Keith Packard wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > x11perf -ucreate highlights an instance where compCreateWindow reassigns
> > the same Pixmap to the Window. Currently this triggers an expensive
> > invalidation of the entire window hierachy, making sure that DRI2
> > clients are kept informed of the handle changes. However, as the backing
> > Pixmap for the Window is actually unchanged, there is no need to
> > invalidate at all.
> 
> Might be better to just catch this in DIX and not call the
> SetWindowPixmap chain at all?

Possibly. I wasn't sure if anything on the callchain was interested in
knowing about calls to SetWindowPixmap() with the same pixmap. It seems
dubious but it might have been used as a notification hook somewhere. I
also wasn't sure if another path would also reset the old Pixmap -
though that possibility seems limited to miext/rootless. As I was
uncertain, I patched hw/xfree86/dri2.
 
> diff --git a/composite/compwindow.c b/composite/compwindow.c
> index 9a6b2da..77bdfa2 100644
> --- a/composite/compwindow.c
> +++ b/composite/compwindow.c
> @@ -567,10 +567,11 @@ compCreateWindow(WindowPtr pWin)
>      if (pWin->parent && ret) {
>          CompSubwindowsPtr csw = GetCompSubwindows(pWin->parent);
>          CompClientWindowPtr ccw;
> +        PixmapPtr parent_pixmap = (*pScreen->GetWindowPixmap)(pWin->parent);
> +        PixmapPtr window_pixmap = (*pScreen->GetWindowPixmap)(pWin);
>  
> -        (*pScreen->SetWindowPixmap) (pWin,
> -                                     (*pScreen->GetWindowPixmap) (pWin->
> -                                                                  parent));
> +        if (window_pixmap != parent_pixmap)
> +            (*pScreen->SetWindowPixmap) (pWin, parent_pixmap);
>          if (csw)
>              for (ccw = csw->clients; ccw; ccw = ccw->next)
>                  compRedirectWindow(clients[CLIENT_ID(ccw->id)],
> 

This patch also fixes the regression, so if you are happier with
applying the fix here,

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
Tested-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre