Kenneth Graunke <[email protected]> writes:

> On Friday, August 29, 2014 02:41:15 PM Eric Anholt wrote:
>> ---
>>  tests/shaders/shader_runner.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
>> index 717be42..2c350cf 100644
>> --- a/tests/shaders/shader_runner.c
>> +++ b/tests/shaders/shader_runner.c
>> @@ -2221,6 +2221,19 @@ piglit_display(void)
>>                      if (!piglit_probe_pixel_rgb(x, y, &c[2])) {
>>                              pass = false;
>>                      }
>> +            } else if (sscanf(line, "probe rect rgba "
>> +                              "( %f , %f , %f , %f ) "
>> +                              "( %f , %f , %f , %f )",
>> +                              c + 0, c + 1, c + 2, c + 3,
>> +                              c + 4, c + 5, c + 6, c + 7) == 8) {
>> +                    x = c[0];
>> +                    y = c[1];
>> +                    w = c[2];
>> +                    h = c[3];
>> +
>> +                    if (!piglit_probe_rect_rgba(x, y, w, h, &c[4])) {
>> +                            pass = false;
>> +                    }
>>              } else if (sscanf(line, "relative probe rect rgb "
>>                                "( %f , %f , %f , %f ) "
>>                                "( %f , %f , %f )",
>> 
>
> I don't think %f makes much sense here - you're reading floats,
> converting them to ints, then passing them to a function expecting
> ints.  For relative probes, it makes sense.
>
> I think you should do:
>
>               } else if (sscanf(line, "probe rect rgba "
>                                 "( %d , %d , %d , %d ) "
>                                 "( %f , %f , %f , %f )",
>                                 &x, &y, &w, &h,
>                                 c, c + 1, c + 2, c + 3) == 8) {
>
>                       if (!piglit_probe_rect_rgba(x, y, w, h, c)) {
>                               pass = false;
>                       }
>               }
>
> With that changed,
> Reviewed-by: Kenneth Graunke <[email protected]>

Nope, that seems obviously good.  And it might have caught an earlier
bug in my code generation :)  Thanks!

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to