[Mesa-dev] [PATCH 1/2] egl-util: Adds probe_front_pixel_rgb function

[Robert Bragg]

On Thu, Sep 20, 2012 at 3:34 PM, Brian Paul <brianp at vmware.com> wrote:
> On 09/20/2012 07:21 AM, Robert Bragg wrote:
>>
>> This adds an egl_probe_front_pixel_rgb function that is analogous to
>> piglit_probe_pixel_rgba except it probes the front buffer instead of
>> probing the back buffer.
>> ---
>>   tests/egl/egl-util.c |   30 ++++++++++++++++++++++++++++++
>>   tests/egl/egl-util.h |    4 ++++
>>   2 files changed, 34 insertions(+), 0 deletions(-)
>>
>> diff --git a/tests/egl/egl-util.c b/tests/egl/egl-util.c
>> index 20cd699..e26add4 100644
>> --- a/tests/egl/egl-util.c
>> +++ b/tests/egl/egl-util.c
>> @@ -37,6 +37,36 @@ static int automatic;
>>
>>   int depth;
>>
>> +int
>> +egl_probe_front_pixel_rgb(struct egl_state *state,
>> +                         int x, int y, const float *expected)
>> +{
>> +       XImage *ximage = XGetImage(state->dpy, state->win,
>> +                                  x, state->height - y - 1, 1, 1,
>> AllPlanes, ZPixmap);
>> +       unsigned long pixel = XGetPixel(ximage, 0, 0);
>> +       uint8_t *probe = (uint8_t *)&pixel;
>> +       int pass = 1;
>> +
>> +       XDestroyImage(ximage);
>> +
>> +       /* NB: XGetPixel returns a normalized BGRA, byte per
>> +        * component, pixel format */
>
>
> Is that always true?  Don't you need to examine the window's XVisualInfo's
> red/green/blue_mask vales to determine the position of the components in the
> ulong?

Yeah *maybe* you need to check the visual mask values to confirm the
order I'm not 100% sure. I didn't quite fully decipher how much
XGetPixel does for you. Certainly it tries to avoid you needing to
handle the full myriad of XImage pixel formats and stepping through
the code I could at least see it was reordering the components
depending on ximage->byte_order, but maybe the component order isn't
normalized.

>
> In any case, 8-bit BGRA can probably be safely assumed for now.
>
>
>
>> +       if (probe[2] != expected[0]*255 ||
>> +           probe[1] != expected[1]*255 ||
>> +           probe[0] != expected[2]*255) {
>> +               pass = 0;
>> +       }
>
>
> The other pixel probing functions in piglit use "fabs(probe - expected) <
> piglit_tolerance" when comparing actual/expected values. Shouldn't you do
> that here too?

Yeah I did see that, and initially figured it'd be ok to assume that
for just clearing the framebuffer to red/green and reading that back
to compare at byte per component precision I could get away without
it.

Since this api could be useful for other things though, it could be
nice if piglit_set_tolerance_for_bits() affected it in the same way as
the piglit_probe_ functions.

thanks for the feedback, I'll send out an updated patch in a bit.

regards,
- Robert