[Piglit] [PATCH 3/4] Add a fast clear test for non-MSRT surfaces

[Neil Roberts]

"Pohjolainen, Topi" <topi.pohjolainen at intel.com> writes:

> On Tue, Dec 01, 2015 at 02:18:59PM +0000, Neil Roberts wrote:
>> "Pohjolainen, Topi" <topi.pohjolainen at intel.com> writes:
>> 
>> >> +	if (single_sample) {
>> >> +		glTexParameteri(tex_target,
>> >> +				GL_TEXTURE_MAG_FILTER,
>> >> +				GL_NEAREST);
>> >> +		glTexParameteri(tex_target,
>> >> +				GL_TEXTURE_MIN_FILTER,
>> >> +				GL_NEAREST);
>> >> +		glTexParameteri(tex_target,
>> >> +				GL_TEXTURE_MAX_LEVEL,
>> >> +				0);
>> >> +		glTexImage2D(tex_target,
>> >> +			     0, /* level */
>> >> +			     format->internalformat,
>> >> +			     128, 128, /* width/height */
>> >
>> > Do we need 128x128 instead of simply 1x1 for the driver to take a
>> > different path?
>> 
>> Yes that's right, tiling is disabled for textures with a height of 1 or
>> with a pitch smaller than a tile and fast clears are disabled for
>> non-tiled surfaces.
>> 
>> >> +			     0, /* border */
>> >> +			     GL_RGBA, GL_UNSIGNED_BYTE,
>> >> +			     NULL);
>> >> +	} else {
>> >> +		piglit_reset_gl_error();
>> >> +
>> >> +		glTexImage2DMultisample(tex_target,
>> >> +					2, /* samples */
>> >> +					format->internalformat,
>> >> +					128, 128, /* width/height */
>> >
>> > Before this was 1x1, it is not clear to me why we need to change this? Or do
>> > we?
>> 
>> You're right, there's no reason to change it. The size is completely
>> arbitrary here but I thought it looked a bit weird to have two different
>> sizes. Maybe I should explain this in the commit message? It would be
>> nice if you could put commit comments directly inline with the diff
>> somehow :)
>
> Should we explain the "glTexImage2D()" case in the test - the fact that i965
> needs large enough buffer for tiling to be used. And then perhaps another
> comment here explaining that in multisample case there is no such restriction.
>
> If we had something in the commit only one would need to be smart enough to
> go and see the commit in order to understand where the sizes come from. What
> do you think?

That sounds sensible, yes. I will make a v2.

- Neil