[Piglit] [PATCH] glsl-1.50-geometry-end-primitive: ensure position data is reasonably aligned

[Jose Fonseca]

On 12/01/16 17:51, Roland Scheidegger wrote:
> ping?
>
> Am 09.01.2016 um 05:40 schrieb sroland at vmware.com:
>> From: Roland Scheidegger <sroland at vmware.com>
>>
>> The rect halves comparison is in fact not valid in general. The reason is that
>> the same geometry does not have the same precision even if it it just shifted
>> by some fixed amount in one direction.
>> As an example, some calculated x value near 7 will be near 263 if drawn with
>> a viewport x value of 256. The value near 7 has 5 bits more precision -
>> when calculating the fixed-point values for rasterization (with subpixel
>> accuracy), the lower value thus may be rounded in a different direction
>> than the higher one, even with correct rounding (not even entirely sure what
>> "correct" rounding here means, nearest-floor or nearest-even or whatever, the
>> problem is independent from that). This can then cause different pixels to be
>> covered by the primitive.
>> This causes a failure with this test in llvmpipe when the rounding mode is
>> changed slightly (from "mostly" nearest-away-from-zero to nearest-even). I was
>> not able to reproduce this with this test on nvidia or amd hardware, but they
>> are definitely affected in theory by the same issue (proven by some quick and
>> dirty test).
>> So, do floor(tmp*2048) / 2048 to ensure everything is reasonably aligned. This
>> still retains some subpixel bits (but only very few).
>>
>> This may be a problem with more tests, albeit most tend to use vertex data
>> which is already sufficiently aligned (e.g. drawing simple aligned rects).
>>
>> v2: use mul/floor/div sequence suggested by Ian Romanick instead of add/sub -
>> trying to beat the compiler may not be reliable (as it could reorder
>> operations).
>> ---
>>   tests/spec/glsl-1.50/execution/geometry/end-primitive.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/spec/glsl-1.50/execution/geometry/end-primitive.c b/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
>> index 6df3a89..97d8375 100644
>> --- a/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
>> +++ b/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
>> @@ -105,7 +105,9 @@ static const char *spiral_text =
>>   	"  if (vertex_id % 2 == 1) r += 1.0;\n"
>>   	"  float max_r = b*sqrt(a*float(num_vertices)) + 1.0;\n"
>>   	"  r /= max_r;\n"
>> -	"  return r*vec2(cos(theta), sin(theta));\n"
>> +	"  vec2 tmp = r*vec2(cos(theta), sin(theta));\n"
>> +	"  // ensure reasonably aligned vertices\n"
>> +	"  return floor(tmp * 2048.0f) / 2048.0f;\n"
>>   	"}\n";
>>
>>   /**
>>
>
Looks good to me too.  Short and simple.

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>

Jose