[Mesa-dev] [PATCH] mesa: fix maximum allowed proxy texture size condition

[Brian Paul]

On 02/09/2012 01:57 PM, Brian Paul wrote:
> On 02/09/2012 12:15 PM, Anuj Phogat wrote:
>> On Thu, Feb 9, 2012 at 11:02 AM, Ian Romanick <idr at freedesktop.org
>> <mailto:idr at freedesktop.org>> wrote:
>>
>> On 02/09/2012 08:14 AM, Brian Paul wrote:
>>
>> Anuj,
>>
>> The patch you committed (15986d2), the first version you
>> posted that tests:
>>
>> if (width < 2 * border || width > maxSize)
>> return GL_FALSE;
>>
>> causes my copy of conform (mustpass.c / proxy texture test) to
>> fail with
>> swrast/softpipe/llvmpipe. It passes w/ NVIDIA's driver.
>>
>> I thought you were going to commit the patch that does:
>>
>> if (width < 2 * border || width > 2 * border + maxSize)
>> return GL_FALSE;
>>
>> That allows conform to pass here.
>>
>>
>> Allow me to remove the egg from my face. I dug a bit deeper into
>> the test case that was previously failing and conform / mustpass.
>> What I said before
>> (http://lists.freedesktop.org/__archives/mesa-dev/2012-__February/018562.html
>>
>> <http://lists.freedesktop.org/archives/mesa-dev/2012-February/018562.html>)
>>
>> was wrong. You and Jose were correct. Sorry.
>
> No biggie.
>
>
>> Intel oglconform test case (max_values
>> negative.textureSize.textureCube) also passes with the patch i earlier
>> posted as per Brian's comments:
>> http://lists.freedesktop.org/archives/mesa-dev/2012-January/018410.html
>> <%20http://lists.freedesktop.org/archives/mesa-dev/2012-January/018410.html>
>>
>> I'll make the required changes as per this patch.
>
> Thanks, the test passes again. But I have a follow-up comment for the
> patch in question...

Actually, this isn't directly related to your patch- the issue is with 
texture borders and array textures.

For a 1D texture array, the border only applies to the width.  For a 
2D texture array the border applies to the width and height but not 
the depth.  We're not handling this correctly everywhere.  For 
example, in _mesa_init_teximage_fields() we compute the 'Height2' 
field like this:

    if (height == 1) { /* 1-D texture */
       img->Height2 = 1;
       img->HeightLog2 = 0;
    }
    else {
       img->Height2 = height - 2 * border; /* == 1 << img->HeightLog2; */
       img->HeightLog2 = _mesa_logbase2(img->Height2);
    }

The else clause does the wrong thing if the texture is a 1D array 
texture with a border.  Similarly for Depth2 and 2D array textures.

Granted, borders are seldom used, but it would be good to go over all 
the teximage code and fix these mistakes.

-Brian