[Piglit] [PATCH v2 1/4] pbo-read-argb8888: make it endian-safe

[Oded Gabbay]

On Wed, Mar 30, 2016 at 4:49 AM, Michel Dänzer <michel at daenzer.net> wrote:
> On 29.03.2016 18:06, Oded Gabbay wrote:
>> In this test we use GL_BGRA + GL_UNSIGNED_BYTE. However, the probe
>> function receives two 4-byte values to compare, expected and
>> observed. This is wrong as the correct way to compare
>> array_of_bytes (GL_UNSIGNED_BYTE) in an endian-safe way is by comparing
>> memory (and not values).
>>
>> This patch fixes this bug by changing the function to receive two
>> pointers instead of values. It also corrects the way the expected values
>> are constructed to be in endian-safe way for array-of-bytes
>>
>> This fixes the test in llvmpipe, softpipe and r600g in big-endian machine.
>>
>> v2: Changed initialization of expected results to be more clear
>>
>> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
>> ---
>>  tests/general/pbo-read-argb8888.c | 18 ++++++++++++------
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/general/pbo-read-argb8888.c b/tests/general/pbo-read-argb8888.c
>> index 99b4b3b..eaaa6ab 100644
>> --- a/tests/general/pbo-read-argb8888.c
>> +++ b/tests/general/pbo-read-argb8888.c
>> @@ -43,12 +43,16 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>>  PIGLIT_GL_TEST_CONFIG_END
>>
>>  static GLboolean
>> -probe(int x, int y, uint32_t expected, uint32_t observed)
>> +probe(int x, int y, uint8_t *expected, uint32_t *observed)
>>  {
>> -     if ((expected & 0xffffff) != (observed & 0xffffff)) {
>> +     uint8_t *p_obs = (uint8_t *) observed;
>> +
>> +     if (expected[0] != p_obs[0] ||
>> +              expected[1] != p_obs[1] ||
>> +              expected[2] != p_obs[2]) {
>>               printf("Probe color at (%i,%i)\n", x, y);
>> -             printf("  Expected: 0x%08x\n", expected);
>> -             printf("  Observed: 0x%08x\n", observed);
>> +             printf("  Expected: 0x%08x\n", *((uint32_t *) expected));
>> +             printf("  Observed: 0x%08x\n", *observed);
>
> For clarity and cleaner code, I'd change the observed pointer to uint8_t
> as well, and not print the expected and observed values as packed 32-bit
> values. E.g.
>
>         printf("  Expected: b=0x%02x g=0x%02x r=0x%02x a=0x%02x\n",
>                expected[0], expected[1], expected[2], expected[3]);
>
> and correspondingly for observed.
>
>
>> @@ -63,6 +67,8 @@ piglit_display(void)
>>       static float red[]   = {1.0, 0.0, 0.0, 0.0};
>>       static float green[] = {0.0, 1.0, 0.0, 0.0};
>>       uint32_t *addr;
>
> This would be uint8_t* as well then.
>
>
>> @@ -84,8 +90,8 @@ piglit_display(void)
>>
>>       addr = glMapBufferARB(GL_PIXEL_PACK_BUFFER, GL_READ_ONLY_ARB);
>>
>> -     pass &= probe(10, 10, 0x0000ff00, addr[0]);
>> -     pass &= probe(10, 10, 0x00ff0000, addr[1]);
>> +     pass &= probe(10, 10, exp_green, &addr[0]);
>> +     pass &= probe(10, 10, exp_red, &addr[1]);
>
> And this would be
>
>         pass &= probe(10, 10, exp_green, addr);
>         pass &= probe(10, 10, exp_red, addr + 4);
>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer

Yes, that seems more correct to display in bytes.
I'll change that in all the patches.

Oded