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

From: Martin Peres <martin.peres@free.fr>
To: Ian Romanick <idr@freedesktop.org>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	xorg-devel@lists.x.org, dri-devel@lists.freedesktop.org,
	mesa-dev@lists.freedesktop.org
Subject: Re: [xorg 1/3] dri2: Allow GetBuffers to match any format
Date: Tue, 16 Jun 2015 16:11:44 +0300	[thread overview]
Message-ID: <55802090.3090704@free.fr> (raw)
In-Reply-To: <54BEBF65.2080408@freedesktop.org>

On 20/01/15 22:49, Ian Romanick wrote:
> 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?

This is still a problem in the current patchset that I have. Since the 
client did not specifically ask for a certain format, why not increase 
the likeliness of us being able to reuse the buffer later on by using a 
format that the application already asked before?

>
> 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

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev