[Mesa-dev] [PATCH] st/dri: Use depth instead of bpp when communicating formats with the X server v2

[Thomas Hellstrom]

On 12/06/2011 06:51 PM, Brian Paul wrote:
> Hi Thomas,
>
> Just a couple nit-picky things below:
>

OK, sent out a v3.

/Thomas



> On 12/06/2011 10:32 AM, Thomas Hellstrom wrote:
>> Some hardware can't reinterpret the format of hardware buffers and thus
>> the X server needs to know the format when the buffer is created.
>>
>> Signed-off-by: Thomas Hellstrom<thellstrom at vmware.com>
>> ---
>>   src/gallium/state_trackers/dri/drm/dri2.c |   43 
>> +++++++++++++++++++++++++++--
>>   1 files changed, 40 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/dri/drm/dri2.c 
>> b/src/gallium/state_trackers/dri/drm/dri2.c
>> index 4e3f106..850ed34 100644
>> --- a/src/gallium/state_trackers/dri/drm/dri2.c
>> +++ b/src/gallium/state_trackers/dri/drm/dri2.c
>> @@ -104,7 +104,7 @@ dri2_drawable_get_buffers(struct dri_drawable 
>> *drawable,
>>      for (i = 0; i<  *count; i++) {
>>         enum pipe_format format;
>>         unsigned bind;
>> -      int att, bpp;
>> +      int att, depth;
>>
>>         dri_drawable_get_format(drawable, statts[i],&format,&bind);
>>         if (format == PIPE_FORMAT_NONE)
>> @@ -134,12 +134,49 @@ dri2_drawable_get_buffers(struct dri_drawable 
>> *drawable,
>>            break;
>>         }
>>
>> -      bpp = util_format_get_blocksizebits(format);
>> +      /*
>> +       * In this switch statement we must support all formats that
>> +       * may occur as the stvis->color_format or stvis->
>> +       *
>> +       */
>> +
>> +
>
> I don't think two blank lines are needed here, if any.  There's also a 
> blank line in the comment itself.
>
>> +      switch(format) {
>> +      case PIPE_FORMAT_B8G8R8A8_UNORM:
>> +     depth = 32;
>> +     break;
>> +      case PIPE_FORMAT_B8G8R8X8_UNORM:
>> +     depth = 24;
>> +     break;
>> +      case PIPE_FORMAT_B5G6R5_UNORM:
>> +     depth = 16;
>> +     break;
>> +      case PIPE_FORMAT_Z16_UNORM:
>> +     att = __DRI_BUFFER_DEPTH;
>> +     depth = 16;
>> +     break;
>> +      case PIPE_FORMAT_Z24X8_UNORM:
>> +      case PIPE_FORMAT_X8Z24_UNORM:
>> +     att = __DRI_BUFFER_DEPTH;
>> +     depth = 24;
>> +     break;
>> +      case PIPE_FORMAT_Z24_UNORM_S8_UINT:
>> +      case PIPE_FORMAT_S8_UINT_Z24_UNORM:
>> +     depth = 32;
>> +     break;
>> +      case PIPE_FORMAT_Z32_UNORM:
>> +     att = __DRI_BUFFER_DEPTH;
>> +     depth = 32;
>> +     break;
>> +      default:
>> +     depth = util_format_get_blocksizebits(format);
>> +     assert(0);
>> +      }
>
> An assertion like
>
> assert(!"unexpected format in dri2_drawable_get_buffers()");
>
> would be more helpful if it ever failed for someone.
>
>
>>
>>         if (att>= 0) {
>>            attachments[num_attachments++] = att;
>>            if (with_format) {
>> -            attachments[num_attachments++] = bpp;
>> +            attachments[num_attachments++] = depth;
>>            }
>>         }
>>      }
>
> -Brian