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

[Oded Gabbay]

On Mon, Mar 28, 2016 at 4:40 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Mon, Mar 28, 2016 at 9:25 AM, Oded Gabbay <oded.gabbay at gmail.com> 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.
>>
>> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
>> ---
>>  tests/general/pbo-read-argb8888.c | 24 ++++++++++++++++++------
>>  1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/general/pbo-read-argb8888.c b/tests/general/pbo-read-argb8888.c
>> index 99b4b3b..e053d0d 100644
>> --- a/tests/general/pbo-read-argb8888.c
>> +++ b/tests/general/pbo-read-argb8888.c
>> @@ -43,12 +43,17 @@ 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, uint32_t *expected, uint32_t *observed)
>>  {
>> -       if ((expected & 0xffffff) != (observed & 0xffffff)) {
>> +       uint8_t *p_exp = (uint8_t *) expected;
>> +       uint8_t *p_obs = (uint8_t *) observed;
>> +
>> +       if (p_exp[0] != p_obs[0] ||
>> +                p_exp[1] != p_obs[1] ||
>> +                p_exp[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", *expected);
>> +               printf("  Observed: 0x%08x\n", *observed);
>>
>>                 return GL_FALSE;
>>         } else {
>> @@ -63,8 +68,15 @@ 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;
>> +       GLuint exp_green = 0, exp_red = 0;
>> +       GLbyte *ptr;
>>         GLuint pbo;
>>
>> +       ptr = (GLbyte *) &exp_green;
>> +       ptr[1] = 0xFF;
>> +       ptr = (GLbyte *) &exp_red;
>> +       ptr[2] = 0xFF;
>
> Could you do something like
>
> GLbyte exp_green[4] = {0x00, 0xFF, 0x00, 0x00};
> ... exp_red ...
>
> And then change addr to be an unsigned char * and change probe() to
> take two of those instead? That seems like it would convey what's
> actually being tested more clearly...
>
Yes, that indeed seems more clear.
I'll resend the patches.

Thanks,

Oded

>> +
>>         glGenBuffersARB(1, &pbo);
>>         glBindBufferARB(GL_PIXEL_PACK_BUFFER, pbo);
>>         glBufferDataARB(GL_PIXEL_PACK_BUFFER, 2 * 4, NULL, GL_STREAM_DRAW_ARB);
>> @@ -84,8 +96,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]);
>>
>>         glUnmapBufferARB(GL_PIXEL_PACK_BUFFER);
>>
>> --
>> 2.5.5
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit