Re: [Mesa-dev] [xorg 1/3] dri2: Allow GetBuffers to match any format

From: Ian Romanick <idr@freedesktop.org>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	xorg-devel@lists.x.org, dri-devel@lists.freedesktop.org,
	mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [xorg 1/3] dri2: Allow GetBuffers to match any format
Date: Tue, 20 Jan 2015 12:49:41 -0800	[thread overview]
Message-ID: <54BEBF65.2080408@freedesktop.org> (raw)
In-Reply-To: <1421665245-5994-3-git-send-email-chris@chris-wilson.co.uk>

On 01/19/2015 03:00 AM, Chris Wilson wrote:
> Since the introduction of DRI2GetBuffersWithFormat, the old
> DRI2GetBuffers interface would always recreate all buffers all the time
> as it was no longer agnostic to the format value being set by the DDXes.
> This causes an issue with clients intermixing the two requests,
> rendering any sharing or caching of buffers (e.g. for triple buffering)
> void.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  hw/xfree86/dri2/dri2.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index 43a1899..f9f594d 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -464,14 +464,16 @@ find_attachment(DRI2DrawablePtr pPriv, unsigned attachment)
>  static Bool
>  allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds,
>                           DRI2DrawablePtr pPriv,
> -                         unsigned int attachment, unsigned int format,
> +                         unsigned int attachment,
> +                         int has_format, unsigned int format,
>                           int dimensions_match, DRI2BufferPtr * buffer)
>  {
>      int old_buf = find_attachment(pPriv, attachment);
>  
>      if ((old_buf < 0)
>          || attachment == DRI2BufferFrontLeft
> -        || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) {
> +        || !dimensions_match
> +        || (has_format && pPriv->buffers[old_buf]->format != format)) {
>          *buffer = create_buffer(ds, pDraw, attachment, format);

Shouldn't the create_buffer change if !has_format?  If !has_format and,
say, !dimensions_match, create_buffer will get format = 0 when it should
get format = pPriv->buffers[old_buf]->format.  Right?

Another alternative would be to have the caller always pass a format:
either the format supplied in the protocol or the format of the old
buffer.  That might be more messy.  Dunno.

>          return TRUE;
>  
> @@ -549,7 +551,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>          const unsigned format = (has_format) ? *(attachments++) : 0;
>  
>          if (allocate_or_reuse_buffer(pDraw, ds, pPriv, attachment,
> -                                     format, dimensions_match, &buffers[i]))
> +                                     has_format, format, dimensions_match,
> +                                     &buffers[i]))
>              buffers_changed = 1;
>  
>          if (buffers[i] == NULL)
> @@ -584,7 +587,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>  
>      if (need_real_front > 0) {
>          if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFrontLeft,
> -                                     front_format, dimensions_match,
> +                                     has_format, front_format, dimensions_match,
>                                       &buffers[i]))
>              buffers_changed = 1;
>  
> @@ -595,7 +598,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>  
>      if (need_fake_front > 0) {
>          if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFakeFrontLeft,
> -                                     front_format, dimensions_match,
> +                                     has_format, front_format, dimensions_match,
>                                       &buffers[i]))
>              buffers_changed = 1;
>  
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel