On Tue, Dec 01, 2015 at 02:47:50PM +0000, Neil Roberts wrote: > "Pohjolainen, Topi" <[email protected]> writes: > > > On Tue, Dec 01, 2015 at 02:18:59PM +0000, Neil Roberts wrote: > >> "Pohjolainen, Topi" <[email protected]> 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.
And you can add to all four patches: Reviewed-by: Topi Pohjolainen <[email protected]> _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
