[Mesa-dev] [PATCH] i965: correct alignment units for 2D compressed textures on Skylake

[Neil Roberts]

Nanley Chery <nanleychery at gmail.com> writes:

> Although most of the patch is incorrect, the following change is still
> necessary isn't it?
>         if (mt->compressed) {
>            mip1_width = ALIGN(minify(mt->physical_width0, 1), mt->align_w) +
> -             ALIGN(minify(mt->physical_width0, 2), bw);
> +             ALIGN(minify(mt->physical_width0, 2), mt->align_w);
>
> Each LOD is aligned by the alignment term. I don't see why we're
> switching to block width here.

I'm pretty sure the original version is correct and we do want to align
to the block width here. This bit of code is not being used to work out
the position of an image so it doesn't need to take into account the
image alignment. It is only being used to calculate the mt->total_width
value. That value is not aligned to the image alignment. You can see
this because it will usually just be directly taken from
mt->physical_width0 and that is not aligned. In the non-compressed case
just below you can see that the second half of the addition is not
aligned to anything at all. The total_width does however need to be at
least a multiple of the block size because it isn't possible to allocate
space for half of a block.

If I understand correctly this code is just trying to cope with cases
where the third image in the mipmap is positioned so that it extends
past the width of the first image. If that happens then the width of the
image containing all of the mipmap images needs to be extended slightly
or it would crop the third mipmap image.

Please see the attached SVG.

Looking at it a bit more I think this bit above is wrong:

   if (mt->compressed) {
       mt->total_width = ALIGN(mt->physical_width0, mt->align_w);
   }

That should also be using bw instead of mt->align_w for the same reason.
I think it could only end up making the total_width a bit larger than
necessary so it probably isn't causing any actual problems.

I'll make a patch and test it.

Regards,
- Neil

-------------- next part --------------
A non-text attachment was scrubbed...
Name: miptree-extend.svg
Type: image/svg+xml
Size: 13923 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150615/25906993/attachment-0001.svg>