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

[Pohjolainen, Topi]

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?