[Mesa-dev] [PATCH 02/11] loader/dri3: Make sure we always have a context for image blit operations

[Thomas Hellstrom]

Hi, Michel, Thanks for reviewing.

On 08/15/2017 08:57 AM, Michel Dänzer wrote:
> On 11/08/17 11:14 PM, Thomas Hellstrom wrote:
>> The code was relying on us always having a current context for client local
>> image blit operations. Otherwise the blit would be skipped. However,
>> glxSwapBuffers, for example, doesn't require a current context and that was a
>> common problem in the dri1 era. It seems the problem has resurfaced with dri3.
>>
>> If we don't have a current context when we want to blit, try creating a private
>> dri context and maintain a context cache of a single context.
>>
>> Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
> [...]
>
>> +/**
>> + * A cached blit context.
>> + */
>> +struct loader_dri3_blit_context {
>> +   mtx_t mtx;
>> +   __DRIcontext *ctx;
>> +   __DRIscreen *cur_screen;
> The cur_screen field seems redundant with __DRIcontextRec::driScreenPriv.

Problem is __DRIcontextRec is opaque here, so we can't access its members.

>
>
>> @@ -149,6 +254,9 @@ loader_dri3_drawable_init(xcb_connection_t *conn,
>>      draw->have_fake_front = 0;
>>      draw->first_init = true;
>>   
>> +   draw->have_image_blit = draw->ext->image->base.version >= 9 &&
>> +      draw->ext->image->blitImage != NULL;
> Is it really worth having a dedicated drawable field for this? Seems
> like open-coding this in loader_dri3_blit_image should be fine.

Sure, will do.

>
>
>> @@ -1340,7 +1426,7 @@ loader_dri3_get_buffers(__DRIdrawable *driDrawable,
>>         return false;
>>   
>>      /* pixmaps always have front buffers */
>> -   if (draw->is_pixmap)
>> +//   if (draw->is_pixmap)
>>         buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;

Hmm, this shouldn't be here. It's a leftover debug thing. I'll fix it up 
to the next version.

> Either leave the line uncommented, or remove it and fix up the
> indentation of the following line.
>
>
Thanks,

Thomas