On Mon, Mar 28, 2016 at 4:40 PM, Ilia Mirkin <[email protected]> wrote: > On Mon, Mar 28, 2016 at 9:25 AM, Oded Gabbay <[email protected]> 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 <[email protected]> >> --- >> 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 >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/piglit _______________________________________________ Piglit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/piglit
