[PATCH xserver] dri2: Invalidate DRI2 buffers for all windows with the same pixmap on swap.

[Michel Dänzer]

On Fre, 2011-03-25 at 14:47 +0200, Ville Syrjälä wrote: 
> On Fri, Mar 25, 2011 at 12:35:37PM +0100, ext Michel Dänzer wrote:
> > From: Michel Dänzer <daenzer at vmware.com>
> > 
> > Without this, when a compositing manager unredirects a fullscreen window which
> > uses DRI2 and page flipping, the DRI2 buffer information for the compositing
> > manager's output window (typically the Composite Overlay Window or root window)
> > may become stale, resulting in all kinds of hilarity.
> 
> Make sense to me.

Thanks, does that mean I have your Reviewed-by: ?

> BTW I'm toying around with offscreen flipping, and for that I'm using
> the following patch to also invalidate the window's pixmap [...]

Makes sense to me as well.


> Subject: [PATCH] dri2: Invalidate window pixmaps
> 
> While a redirected window is flipped, it's pixmap may still be used as
> and EGL image and should also get invalidated. When sending invalidate
> events for a window, also send the events for it's pixmap.

it's -> its

in both cases.


> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index 03011ff..5faf042 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -1248,6 +1261,7 @@ DRI2ConfigNotify(WindowPtr pWin, int x, int y, int w, int h, int bw,
>  	return Success;
>  
>      DRI2InvalidateDrawable(DRI2GetDrawable(pDraw));
> +    DRI2InvalidateWindowPixmap(pWin);

Could just do something like

    DRI2InvalidateDrawable(&pScreen->GetWindowPixmap(pWin)->drawable);

either in DRI2InvalidateWindowPixmap() or in its callers directly.


> I do wonder if we could always attach the ref list to the window pixmap
> instead of the window itself. That would avoid the need to walk the
> whole window tree when a flip occurs. But I didn't look into this any
> further, so I'm not sure how we would handle cases where the window
> pixmap changes.

I think the problems start earlier than that. E.g. unredirected windows
all share the screen pixmap, but the DRI2 buffers can't be shared
between windows in general.


> > Also re-generate the real front buffer information every time the client asks
> > for it, or we might keep around stale cached information.
> 
> For offscreen flipping I solved this by always updating the buffer
> information in the ReuseBufferNotify() hook.

That hasn't landed in master yet, has it?


> BTW I don't really like the current dri2 design. The old interface with
> one CreateBuffers call instead of multiple CreateBuffer calls seemed
> better. It made it easier to do some things in the driver code. For
> example the depth_pixmap handling in the ati driver looks broken with
> the new dri2 interface.

Quite possibly. I think the DRI2 driver interface generally puts too
much logic in the drivers. So e.g. the drivers do flipping in a hackish
way internally, which is why this patch needs to always re-generate the
front buffer information.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer