[Mesa-dev] [PATCH 03/11] intel: Add block alignment for RGTC textures.

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/10/2011 03:44 PM, Eric Anholt wrote:
> On Fri, 10 Jun 2011 15:17:12 -0600, Brian Paul <brianp at vmware.com> wrote:
>> On 06/10/2011 02:38 PM, Eric Anholt wrote:
>>> We were using the default 4x2 alignment instead of the 4x4 required
>>> for non-FXT compressed textures.
>>> ---
>>>   src/mesa/drivers/dri/i965/brw_tex_layout.c     |    2 +-
>>>   src/mesa/drivers/dri/intel/intel_mipmap_tree.c |    6 +--
>>>   src/mesa/drivers/dri/intel/intel_tex_layout.c  |   42 +++++++++---------------
>>>   src/mesa/drivers/dri/intel/intel_tex_layout.h  |    3 +-
>>>   4 files changed, 21 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>> index 4a3a2bf..f462f32 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
>>> @@ -88,7 +88,7 @@ GLboolean brw_miptree_layout(struct intel_context *intel,
>>>         GLuint align_w = 4;
>>>
>>>         mt->total_height = 0;
>>> -      intel_get_texture_alignment_unit(mt->internal_format,&align_w,&align_h);
>>> +      intel_get_texture_alignment_unit(mt->format,&align_w,&align_h);
>>>
>>>         if (mt->compressed) {
>>>             mt->total_width = ALIGN(width, align_w);
>>> diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>>> index aa6fc5b..9a1fb18 100644
>>> --- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>>> +++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
>>> @@ -402,8 +402,7 @@ intel_miptree_image_data(struct intel_context *intel,
>>>         if(dst->compressed) {
>>>   	 unsigned int align_w, align_h;
>>>
>>> -	 intel_get_texture_alignment_unit(dst->internal_format,
>>> -					&align_w,&align_h);
>>> +	 intel_get_texture_alignment_unit(dst->format,&align_w,&align_h);
>>>   	 height = (height + align_h - 1) / align_h;
>>>   	 width = ALIGN(width, align_w);
>>>         }
>>> @@ -445,8 +444,7 @@ intel_miptree_image_copy(struct intel_context *intel,
>>>      if (dst->compressed) {
>>>          GLuint align_w, align_h;
>>>
>>> -       intel_get_texture_alignment_unit(dst->internal_format,
>>> -&align_w,&align_h);
>>> +       intel_get_texture_alignment_unit(dst->format,&align_w,&align_h);
>>>          height = (height + 3) / 4;
>>>          width = ALIGN(width, align_w);
>>>      }
>>> diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.c b/src/mesa/drivers/dri/intel/intel_tex_layout.c
>>> index d39733b..7493b42 100644
>>> --- a/src/mesa/drivers/dri/intel/intel_tex_layout.c
>>> +++ b/src/mesa/drivers/dri/intel/intel_tex_layout.c
>>> @@ -35,32 +35,22 @@
>>>   #include "intel_context.h"
>>>   #include "main/macros.h"
>>>
>>> -void intel_get_texture_alignment_unit(GLenum internalFormat, GLuint *w, GLuint *h)
>>> +void
>>> +intel_get_texture_alignment_unit(gl_format format,
>>> +				 unsigned int *w, unsigned int *h)
>>>   {
>>> -    switch (internalFormat) {
>>> -    case GL_COMPRESSED_RGB_FXT1_3DFX:
>>> -    case GL_COMPRESSED_RGBA_FXT1_3DFX:
>>> -        *w = 8;
>>> -        *h = 4;
>>> -        break;
>>> -
>>> -    case GL_RGB_S3TC:
>>> -    case GL_RGB4_S3TC:
>>> -    case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
>>> -    case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
>>> -    case GL_RGBA_S3TC:
>>> -    case GL_RGBA4_S3TC:
>>> -    case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
>>> -    case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
>>> -        *w = 4;
>>> -        *h = 4;
>>> -        break;
>>> -
>>> -    default:
>>> -        *w = 4;
>>> -        *h = 2;
>>> -        break;
>>> -    }
>>> +   if (_mesa_is_format_compressed(format)) {
>>> +      if (format == MESA_FORMAT_RGB_FXT1 || format == MESA_FORMAT_RGBA_FXT1) {
>>> +	 *w = 8;
>>> +	 *h = 4;
>>> +      } else {
>>> +	 *w = 4;
>>> +	 *h = 4;
>>> +      }
>>> +   } else {
>>> +      *w = 4;
>>> +      *h = 2;
>>> +   }
>>
>> Could you make use of _mesa_get_format_block_size() here?
> 
> The compressed formats happen to match, but the default case doesn't.
> Given that this is coming out of a table in the hardware spec (and we're
> going to need a bunch of overrides for it in the future with MSAA and
> things), I'd rather just reproduce the hw spec here.

But it should match for all the compressed formats, right?  I think
Brian was suggesting to use _mesa_get_format_block_size just for the
compressed formats.  Some day there will be more compressed formats. *shrug*
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk3yvNsACgkQX1gOwKyEAw80AgCffQYULVx7ap+ZzG5XC+j8xtYi
uDIAoJjWQMw/E6f2ORHHPlwTnFsm6BT1
=5Uv6
-----END PGP SIGNATURE-----