On Fri, Mar 25, 2011 at 12:35:37PM +0100, ext Michel Dänzer wrote:
> From: Michel Dänzer <[email protected]>
> 
> 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.

BTW I'm toying around with offscreen flipping, and for that I'm using
the following patch to also invalidate the window's pixmap (the patch
is against our internal codebase which has the DRI2 ref counting
patches from Pauli and Christopher).

>From 697edc6a648be4ba6de4394ea21d968caf75d4cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <[email protected]>
Date: Mon, 7 Feb 2011 20:52:57 +0200
Subject: [PATCH] dri2: Invalidate window pixmaps
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

Signed-off-by: Ville Syrjälä <[email protected]>
---
 hw/xfree86/dri2/dri2.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

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
@@ -673,6 +673,17 @@ DRI2InvalidateDrawable(DRI2DrawablePtr pPriv)
        ref->invalidate(pPriv, ref->priv, ref->id);
 }
 
+static void
+DRI2InvalidateWindowPixmap(WindowPtr pWin)
+{
+    ScreenPtr pScreen = pWin->drawable.pScreen;
+    PixmapPtr pPixmap = pScreen->GetWindowPixmap(pWin);
+    DRI2DrawablePtr pPriv = DRI2GetDrawable(&pPixmap->drawable);
+
+    if (pPriv)
+       DRI2InvalidateDrawable(pPriv);
+}
+
 /*
  * In the direct rendered case, we throttle the clients that have more
  * than their share of outstanding swaps (and thus busy buffers) when a
@@ -1071,6 +1082,8 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, 
CARD64 target_msc,
     *swap_target = pPriv->swap_count + pPriv->swapsPending;
 
     DRI2InvalidateDrawable(pPriv);
+    if (pDraw->type == DRAWABLE_WINDOW)
+       DRI2InvalidateWindowPixmap((WindowPtr)pDraw);
 
     DRI2CopyFrontToFakeFront(pDraw, pPriv);
     return Success;
@@ -1248,6 +1261,7 @@ DRI2ConfigNotify(WindowPtr pWin, int x, int y, int w, int 
h, int bw,
        return Success;
 
     DRI2InvalidateDrawable(DRI2GetDrawable(pDraw));
+    DRI2InvalidateWindowPixmap(pWin);
     return Success;
 }
 
-- 
1.7.3.4

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.

> 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. But I suppose your solution
fits better into the current dri2 design.

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. It would also allow the driver to make a copy
from the old buffer to the new buffer in case the client had already
started rendering. OTOH I suppose the client could just avoid
re-getting the buffers after it's started rendering. And I suppose
the copy could also be done by adding a new hook to the current dri2
interface.

> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=35452 .
> 
> Signed-off-by: Michel Dänzer <[email protected]>
> ---
>  hw/xfree86/dri2/dri2.c |   19 ++++++++++++++++++-
>  1 files changed, 18 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index d8851f2..c229959 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -339,6 +339,7 @@ allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr 
> ds,
>      int old_buf = find_attachment(pPriv, attachment);
>  
>      if ((old_buf < 0)
> +     || attachment == DRI2BufferFrontLeft
>       || !dimensions_match
>       || (pPriv->buffers[old_buf]->format != format)) {
>       *buffer = (*ds->CreateBuffer)(pDraw, attachment, format);
> @@ -814,6 +815,15 @@ DRI2WaitSwap(ClientPtr client, DrawablePtr pDrawable)
>      return FALSE;
>  }
>  
> +static int
> +DRI2SwapWalk(WindowPtr pWin, pointer data)
> +{
> +    if (pWin->drawable.pScreen->GetWindowPixmap(pWin) == data)
> +     DRI2InvalidateDrawable(&pWin->drawable);
> +
> +    return WT_WALKCHILDREN;
> +}
> +
>  int
>  DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>               CARD64 divisor, CARD64 remainder, CARD64 *swap_target,
> @@ -914,7 +924,14 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, 
> CARD64 target_msc,
>       */
>      *swap_target = pPriv->swap_count + pPriv->swapsPending;
>  
> -    DRI2InvalidateDrawable(pDraw);
> +    if (pDraw->type == DRAWABLE_WINDOW) {
> +     /* Invalidate DRI2 buffers for all windows with the same window pixmap
> +      * as this one.
> +      */
> +     WalkTree(pScreen, DRI2SwapWalk,
> +              pScreen->GetWindowPixmap((WindowPtr)pDraw));
> +    } else
> +     DRI2InvalidateDrawable(pDraw);
>  
>      return Success;
>  }
> -- 
> 1.7.4.1
> 
> _______________________________________________
> [email protected]: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

-- 
Ville Syrjälä
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to