[Mesa-dev] [PATCH] softpipe: fix clamping when using unormalized coordinates

[Brian Paul]

On 11/14/2011 03:03 PM, Morgan Armand wrote:
> On 11/14/2011 6:40 PM, Brian Paul wrote:
>> On 11/14/2011 10:24 AM, Morgan Armand wrote:
>>> On 11/14/2011 3:44 PM, Brian Paul wrote:
>>>> On 11/13/2011 03:24 AM, Morgan Armand wrote:
>>>>> ---
>>>>>     src/gallium/drivers/softpipe/sp_tex_sample.c |    7 ++++---
>>>>>     1 files changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/src/gallium/drivers/softpipe/sp_tex_sample.c b/src/gallium/drivers/softpipe/sp_tex_sample.c
>>>>> index 72629a0..9b0e54e1 100644
>>>>> --- a/src/gallium/drivers/softpipe/sp_tex_sample.c
>>>>> +++ b/src/gallium/drivers/softpipe/sp_tex_sample.c
>>>>> @@ -491,7 +491,8 @@ wrap_linear_unorm_clamp(const float s[4], unsigned size,
>>>>>        uint ch;
>>>>>        for (ch = 0; ch<    4; ch++) {
>>>>>           /* Not exactly what the spec says, but it matches NVIDIA output */
>>>>> -      float u = CLAMP(s[ch] - 0.5F, 0.0f, (float) size - 1.0f);
>>>>> +      float u = CLAMP(s[ch], 0.0f, (float) size);
>>>>> +      u -= 0.5F;
>>>>>           icoord0[ch] = util_ifloor(u);
>>>>
>>>> If s=0, then icoord0 = -1 and that's not right.  The 'i' coordinates must be in the range [0,size-1].
>>>>
>>>> Are you trying to fix a specific bug or piglit test?
>>>>
>>>>
>>>>>           icoord1[ch] = icoord0[ch] + 1;
>>>>>           w[ch] = frac(u);
>>>>> @@ -512,8 +513,8 @@ wrap_linear_unorm_clamp_to_border(const float s[4], unsigned size,
>>>>>           u -= 0.5F;
>>>>>           icoord0[ch] = util_ifloor(u);
>>>>>           icoord1[ch] = icoord0[ch] + 1;
>>>>> -      if (icoord1[ch]>    (int) size - 1)
>>>>> -         icoord1[ch] = size - 1;
>>>>> +      if (icoord1[ch]>    (int) size)
>>>>> +         icoord1[ch] = size;
>>>>>           w[ch] = frac(u);
>>>>>        }
>>>>>     }
>>>>
>>>> -Brian
>>>
>>> Yes, sorry, I forgot to mention it. This patch fixes texwrap-RECT-bordercolor and texwrap-RECT-proj-bordercolor.
>>>
>>>   From what I understand from the spec, we expect to get the border color when sampling with out-of-range coordinates, or the
>>> correct interpolation between the border color and the texel color when sampling with coordinates in the range [0; 1/2N[ or
>>> [1-1/2N; max[. That's why I converted the coordinates to [-1;size] instead. get_texel_*d functions handle the case when a texture
>>> coordinate is out-of-range but it may not the case of all functions so I probably need to check that carefully.
>>> Please correct me if I misunderstood something.
>>
>> I think I was wrong above.  I thought you were changing the clamp-to-edge behaviour, but that case is implemented in the wrap_linear_unorm_clamp_to_edge() function.
>>
>> In any case, I remember that implementing what the spec says didn't match the output from NVIDIA's driver (hence the comment there).
>>
>> Do you have an NVIDIA GPU to compare against?
>>
>> I could do some testing/comparing later...
>>
>> -Brian
>
> Yes I have, and AFAICT the results are identical (pixel-perfect, in fact). I've made the test on a windows machine, with the last
> drivers.

OK, sounds good.

Sorry for the initial confusion.

Reviewed-by: Brian Paul <brianp at vmware.com>