On Wed, Mar 30, 2016 at 4:49 AM, Michel Dänzer <[email protected]> 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 <[email protected]> >> --- >> 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 _______________________________________________ Piglit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/piglit
