Re: [Mesa3d-dev] Eliminating pipe_texture->cpp and pipe_surface->cpp in gallium

[J. F. <jrf...@tu...>]

On Fri, Jun 27, 2008 at 9:37 PM, Jakob Bornecrantz <wal...@gm...> wrote:
> On Fri, Jun 27, 2008 at 12:57 PM, José Fonseca
> <jrf...@tu...> wrote:
>> In order to properly represent the compressed textures and (yuv formats
>> to some extent) it is necessary to abandon the chars-per-pixels concept
>> in gallium, and use instead the concept of pixel blocks (in p_format.h):
>>
>> /**
>>  * Describe accurately the pixel format.
>>  *
>>  * The chars-per-pixel concept falls apart with compressed and yuv images, where
>>  * more than one pixel are coded in a single data block. This structure
>>  * describes that block.
>>  *
>>  * Simple pixel formats are effectively a 1x1xcpp block.
>>  */
>> struct pipe_format_block
>> {
>>   /** Block size in bytes */
>>   unsigned size;
>>
>>   /** Block width in pixels */
>>   unsigned width;
>>
>>   /** Block height in pixels */
>>   unsigned height;
>> };
>>
>> /**
>>  * Describe pixel format's block.
>>  *
>>  * @sa http://msdn2.microsoft.com/en-us/library/ms796147.aspx
>>  */
>> static INLINE void
>> pf_get_block(enum pipe_format format, struct pipe_format_block *block)
>> {
>>   switch(format) {
>>   case PIPE_FORMAT_DXT1_RGBA:
>>   case PIPE_FORMAT_DXT1_RGB:
>>      block->size = 8;
>>      block->width = 4;
>>      block->height = 4;
>>      break;
>>   case PIPE_FORMAT_DXT3_RGBA:
>>   case PIPE_FORMAT_DXT5_RGBA:
>>      block->size = 16;
>>      block->width = 4;
>>      block->height = 4;
>>      break;
>>   case PIPE_FORMAT_YCBCR:
>>   case PIPE_FORMAT_YCBCR_REV:
>>      block->size = 4; /* 2*cpp */
>>      block->width = 2;
>>      block->height = 1;
>>      break;
>>   default:
>>      block->size = pf_get_size(format);
>>      block->width = 1;
>>      block->height = 1;
>>      break;
>>   }
>> }
>>
>> static INLINE unsigned
>> pf_get_nblocksx(const struct pipe_format_block *block, unsigned x)
>> {
>>   return (x + block->width - 1)/block->width;
>> }
>>
>> static INLINE unsigned
>> pf_get_nblocksy(const struct pipe_format_block *block, unsigned y)
>> {
>>   return (y + block->height - 1)/block->height;
>> }
>>
>>
>> Pipe texture will become:
>>
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -303,7 +303,10 @@ struct pipe_texture
>>    unsigned height[PIPE_MAX_TEXTURE_LEVELS];
>>    unsigned depth[PIPE_MAX_TEXTURE_LEVELS];
>>
>> -   unsigned cpp:8;
>> +   struct pipe_format_block block;
>> +   unsigned nblocksx[PIPE_MAX_TEXTURE_LEVELS];
>> +   unsigned nblocksy[PIPE_MAX_TEXTURE_LEVELS];
>> +
>>    unsigned last_level:8;    /**< Index of last mipmap level present/defined */
>>    unsigned compressed:1;
>>
>>
>> Also, with chars per pixel gone, pitch is better be specified in bytes,
>> rather than pixels. To make sure that nothing gets forgotten I'm going
>> to rename all pipe_texture and pipe_surface "pitch" members to "stride",
>> so that if somewhere is forgotten one gets a compiler error:
>>
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -267,10 +267,12 @@ struct pipe_surface
>>    enum pipe_format format;      /**< PIPE_FORMAT_x */
>>    unsigned status;              /**< PIPE_SURFACE_STATUS_x */
>>    unsigned clear_value;         /**< XXX may be temporary */
>> -   unsigned cpp;                 /**< bytes per pixel */
>>    unsigned width;
>>    unsigned height;
>> -   unsigned pitch;               /**< in pixels */
>> +   struct pipe_format_block block;
>> +   unsigned nblocksx;
>> +   unsigned nblocksy;
>> +   unsigned stride;              /**< in bytes */
>>    unsigned layout;              /**< PIPE_SURFACE_LAYOUT_x */
>>    unsigned offset;              /**< offset from start of buffer, in bytes */
>>    unsigned refcount;
>
> Looks good.
>
> I have fixed the problems in the i915simple driver and dri/intel
> winsys that where remaining. So that works now.

Thanks Jakob.

Jose