[Piglit] [PATCH] teximage-colors: accept -127 instead of -128 for exact snorm up/download

[Roland Scheidegger]

Am 11.01.2018 um 23:08 schrieb Brian Paul:
> On 01/11/2018 02:45 PM, Roland Scheidegger wrote:
>> ping?
>>
>> Am 08.01.2018 um 01:20 schrieb sroland at vmware.com:
>>> From: Roland Scheidegger <sroland at vmware.com>
>>>
>>> -128 and -127 represent exactly the same value (-1.0) for snorm8 values,
>>> as do -32768/-32767 for snorm16. Therefore it seems quite reasonable
>>> that an
>>> implementation would return the other representation (in particular
>>> r600 will
>>> do this with a blit, and the snorm->float->snorm conversion implied
>>> by this
>>> will never return the -128/-32768 values).
>>> ---
>>>   tests/texturing/teximage-colors.c | 39
>>> +++++++++++++++++++++++++++++++++++----
>>>   1 file changed, 35 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/texturing/teximage-colors.c
>>> b/tests/texturing/teximage-colors.c
>>> index de2024644..61a3c5d15 100644
>>> --- a/tests/texturing/teximage-colors.c
>>> +++ b/tests/texturing/teximage-colors.c
>>> @@ -833,10 +833,41 @@ test_exact()
>>>                 observed);
>>>       pass &= piglit_check_gl_error(GL_NO_ERROR);
>>>   -    for (i = 0; i < texture_size; ++i)
>>> -        pass &= memcmp(&data[i * texture_size * Bpp],
>>> -                   &observed[i * tex_width * Bpp],
>>> -                   texture_size * Bpp) == 0;
>>> +    /*
>>> +     * For snorm formats, -127/-128 and -32767/-32768 represent the
>>> exact
>>> +     * same value (-1.0). Therefore, it is quite reasonable to expect
>>> +     * an implementation could return the other representation.
>>> +     * (We'll assume it will happen only one way the other way seems
>>> rather
>>> +     * unlikely.)
>>> +     */
>>> +    if (format->data_type == GL_BYTE) {
>>> +        int j;
>>> +        for (j = 0; j < texture_size; ++j) {
>>> +            for (i = 0; i < tex_width * channels; i++) {
>>> +                if (!(data[i] == observed[i] ||
>>> +                      (data[i] == 128 && observed[i] == 129))) {
> 
> This might be more obvious if you you'd cast the data and observed
> pointers to GLbyte types (like you do for GLshort below) and then test
> for the -127 and -128 values directly.
Ah yes, I was lazy with the typing there (I didn't actually fix the
GLshort case initially, because I needed to try like 20 seed values to
finally hit a failure)...
> In any case, Reviewed-by: Brian Paul <brianp at vmware.com>

Thanks!

Roland

> 
> 
>>> +                    pass = GL_FALSE;
>>> +                }
>>> +            }
>>> +        }
>>> +    } else if (format->data_type == GL_SHORT) {
>>> +        int j;
>>> +        for (j = 0; j < texture_size; ++j) {
>>> +            for (i = 0; i < tex_width * channels; i++) {
>>> +                GLshort datas = ((GLshort *)data)[i];
>>> +                GLshort obss = ((GLshort *)observed)[i];
>>> +                if (!(datas == obss ||
>>> +                      (datas == -32768 && obss == -32767))) {
>>> +                    pass = GL_FALSE;
>>> +                }
>>> +            }
>>> +        }
>>> +    } else {
>>> +        for (i = 0; i < texture_size; ++i)
>>> +            pass &= memcmp(&data[i * texture_size * Bpp],
>>> +                &observed[i * tex_width * Bpp],
>>> +                texture_size * Bpp) == 0; >> +    }
>>>         free(observed);
>>>       free(tmp_float);
>>>
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit
>>
>