[Piglit] [PATCH] ext_render_snorm-render: fix read back assumptions

[Tapani Pälli]

On 29.08.2018 00:23, Nanley Chery wrote:
> On Tue, Aug 28, 2018 at 09:58:14AM +0300, Tapani Pälli wrote:
>> Test assumed that we can read back directly from R8, RG8 fbo but
>> this is not actually enabled by the extension. Patch introduces
>> additional read from window to sanity check fbo contents.
>>
> 
> I think we're allowed to read from R8 and RG8 SNORM FBOs. Page 340 of
> the GLES 3.1 spec says that apps can query the driver to find out which
> formats are supported for ReadPixels:
> 
>     The second is an implementation-chosen format from among those
>     defined in table 8.2, excluding formats DEPTH_COMPONENT and
>     DEPTH_STENCIL. The values of format and type for this format may be
>     determined by calling GetIntegerv with the symbolic constants
>     IMPLEMENTATION_COLOR_READ_FORMAT and IMPLEMENTATION_COLOR_READ_TYPE,
>     respectively. The implementation-chosen format may vary depending on
>     the format of the selected read buffer of the currently bound read
>     framebuffer.
> 
> Table 8.2 has entries for R8 and RG8 SNORM.

Maybe the issue is that reading should be allowed only to matching 
number of components with GL_BYTE? I'm currently reading every format as 
GL_RGBA but I could change to use GL_RED, GL_RG, GL_RGBA depending on 
the base format. If I enforce such rules for GL_BYTE formats in Mesa the 
rest of failing dEQP tests start to pass. So for example, reading fbo 
with internalFormat of GL_R8_SNORM would require:

glReadPixels(0, 0, w, h, GL_RED, GL_BYTE, pix);

Does this make sense?



> -Nanley
> 
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   tests/spec/ext_render_snorm/render.c | 32 ++++++++++++++++++++++++++++----
>>   1 file changed, 28 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/spec/ext_render_snorm/render.c b/tests/spec/ext_render_snorm/render.c
>> index 1d4a69c2d..ad309f17c 100644
>> --- a/tests/spec/ext_render_snorm/render.c
>> +++ b/tests/spec/ext_render_snorm/render.c
>> @@ -82,10 +82,11 @@ static const struct fmt_test {
>>   	GLenum iformat;
>>   	GLenum base_format;
>>   	unsigned bpp;
>> +	bool can_read;
>>   } tests[] = {
>> -	{ GL_R8_SNORM,		GL_RED,		1, },
>> -	{ GL_RG8_SNORM,		GL_RG,		2, },
>> -	{ GL_RGBA8_SNORM,	GL_RGBA,	4, },
>> +	{ GL_R8_SNORM,		GL_RED,		1,	false },
>> +	{ GL_RG8_SNORM,		GL_RG,		2,	false },
>> +	{ GL_RGBA8_SNORM,	GL_RGBA,	4, 	true  },
>>   };
>>   
>>   static GLuint prog;
>> @@ -229,6 +230,25 @@ verify_contents(const struct fmt_test *test)
>>   	return result;
>>   }
>>   
>> +static bool
>> +verify_contents_float(const struct fmt_test *test)
>> +{
>> +	/* Setup expected value, alpha is always max in the test. */
>> +	char value[4] = { 0, 0, 0, SCHAR_MAX };
>> +	value_for_format(test, value);
>> +
>> +	const float expected[4] = {
>> +		value[0] / SCHAR_MAX,
>> +		value[1] / SCHAR_MAX,
>> +		value[2] / SCHAR_MAX,
>> +		value[3] / SCHAR_MAX,
>> +	};
>> +
>> +	bool res = piglit_probe_rect_rgba(0, 0, piglit_width, piglit_height,
>> +					  expected);
>> +	return res;
>> +}
>> +
>>   static bool
>>   test_format(const struct fmt_test *test)
>>   {
>> @@ -282,13 +302,17 @@ test_format(const struct fmt_test *test)
>>   	glDeleteTextures(1, &tmp_tex);
>>   
>>   	/* Verify contents. */
>> -	pass &= verify_contents(test);
>> +	if (test->can_read)
>> +		pass &= verify_contents(test);
>>   
>>   	glDeleteFramebuffers(1, &fbo);
>>   
>>   	/* Render fbo contents to window. */
>>   	render_texture(fbo_tex, GL_TEXTURE_2D, 0);
>>   
>> +	/* Verify window contents. */
>> +	pass &= verify_contents_float(test);
>> +
>>   	piglit_present_results();
>>   
>>   	glDeleteTextures(1, &fbo_tex);
>> -- 
>> 2.14.4
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit