[PATCH 2/9] dri2: Refactor interface to take DRI2DrawablePtr

Christopher James Halse Rogers christopher.halse.rogers at canonical.com
Thu Feb 3 18:33:01 PST 2011


On Thu, 2011-02-03 at 19:48 +0200, Pauli wrote:
> From: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> 
> DRI2 swaps may complete after Drawable has been destroyed. This causes
> memory management problems as DRI2 internal state is tighly coupled with
> Drawable.
> 
> DRI2DrawablePtr can be used to access DRI2 functionality. This provides
> option that DRI2 drawable can live longer than underlying Drawable.
> 
> Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> ---
>  glx/glxdri2.c             |   23 +++++---
>  hw/xfree86/dri2/dri2.c    |  131 +++++++++++++++++----------------------------
>  hw/xfree86/dri2/dri2.h    |   47 ++++++++++++-----
>  hw/xfree86/dri2/dri2ext.c |  116 +++++++++++++++++++++++++---------------
>  4 files changed, 170 insertions(+), 147 deletions(-)
> 
> diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> index 8d21c93..027615a 100644
> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -88,6 +88,7 @@ struct __GLXDRIdrawable {
>      __GLXdrawable	 base;
>      __DRIdrawable	*driDrawable;
>      __GLXDRIscreen	*screen;
> +    DRI2DrawablePtr     pDRI2Draw;
>  
>      /* Dimensions as last reported by DRI2GetBuffers. */
>      int width;
> @@ -123,7 +124,7 @@ __glXDRIdrawableCopySubBuffer(__GLXdrawable *drawable,
>      box.y2 = private->height - y;
>      RegionInit(&region, &box, 0);
>  
> -    DRI2CopyRegion(drawable->pDraw, &region,
> +    DRI2CopyRegion(private->pDRI2Draw, &region,
>  		   DRI2BufferFrontLeft, DRI2BufferBackLeft);
>  }
>  
> @@ -140,7 +141,7 @@ __glXDRIdrawableWaitX(__GLXdrawable *drawable)
>      box.y2 = private->height;
>      RegionInit(&region, &box, 0);
>  
> -    DRI2CopyRegion(drawable->pDraw, &region,
> +    DRI2CopyRegion(private->pDRI2Draw, &region,
>  		   DRI2BufferFakeFrontLeft, DRI2BufferFrontLeft);
>  }
>  
> @@ -157,7 +158,7 @@ __glXDRIdrawableWaitGL(__GLXdrawable *drawable)
>      box.y2 = private->height;
>      RegionInit(&region, &box, 0);
>  
> -    DRI2CopyRegion(drawable->pDraw, &region,
> +    DRI2CopyRegion(private->pDRI2Draw, &region,
>  		   DRI2BufferFrontLeft, DRI2BufferFakeFrontLeft);
>  }
>  
> @@ -220,7 +221,7 @@ __glXDRIdrawableSwapBuffers(ClientPtr client, __GLXdrawable *drawable)
>  	(*screen->flush->flushInvalidate)(priv->driDrawable);
>  #endif
>  
> -    if (DRI2SwapBuffers(client, drawable->pDraw, 0, 0, 0, &unused,
> +    if (DRI2SwapBuffers(client, priv->pDRI2Draw, 0, 0, 0, &unused,
>  			__glXdriSwapEvent, drawable->pDraw) != Success)
>  	return FALSE;
>  
> @@ -230,10 +231,11 @@ __glXDRIdrawableSwapBuffers(ClientPtr client, __GLXdrawable *drawable)
>  static int
>  __glXDRIdrawableSwapInterval(__GLXdrawable *drawable, int interval)
>  {
> +    __GLXDRIdrawable *priv = (__GLXDRIdrawable *) drawable;
>      if (interval <= 0) /* || interval > BIGNUM? */
>  	return GLX_BAD_VALUE;
>  
> -    DRI2SwapInterval(drawable->pDraw, interval);
> +    DRI2SwapInterval(priv->pDRI2Draw, interval);
>  
>      return 0;
>  }
> @@ -300,7 +302,8 @@ static Bool
>  __glXDRIcontextWait(__GLXcontext *baseContext,
>  		    __GLXclientState *cl, int *error)
>  {
> -    if (DRI2WaitSwap(cl->client, baseContext->drawPriv->pDraw)) {
> +    __GLXDRIdrawable *priv = (__GLXDRIdrawable *) baseContext->drawPriv;
> +    if (DRI2WaitSwap(cl->client, priv->pDRI2Draw)) {
>  	*error = cl->client->noClientException;
>  	return TRUE;
>      }
> @@ -428,7 +431,7 @@ __glXDRIscreenCreateContext(__GLXscreen *baseScreen,
>  }
>  
>  static void
> -__glXDRIinvalidateBuffers(DrawablePtr pDraw, void *priv)
> +__glXDRIinvalidateBuffers(DRI2DrawablePtr pDRI2Draw, void *priv, XID id)
>  {
>  #if __DRI2_FLUSH_VERSION >= 3
>      __GLXDRIdrawable *private = priv;
> @@ -475,6 +478,8 @@ __glXDRIscreenCreateDrawable(ClientPtr client,
>  	    return NULL;
>      }
>  
> +    private->pDRI2Draw = DRI2GetDrawable(pDraw);
> +
>      private->driDrawable =
>  	(*driScreen->dri2->createNewDrawable)(driScreen->driScreen,
>  					      config->driConfig, private);
> @@ -493,7 +498,7 @@ dri2GetBuffers(__DRIdrawable *driDrawable,
>      int i;
>      int j;
>  
> -    buffers = DRI2GetBuffers(private->base.pDraw,
> +    buffers = DRI2GetBuffers(private->pDRI2Draw,
>  			     width, height, attachments, count, out_count);
>      if (*out_count > MAX_DRAWABLE_BUFFERS) {
>  	*out_count = 0;
> @@ -537,7 +542,7 @@ dri2GetBuffersWithFormat(__DRIdrawable *driDrawable,
>      int i;
>      int j = 0;
>  
> -    buffers = DRI2GetBuffersWithFormat(private->base.pDraw,
> +    buffers = DRI2GetBuffersWithFormat(private->pDRI2Draw,
>  				       width, height, attachments, count,
>  				       out_count);
>      if (*out_count > MAX_DRAWABLE_BUFFERS) {
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index 66d217c..91ae1a0 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -60,7 +60,7 @@ static DevPrivateKeyRec dri2WindowPrivateKeyRec;
>  static DevPrivateKeyRec dri2PixmapPrivateKeyRec;
>  #define dri2PixmapPrivateKey (&dri2PixmapPrivateKeyRec)
>  
> -static RESTYPE       dri2DrawableRes;
> +RESTYPE       dri2DrawableRes;
>  
>  typedef struct _DRI2Screen *DRI2ScreenPtr;
>  
> @@ -83,7 +83,7 @@ typedef struct _DRI2Drawable {
>      CARD64		 last_swap_ust; /* ust at completion of most recent swap */
>      int			 swap_limit; /* for N-buffering */
>      unsigned long	 serialNumber;
> -} DRI2DrawableRec, *DRI2DrawablePtr;
> +} DRI2DrawableRec;
>  
>  typedef struct _DRI2Screen {
>      ScreenPtr			 screen;
> @@ -113,7 +113,7 @@ DRI2GetScreen(ScreenPtr pScreen)
>      return dixLookupPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey);
>  }
>  
> -static DRI2DrawablePtr
> +DRI2DrawablePtr
>  DRI2GetDrawable(DrawablePtr pDraw)
>  {
>      WindowPtr pWin;
> @@ -131,6 +131,12 @@ DRI2GetDrawable(DrawablePtr pDraw)
>      }
>  }
>  
> +DrawablePtr
> +DRI2DrawableGetDrawable(DRI2DrawablePtr pPriv)
> +{
> +    return pPriv->drawable;
> +}
> +
>  static unsigned long
>  DRI2DrawableSerial(DrawablePtr pDraw)
>  {
> @@ -271,13 +277,9 @@ DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID id,
>  }
>  
>  int
> -DRI2DestroyDrawable(ClientPtr client, DrawablePtr pDraw, XID id)
> +DRI2DestroyDrawable(ClientPtr client, DRI2DrawablePtr pPriv, XID id)
>  {
>      DRI2DrawableRefPtr ref;
> -    DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw);
> -
> -    if (!pPriv)
> -	return BadDrawable;
>  
>      ref = DRI2LookupClientDrawableRef(pPriv, client, id);
>  
> @@ -408,12 +410,12 @@ update_dri2_drawable_buffers(DRI2DrawablePtr pPriv, DrawablePtr pDraw,
>  }
>  
>  static DRI2BufferPtr *
> -do_get_buffers(DrawablePtr pDraw, int *width, int *height,
> +do_get_buffers(DRI2DrawablePtr pPriv, int *width, int *height,
>  	       unsigned int *attachments, int count, int *out_count,
>  	       int has_format)
>  {
> -    DRI2ScreenPtr   ds = DRI2GetScreen(pDraw->pScreen);
> -    DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw);
> +    DrawablePtr     pDraw = pPriv->drawable;
> +    DRI2ScreenPtr   ds = pPriv->dri2_screen;
>      DRI2BufferPtr  *buffers;
>      int need_real_front = 0;
>      int need_fake_front = 0;
> @@ -423,7 +425,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>      int buffers_changed = 0;
>      int i;
>  
> -    if (!pPriv) {
> +    if (!pDraw) {
>  	*width = pDraw->width;
>  	*height = pDraw->height;
>  	*out_count = 0;
> @@ -517,7 +519,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>  	box.y2 = pPriv->height;
>  	RegionInit(&region, &box, 0);
>  
> -	DRI2CopyRegion(pDraw, &region, DRI2BufferFakeFrontLeft,
> +	DRI2CopyRegion(pPriv, &region, DRI2BufferFakeFrontLeft,
>  		       DRI2BufferFrontLeft);
>      }
>  
> @@ -541,32 +543,28 @@ err_out:
>  }
>  
>  DRI2BufferPtr *
> -DRI2GetBuffers(DrawablePtr pDraw, int *width, int *height,
> +DRI2GetBuffers(DRI2DrawablePtr pPriv, int *width, int *height,
>  	       unsigned int *attachments, int count, int *out_count)
>  {
> -    return do_get_buffers(pDraw, width, height, attachments, count,
> +    return do_get_buffers(pPriv, width, height, attachments, count,
>  			  out_count, FALSE);
>  }
>  
>  DRI2BufferPtr *
> -DRI2GetBuffersWithFormat(DrawablePtr pDraw, int *width, int *height,
> +DRI2GetBuffersWithFormat(DRI2DrawablePtr pPriv, int *width, int *height,
>  			 unsigned int *attachments, int count, int *out_count)
>  {
> -    return do_get_buffers(pDraw, width, height, attachments, count,
> +    return do_get_buffers(pPriv, width, height, attachments, count,
>  			  out_count, TRUE);
>  }
>  
>  static void
> -DRI2InvalidateDrawable(DrawablePtr pDraw)
> +DRI2InvalidateDrawable(DRI2DrawablePtr pPriv)
>  {
> -    DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw);
>      DRI2DrawableRefPtr ref;
>  
> -    if (!pPriv)
> -        return;
> -
>      list_for_each_entry(ref, &pPriv->reference_list, link)
> -	ref->invalidate(pDraw, ref->priv);
> +	ref->invalidate(pPriv, ref->priv, ref->id);
>  }
>  
>  /*
> @@ -577,14 +575,8 @@ DRI2InvalidateDrawable(DrawablePtr pDraw)
>   * comes in (see __glXDRIcontextWait()).
>   */
>  Bool
> -DRI2ThrottleClient(ClientPtr client, DrawablePtr pDraw)
> +DRI2ThrottleClient(ClientPtr client, DRI2DrawablePtr pPriv)
>  {
> -    DRI2DrawablePtr pPriv;
> -
> -    pPriv = DRI2GetDrawable(pDraw);
> -    if (pPriv == NULL)
> -	return FALSE;
> -
>      /* Throttle to swap limit */
>      if ((pPriv->swapsPending >= pPriv->swap_limit) &&
>  	!pPriv->blockedClient) {
> @@ -621,16 +613,15 @@ DRI2BlockClient(ClientPtr client, DrawablePtr pDraw)
>  }
>  
>  int
> -DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion,
> +DRI2CopyRegion(DRI2DrawablePtr pPriv, RegionPtr pRegion,
>  	       unsigned int dest, unsigned int src)
>  {
> -    DRI2ScreenPtr   ds = DRI2GetScreen(pDraw->pScreen);
> -    DRI2DrawablePtr pPriv;
> +    DrawablePtr     pDraw = pPriv->drawable;
> +    DRI2ScreenPtr   ds = pPriv->dri2_screen;
>      DRI2BufferPtr   pDestBuffer, pSrcBuffer;
>      int		    i;
>  
> -    pPriv = DRI2GetDrawable(pDraw);
> -    if (pPriv == NULL)
> +    if (pDraw == NULL)
>  	return BadDrawable;
>  
>      pDestBuffer = NULL;
> @@ -713,14 +704,12 @@ DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw, int frame,
>  }
>  
>  static void
> -DRI2WakeClient(ClientPtr client, DrawablePtr pDraw, int frame,
> +DRI2WakeClient(ClientPtr client, DRI2DrawablePtr pPriv, int frame,
>  	       unsigned int tv_sec, unsigned int tv_usec)
>  {
> -    ScreenPtr	    pScreen = pDraw->pScreen;
> -    DRI2DrawablePtr pPriv;
> +    ScreenPtr	    pScreen = pPriv->dri2_screen->screen;
>  
> -    pPriv = DRI2GetDrawable(pDraw);
> -    if (pPriv == NULL) {
> +    if (pPriv->drawable == NULL) {
>          xf86DrvMsg(pScreen->myNum, X_ERROR,
>  		   "[DRI2] %s: bad drawable\n", __func__);
>  	return;
> @@ -776,7 +765,7 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame,
>      box.x2 = pDraw->width;
>      box.y2 = pDraw->height;
>      RegionInit(&region, &box, 0);
> -    DRI2CopyRegion(pDraw, &region, DRI2BufferFakeFrontLeft,
> +    DRI2CopyRegion(pPriv, &region, DRI2BufferFakeFrontLeft,
>  		   DRI2BufferFrontLeft);
>  
>      ust = ((CARD64)tv_sec * 1000000) + tv_usec;
> @@ -786,14 +775,12 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame,
>      pPriv->last_swap_msc = frame;
>      pPriv->last_swap_ust = ust;
>  
> -    DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec);
> +    DRI2WakeClient(client, pPriv, frame, tv_sec, tv_usec);
>  }
>  
>  Bool
> -DRI2WaitSwap(ClientPtr client, DrawablePtr pDrawable)
> +DRI2WaitSwap(ClientPtr client, DRI2DrawablePtr pPriv)
>  {
> -    DRI2DrawablePtr pPriv = DRI2GetDrawable(pDrawable);
> -
>      /* If we're currently waiting for a swap on this drawable, reset
>       * the request and suspend the client.  We only support one
>       * blocked client per drawable. */
> @@ -809,19 +796,18 @@ DRI2WaitSwap(ClientPtr client, DrawablePtr pDrawable)
>  }
>  
>  int
> -DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
> +DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
>  		CARD64 divisor, CARD64 remainder, CARD64 *swap_target,
>  		DRI2SwapEventPtr func, void *data)
>  {
> -    ScreenPtr       pScreen = pDraw->pScreen;
> -    DRI2ScreenPtr   ds = DRI2GetScreen(pDraw->pScreen);
> -    DRI2DrawablePtr pPriv;
> +    DrawablePtr     pDraw = pPriv->drawable;
> +    ScreenPtr       pScreen = pPriv->dri2_screen->screen;
> +    DRI2ScreenPtr   ds = pPriv->dri2_screen;
>      DRI2BufferPtr   pDestBuffer = NULL, pSrcBuffer = NULL;
>      int             ret, i;
>      CARD64          ust, current_msc;
>  
> -    pPriv = DRI2GetDrawable(pDraw);
> -    if (pPriv == NULL) {
> +    if (pDraw == NULL) {
>          xf86DrvMsg(pScreen->myNum, X_ERROR,
>  		   "[DRI2] %s: bad drawable\n", __func__);
>  	return BadDrawable;
> @@ -908,37 +894,27 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>       */
>      *swap_target = pPriv->swap_count + pPriv->swapsPending;
>  
> -    DRI2InvalidateDrawable(pDraw);
> +    DRI2InvalidateDrawable(pPriv);
>  
>      return Success;
>  }
>  
>  void
> -DRI2SwapInterval(DrawablePtr pDrawable, int interval)
> +DRI2SwapInterval(DRI2DrawablePtr pPriv, int interval)
>  {
> -    ScreenPtr       pScreen = pDrawable->pScreen;
> -    DRI2DrawablePtr pPriv = DRI2GetDrawable(pDrawable);
> -
> -    if (pPriv == NULL) {
> -        xf86DrvMsg(pScreen->myNum, X_ERROR,
> -		   "[DRI2] %s: bad drawable\n", __func__);
> -	return;
> -    }
> -
>      /* fixme: check against arbitrary max? */
>      pPriv->swap_interval = interval;
>  }
>  
>  int
> -DRI2GetMSC(DrawablePtr pDraw, CARD64 *ust, CARD64 *msc, CARD64 *sbc)
> +DRI2GetMSC(DRI2DrawablePtr pPriv, CARD64 *ust, CARD64 *msc, CARD64 *sbc)
>  {
> -    ScreenPtr pScreen = pDraw->pScreen;
> -    DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen);
> -    DRI2DrawablePtr pPriv;
> +    ScreenPtr pScreen = pPriv->dri2_screen->screen;
> +    DRI2ScreenPtr ds = pPriv->dri2_screen;
> +    DrawablePtr pDraw = pPriv->drawable;
>      Bool ret;
>  
> -    pPriv = DRI2GetDrawable(pDraw);
> -    if (pPriv == NULL) {
> +    if (pDraw == NULL) {
>          xf86DrvMsg(pScreen->myNum, X_ERROR,
>  		   "[DRI2] %s: bad drawable\n", __func__);
>  	return BadDrawable;
> @@ -966,15 +942,14 @@ DRI2GetMSC(DrawablePtr pDraw, CARD64 *ust, CARD64 *msc, CARD64 *sbc)
>  }
>  
>  int
> -DRI2WaitMSC(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
> +DRI2WaitMSC(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
>  	    CARD64 divisor, CARD64 remainder)
>  {
> -    DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen);
> -    DRI2DrawablePtr pPriv;
> +    DRI2ScreenPtr ds = pPriv->dri2_screen;
> +    DrawablePtr   pDraw = pPriv->drawable;
>      Bool ret;
>  
> -    pPriv = DRI2GetDrawable(pDraw);
> -    if (pPriv == NULL)
> +    if (pDraw == NULL)
>  	return BadDrawable;
>  
>      /* Old DDX just completes immediately */
> @@ -992,14 +967,8 @@ DRI2WaitMSC(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>  }
>  
>  int
> -DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc)
> +DRI2WaitSBC(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_sbc)
>  {
> -    DRI2DrawablePtr pPriv;
> -
> -    pPriv = DRI2GetDrawable(pDraw);
> -    if (pPriv == NULL)
> -	return BadDrawable;
> -
>      /* target_sbc == 0 means to block until all pending swaps are
>       * finished. Recalculate target_sbc to get that behaviour.
>       */
> @@ -1086,7 +1055,7 @@ DRI2ConfigNotify(WindowPtr pWin, int x, int y, int w, int h, int bw,
>      if (!dd || (dd->width == w && dd->height == h))
>  	return Success;
>  
> -    DRI2InvalidateDrawable(pDraw);
> +    DRI2InvalidateDrawable(DRI2GetDrawable(pDraw));
>      return Success;
>  }
>  
> diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
> index dd63c30..509d4f6 100644
> --- a/hw/xfree86/dri2/dri2.h
> +++ b/hw/xfree86/dri2/dri2.h
> @@ -46,6 +46,10 @@ typedef struct {
>      void *driverPrivate;
>  } DRI2BufferRec, *DRI2BufferPtr;
>  
> +struct _DRI2Drawable;
> +
> +typedef struct _DRI2Drawable *DRI2DrawablePtr;
> +
>  extern CARD8 dri2_major; /* version of DRI2 supported by DDX */
>  extern CARD8 dri2_minor;
>  
> @@ -155,8 +159,9 @@ typedef int		(*DRI2ScheduleWaitMSCProcPtr)(ClientPtr client,
>  						      CARD64 divisor,
>  						      CARD64 remainder);
>  
> -typedef void		(*DRI2InvalidateProcPtr)(DrawablePtr pDraw,
> -						 void *data);
> +typedef void		(*DRI2InvalidateProcPtr)(DRI2DrawablePtr pPriv,
> +						 void *data,
> +						 XID id);
>  
>  /**
>   * Version of the DRI2InfoRec structure defined in this header
> @@ -215,17 +220,17 @@ extern _X_EXPORT int DRI2CreateDrawable(ClientPtr client,
>  					void *priv);
>  
>  extern _X_EXPORT int DRI2DestroyDrawable(ClientPtr client,
> -                                         DrawablePtr pDraw,
> +                                         DRI2DrawablePtr pPriv,
>                                           XID id);
>  
> -extern _X_EXPORT DRI2BufferPtr *DRI2GetBuffers(DrawablePtr pDraw,
> +extern _X_EXPORT DRI2BufferPtr *DRI2GetBuffers(DRI2DrawablePtr pPriv,
>  			     int *width,
>  			     int *height,
>  			     unsigned int *attachments,
>  			     int count,
>  			     int *out_count);
>  
> -extern _X_EXPORT int DRI2CopyRegion(DrawablePtr pDraw,
> +extern _X_EXPORT int DRI2CopyRegion(DRI2DrawablePtr pPriv,
>  		   RegionPtr pRegion,
>  		   unsigned int dest,
>  		   unsigned int src);
> @@ -248,27 +253,27 @@ extern _X_EXPORT int DRI2CopyRegion(DrawablePtr pDraw,
>   */
>  extern _X_EXPORT void DRI2Version(int *major, int *minor);
>  
> -extern _X_EXPORT DRI2BufferPtr *DRI2GetBuffersWithFormat(DrawablePtr pDraw,
> +extern _X_EXPORT DRI2BufferPtr *DRI2GetBuffersWithFormat(DRI2DrawablePtr pPriv,
>  	int *width, int *height, unsigned int *attachments, int count,
>  	int *out_count);
>  
> -extern _X_EXPORT void DRI2SwapInterval(DrawablePtr pDrawable, int interval);
> -extern _X_EXPORT int DRI2SwapBuffers(ClientPtr client, DrawablePtr pDrawable,
> +extern _X_EXPORT void DRI2SwapInterval(DRI2DrawablePtr pPriv, int interval);
> +extern _X_EXPORT int DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv,
>  				     CARD64 target_msc, CARD64 divisor,
>  				     CARD64 remainder, CARD64 *swap_target,
>  				     DRI2SwapEventPtr func, void *data);
> -extern _X_EXPORT Bool DRI2WaitSwap(ClientPtr client, DrawablePtr pDrawable);
> +extern _X_EXPORT Bool DRI2WaitSwap(ClientPtr client, DRI2DrawablePtr pPriv);
>  
> -extern _X_EXPORT int DRI2GetMSC(DrawablePtr pDrawable, CARD64 *ust,
> +extern _X_EXPORT int DRI2GetMSC(DRI2DrawablePtr pPriv, CARD64 *ust,
>  				CARD64 *msc, CARD64 *sbc);
> -extern _X_EXPORT int DRI2WaitMSC(ClientPtr client, DrawablePtr pDrawable,
> +extern _X_EXPORT int DRI2WaitMSC(ClientPtr client, DRI2DrawablePtr pPriv,
>  				 CARD64 target_msc, CARD64 divisor,
>  				 CARD64 remainder);
>  extern _X_EXPORT int ProcDRI2WaitMSCReply(ClientPtr client, CARD64 ust,
>  					  CARD64 msc, CARD64 sbc);
> -extern _X_EXPORT int DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw,
> +extern _X_EXPORT int DRI2WaitSBC(ClientPtr client, DRI2DrawablePtr pPriv,
>  				 CARD64 target_sbc);
> -extern _X_EXPORT Bool DRI2ThrottleClient(ClientPtr client, DrawablePtr pDraw);
> +extern _X_EXPORT Bool DRI2ThrottleClient(ClientPtr client, DRI2DrawablePtr pPriv);
>  
>  extern _X_EXPORT Bool DRI2CanFlip(DrawablePtr pDraw);
>  
> @@ -285,5 +290,21 @@ extern _X_EXPORT void DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw,
>  extern _X_EXPORT void DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw,
>  					  int frame, unsigned int tv_sec,
>  					  unsigned int tv_usec);
> +/**
> + * Provides access to DrawablePtr trough DRI2DrawablePtr
> + *
> + * If Drawable is window and already destroyed return value is NULL. Otherwise
> + * returned pointer is valid DrawablePtr.
> + *
> + * \param pPriv DRI2 private drawable
> + */
> +extern _X_EXPORT DrawablePtr DRI2DrawableGetDrawable(DRI2DrawablePtr pPriv);
>  
> +/**
> + * Provides access to DRI2DrawablePtr trough DrawablePtr
> + *
> + * \param pDraw drawable pointer
> + * \return Valid DRI2DrawablePtr if DRI2Drawable exists. Otherwise NULL.
> + */
> +extern _X_EXPORT DRI2DrawablePtr DRI2GetDrawable(DrawablePtr pDraw);
>  #endif
> diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c
> index c7e6be1..639bbc5 100644
> --- a/hw/xfree86/dri2/dri2ext.c
> +++ b/hw/xfree86/dri2/dri2ext.c
> @@ -50,6 +50,33 @@
>  #include "xf86Module.h"
>  
>  static ExtensionEntry	*dri2Extension;
> +extern RESTYPE       dri2DrawableRes;
> +
> +static Bool
> +validDRI2Drawable(ClientPtr client, XID id, Mask access,
> +		  DRI2DrawablePtr *pPriv, int *status)
> +{
> +    DRI2DrawablePtr pTmp;
> +    int rc;
> +
> +    if (id == INVALID) {
> +	*status = BadDrawable;
> +	return FALSE;
> +    }
> +
> +    rc = dixLookupResourceByType((pointer*)&pTmp, id, dri2DrawableRes, client, access);
> +
> +    if (rc == BadValue) {
> +	*status = BadDrawable;
> +	return FALSE;
> +    }
> +    *status = rc;
> +    if (rc != Success)
> +	return FALSE;
> +
> +    *pPriv = pTmp;
> +    return TRUE;
> +}
>  
>  static Bool
>  validDrawable(ClientPtr client, XID drawable, Mask access_mode,
> @@ -156,13 +183,13 @@ ProcDRI2Authenticate(ClientPtr client)
>  }
>  
>  static void
> -DRI2InvalidateBuffersEvent(DrawablePtr pDraw, void *priv)
> +DRI2InvalidateBuffersEvent(DRI2DrawablePtr pPriv, void *priv, XID id)
>  {
>      xDRI2InvalidateBuffers event;
>      ClientPtr client = priv;
>  
>      event.type = DRI2EventBase + DRI2_InvalidateBuffers;
> -    event.drawable = pDraw->id;
> +    event.drawable = id;
>  
>      WriteEventsToClient(client, 1, (xEvent *)&event);
>  }
> @@ -192,30 +219,31 @@ static int
>  ProcDRI2DestroyDrawable(ClientPtr client)
>  {
>      REQUEST(xDRI2DestroyDrawableReq);
> -    DrawablePtr pDrawable;
> +    DRI2DrawablePtr pPriv;
>      int status;
>  
>      REQUEST_SIZE_MATCH(xDRI2DestroyDrawableReq);
> -    if (!validDrawable(client, stuff->drawable, DixRemoveAccess,
> -		       &pDrawable, &status))
> +    if (!validDRI2Drawable(client, stuff->drawable, DixRemoveAccess,
> +		       &pPriv, &status))
>  	return status;
>  
> -    return DRI2DestroyDrawable(client, pDrawable, stuff->drawable);
> +    return DRI2DestroyDrawable(client, pPriv, stuff->drawable);
>  }
>  
> 
>  static int
> -send_buffers_reply(ClientPtr client, DrawablePtr pDrawable,
> +send_buffers_reply(ClientPtr client, DRI2DrawablePtr pPriv,
>  		   DRI2BufferPtr *buffers, int count, int width, int height)
>  {
>      xDRI2GetBuffersReply rep;
>      int skip = 0;
>      int i;
> +    DrawablePtr pDrawable = DRI2DrawableGetDrawable(pPriv);
>  
>      if (buffers == NULL)
>  	    return BadAlloc;
>  
> -    if (pDrawable->type == DRAWABLE_WINDOW) {
> +    if (!pDrawable || pDrawable->type == DRAWABLE_WINDOW) {
>  	for (i = 0; i < count; i++) {
>  	    /* Do not send the real front buffer of a window to the client.
>  	     */

The comment doesn't quite make sense at this point of the patch series,
although later patches mean pDrawable can be null IFF the DRI2Drawable
was created from a DRAWABLE_WINDOW.

It might be nice to add that to the comment, otherwise it's not clear
that !pDrawable implies pDrawable->type == DRAWABLE_WINDOW used to be
true.

> @@ -239,7 +267,7 @@ send_buffers_reply(ClientPtr client, DrawablePtr pDrawable,
>  
>  	/* Do not send the real front buffer of a window to the client.
>  	 */
> -	if ((pDrawable->type == DRAWABLE_WINDOW)
> +	if ((!pDrawable || pDrawable->type == DRAWABLE_WINDOW)
>  	    && (buffers[i]->attachment == DRI2BufferFrontLeft)) {
>  	    continue;
>  	}
> @@ -259,25 +287,25 @@ static int
>  ProcDRI2GetBuffers(ClientPtr client)
>  {
>      REQUEST(xDRI2GetBuffersReq);
> -    DrawablePtr pDrawable;
> +    DRI2DrawablePtr pPriv;
>      DRI2BufferPtr *buffers;
>      int status, width, height, count;
>      unsigned int *attachments;
>  
>      REQUEST_FIXED_SIZE(xDRI2GetBuffersReq, stuff->count * 4);
> -    if (!validDrawable(client, stuff->drawable, DixReadAccess | DixWriteAccess,
> -		       &pDrawable, &status))
> +    if (!validDRI2Drawable(client, stuff->drawable, DixReadAccess | DixWriteAccess,
> +		       &pPriv, &status))
>  	return status;
>  
> -    if (DRI2ThrottleClient(client, pDrawable))
> +    if (DRI2ThrottleClient(client, pPriv))
>  	return Success;
>  
>      attachments = (unsigned int *) &stuff[1];
> -    buffers = DRI2GetBuffers(pDrawable, &width, &height,
> +    buffers = DRI2GetBuffers(pPriv, &width, &height,
>  			     attachments, stuff->count, &count);
>  
> 
> -    return send_buffers_reply(client, pDrawable, buffers, count, width, height);
> +    return send_buffers_reply(client, pPriv, buffers, count, width, height);
>  
>  }
>  
> @@ -285,24 +313,24 @@ static int
>  ProcDRI2GetBuffersWithFormat(ClientPtr client)
>  {
>      REQUEST(xDRI2GetBuffersReq);
> -    DrawablePtr pDrawable;
> +    DRI2DrawablePtr pPriv;
>      DRI2BufferPtr *buffers;
>      int status, width, height, count;
>      unsigned int *attachments;
>  
>      REQUEST_FIXED_SIZE(xDRI2GetBuffersReq, stuff->count * (2 * 4));
> -    if (!validDrawable(client, stuff->drawable, DixReadAccess | DixWriteAccess,
> -		       &pDrawable, &status))
> +    if (!validDRI2Drawable(client, stuff->drawable, DixReadAccess | DixWriteAccess,
> +		       &pPriv, &status))
>  	return status;
>  
> -    if (DRI2ThrottleClient(client, pDrawable))
> +    if (DRI2ThrottleClient(client, pPriv))
>  	return Success;
>  
>      attachments = (unsigned int *) &stuff[1];
> -    buffers = DRI2GetBuffersWithFormat(pDrawable, &width, &height,
> +    buffers = DRI2GetBuffersWithFormat(pPriv, &width, &height,
>  				       attachments, stuff->count, &count);
>  
> -    return send_buffers_reply(client, pDrawable, buffers, count, width, height);
> +    return send_buffers_reply(client, pPriv, buffers, count, width, height);
>  }
>  
>  static int
> @@ -310,19 +338,19 @@ ProcDRI2CopyRegion(ClientPtr client)
>  {
>      REQUEST(xDRI2CopyRegionReq);
>      xDRI2CopyRegionReply rep;
> -    DrawablePtr pDrawable;
> +    DRI2DrawablePtr pPriv;
>      int status;
>      RegionPtr pRegion;
>  
>      REQUEST_SIZE_MATCH(xDRI2CopyRegionReq);
>  
> -    if (!validDrawable(client, stuff->drawable, DixWriteAccess,
> -		       &pDrawable, &status))
> +    if (!validDRI2Drawable(client, stuff->drawable, DixWriteAccess,
> +		       &pPriv, &status))
>  	return status;
>  
>      VERIFY_REGION(pRegion, stuff->region, client, DixReadAccess);
>  
> -    status = DRI2CopyRegion(pDrawable, pRegion, stuff->dest, stuff->src);
> +    status = DRI2CopyRegion(pPriv, pRegion, stuff->dest, stuff->src);
>      if (status != Success)
>  	return status;
>  
> @@ -380,29 +408,29 @@ ProcDRI2SwapBuffers(ClientPtr client)
>  {
>      REQUEST(xDRI2SwapBuffersReq);
>      xDRI2SwapBuffersReply rep;
> -    DrawablePtr pDrawable;
> +    DRI2DrawablePtr pPriv;
>      CARD64 target_msc, divisor, remainder, swap_target;
>      int status;
>  
>      REQUEST_SIZE_MATCH(xDRI2SwapBuffersReq);
>  
> -    if (!validDrawable(client, stuff->drawable,
> -		       DixReadAccess | DixWriteAccess, &pDrawable, &status))
> +    if (!validDRI2Drawable(client, stuff->drawable,
> +		       DixReadAccess | DixWriteAccess, &pPriv, &status))
>  	return status;
>  
>      /*
>       * Ensures an out of control client can't exhaust our swap queue, and
>       * also orders swaps.
>       */
> -    if (DRI2ThrottleClient(client, pDrawable))
> +    if (DRI2ThrottleClient(client, pPriv))
>  	return Success;
>  
>      target_msc = vals_to_card64(stuff->target_msc_lo, stuff->target_msc_hi);
>      divisor = vals_to_card64(stuff->divisor_lo, stuff->divisor_hi);
>      remainder = vals_to_card64(stuff->remainder_lo, stuff->remainder_hi);
>  
> -    status = DRI2SwapBuffers(client, pDrawable, target_msc, divisor, remainder,
> -			     &swap_target, DRI2SwapEvent, pDrawable);
> +    status = DRI2SwapBuffers(client, pPriv, target_msc, divisor, remainder,
> +			     &swap_target, DRI2SwapEvent, pPriv);
>      if (status != Success)
>  	return BadDrawable;
>  
> @@ -432,17 +460,17 @@ ProcDRI2GetMSC(ClientPtr client)
>  {
>      REQUEST(xDRI2GetMSCReq);
>      xDRI2MSCReply rep;
> -    DrawablePtr pDrawable;
> +    DRI2DrawablePtr pPriv;
>      CARD64 ust, msc, sbc;
>      int status;
>  
>      REQUEST_SIZE_MATCH(xDRI2GetMSCReq);
>  
> -    if (!validDrawable(client, stuff->drawable, DixReadAccess, &pDrawable,
> +    if (!validDRI2Drawable(client, stuff->drawable, DixReadAccess, &pPriv,
>  		       &status))
>  	return status;
>  
> -    status = DRI2GetMSC(pDrawable, &ust, &msc, &sbc);
> +    status = DRI2GetMSC(pPriv, &ust, &msc, &sbc);
>      if (status != Success)
>  	return status;
>  
> @@ -460,7 +488,7 @@ static int
>  ProcDRI2WaitMSC(ClientPtr client)
>  {
>      REQUEST(xDRI2WaitMSCReq);
> -    DrawablePtr pDrawable;
> +    DRI2DrawablePtr pPriv;
>      CARD64 target, divisor, remainder;
>      int status;
>  
> @@ -468,7 +496,7 @@ ProcDRI2WaitMSC(ClientPtr client)
>  
>      REQUEST_SIZE_MATCH(xDRI2WaitMSCReq);
>  
> -    if (!validDrawable(client, stuff->drawable, DixReadAccess, &pDrawable,
> +    if (!validDRI2Drawable(client, stuff->drawable, DixReadAccess, &pPriv,
>  		       &status))
>  	return status;
>  
> @@ -476,7 +504,7 @@ ProcDRI2WaitMSC(ClientPtr client)
>      divisor = vals_to_card64(stuff->divisor_lo, stuff->divisor_hi);
>      remainder = vals_to_card64(stuff->remainder_lo, stuff->remainder_hi);
>  
> -    status = DRI2WaitMSC(client, pDrawable, target, divisor, remainder);
> +    status = DRI2WaitMSC(client, pPriv, target, divisor, remainder);
>      if (status != Success)
>  	return status;
>  
> @@ -502,18 +530,18 @@ static int
>  ProcDRI2SwapInterval(ClientPtr client)
>  {
>      REQUEST(xDRI2SwapIntervalReq);
> -    DrawablePtr pDrawable;
> +    DRI2DrawablePtr pPriv;
>      int status;
>  
>      /* FIXME: in restart case, client may be gone at this point */
>  
>      REQUEST_SIZE_MATCH(xDRI2SwapIntervalReq);
>  
> -    if (!validDrawable(client, stuff->drawable, DixReadAccess | DixWriteAccess,
> -		       &pDrawable, &status))
> +    if (!validDRI2Drawable(client, stuff->drawable, DixReadAccess | DixWriteAccess,
> +		       &pPriv, &status))
>  	return status;
>  
> -    DRI2SwapInterval(pDrawable, stuff->interval);
> +    DRI2SwapInterval(pPriv, stuff->interval);
>  
>      return Success;
>  }
> @@ -522,18 +550,18 @@ static int
>  ProcDRI2WaitSBC(ClientPtr client)
>  {
>      REQUEST(xDRI2WaitSBCReq);
> -    DrawablePtr pDrawable;
> +    DRI2DrawablePtr pPriv;
>      CARD64 target;
>      int status;
>  
>      REQUEST_SIZE_MATCH(xDRI2WaitSBCReq);
>  
> -    if (!validDrawable(client, stuff->drawable, DixReadAccess, &pDrawable,
> +    if (!validDRI2Drawable(client, stuff->drawable, DixReadAccess, &pPriv,
>  		       &status))
>  	return status;
>  
>      target = vals_to_card64(stuff->target_sbc_lo, stuff->target_sbc_hi);
> -    status = DRI2WaitSBC(client, pDrawable, target);
> +    status = DRI2WaitSBC(client, pPriv, target);
>  
>      return status;
>  }

Other than the minor comment quibble,
Reviewed-By: Christopher James Halse Rogers
<christopher.halse.rogers at canonical.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110204/b9874290/attachment-0001.pgp>


More information about the xorg-devel mailing list