[Piglit] [PATCH 1/2] multisample-accuracy: Add an error margin for lit/unlit pixels

[Neil Roberts]

Mark Janes <mark.a.janes at intel.com> writes:

> We still get intermittent failures with this threshold, and have
> historically disabled the multisample accuracy tests in CI.
>
> Can we relax the threshold a bit more?

I guess the threshold is a bit of a kludge and I don’t think I picked
the value based on anything other than it being quite a small value that
made the test pass. So I guess in that sense it’s no more of a kludge to
pick a slightly higher value, so I’d be fine with that.

I guess ideally it might be nice to change the test pattern so that we
can mathematically check whether the geometry hits each pixel and count
it that way rather than relying on downsampling a high-res test image.

Regards,
- Neil

> -Mark
>
> Neil Roberts <neil at linux.intel.com> writes:
>
>> The unlit and totally_lit stats are supposed to count the pixels where
>> either a primitive completely covers the pixel or no primitive touches
>> it at all. However this is effectively only determined by checking
>> whether any primitive has intersected one of the supersample positions
>> of the reference image. It's completely possible for a primitive to
>> intersect the pixel boundary but completely miss any of the
>> supersample positions. The GL implementation is free to pick whatever
>> multisample positions it wants within the pixel boundary so it's also
>> possible for a primitive to intersect a multisample position but miss
>> all of the supersample positions of the reference image. To cope with
>> this this patch now allows a small margin of error for the pixels that
>> are either fully lit or fully unlit.
>>
>> This was causing the test to give false negatives for i965 with the
>> 16x MSAA positions because in that case it chooses positions that are
>> exactly on the pixel boundary and these would be outside of the grid
>> of positions chosen by the supersampling in the reference image. The
>> positions used are actually those described by the D3D API so it is
>> possible that other hardware would have the same problem.
>> ---
>>  tests/spec/ext_framebuffer_multisample/common.cpp | 43 +++++++++++++++++------
>>  1 file changed, 33 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/spec/ext_framebuffer_multisample/common.cpp b/tests/spec/ext_framebuffer_multisample/common.cpp
>> index d7be84f..fb42cc5 100644
>> --- a/tests/spec/ext_framebuffer_multisample/common.cpp
>> +++ b/tests/spec/ext_framebuffer_multisample/common.cpp
>> @@ -530,15 +530,6 @@ Test::measure_accuracy()
>>  		}
>>  	}
>>  
>> -	printf("Pixels that should be unlit\n");
>> -	unlit_stats.summarize();
>> -	pass = unlit_stats.is_perfect() && pass;
>> -	printf("Pixels that should be totally lit\n");
>> -	totally_lit_stats.summarize();
>> -	pass = totally_lit_stats.is_perfect() && pass;
>> -	printf("Pixels that should be partially lit\n");
>> -	partially_lit_stats.summarize();
>> -
>>  	double error_threshold;
>>  	if (test_resolve) {
>>  		/* For depth and stencil resolves, the implementation
>> @@ -558,7 +549,39 @@ Test::measure_accuracy()
>>  		error_threshold = 0.333 *
>>  			pow(0.6, log((double)effective_num_samples) / log(2.0));
>>  	}
>> -	printf("The error threshold for this test is %f\n", error_threshold);
>> +
>> +	/* The unlit and totally_lit stats are supposed to count the
>> +	 * pixels where either a primitive completely covers the pixel
>> +	 * or no primitive touches it at all. However this is
>> +	 * effectively only determined by checking whether any
>> +	 * primitive has intersected one of the supersample positions
>> +	 * of the reference image. It's completely possible for a
>> +	 * primitive to intersect the pixel boundary but completely
>> +	 * miss any of the supersample positions. The GL
>> +	 * implementation is free to pick whatever multisample
>> +	 * positions it wants within the pixel boundary so it's also
>> +	 * possible for a primitive to intersect a multisample
>> +	 * position but miss all of the supersample positions of the
>> +	 * reference image. To cope with this we allow a small margin
>> +	 * of error for the pixels that are either fully lit or fully
>> +	 * unlit.
>> +	 */
>> +	double full_pixel_threshold = error_threshold * 0.05f;
>> +
>> +	printf("Pixels that should be unlit\n");
>> +	unlit_stats.summarize();
>> +	pass = unlit_stats.is_better_than(full_pixel_threshold) && pass;
>> +	printf("Pixels that should be totally lit\n");
>> +	totally_lit_stats.summarize();
>> +	pass = totally_lit_stats.is_better_than(full_pixel_threshold) && pass;
>> +	printf("The error threshold for unlit and totally lit "
>> +               "pixels test is %f\n",
>> +               full_pixel_threshold);
>> +	printf("Pixels that should be partially lit\n");
>> +	partially_lit_stats.summarize();
>> +
>> +	printf("The error threshold for partially lit pixels is %f\n",
>> +               error_threshold);
>>  	pass = partially_lit_stats.is_better_than(error_threshold) && pass;
>>  	// TODO: deal with sRGB.
>>  	return pass;
>> -- 
>> 1.9.3
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit