Default window sizes and NPOT

Submitted by Brian Paul on Aug. 15, 2014, 3:19 p.m.

Details

Message ID 53EE2500.3040906@vmware.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Brian Paul Aug. 15, 2014, 3:19 p.m.
On 08/15/2014 08:52 AM, Ilia Mirkin wrote:
> Brian,
>
> You recently went on a campaign against setting .window_width/height
> in piglit tests, in order to fix issues on Windows I guess? I noticed
> that a bunch of failures on a NV17 card (GeForce4 MX 440) are due to
> the fact that the window sizes are now 160x160 (NPOT) and the tests
> use that in order to size their textures.
>
> What is the proper way to resolve this? Temporarily I've changed the
> default window width/height to 128, but that's not going to work more
> generally for the same reason you increased it to 160x160. Should I
> just do so on tests I identify as runnable on nouveau_vieux and that
> also have this issue? Perhaps increase to 256x256? Something else?
>
> You can find such tests with
>
> git grep -C2 piglit_width tests/fbo | grep glTexImage
>
> Not all of them run on nouveau_vieux, but some do.
>
> Thanks for any advice,

If a test like fbo-alphatest-formats.c creates a texture equal in size 
to the window and the window size is not explicit and the test doesn't 
check if NPOT textures are supported, I'd say the test is broken.  I 
didn't see any regressions when I removed the original window size 
because llvmpipe and more recent nvidia hardware support NPOT textures.

For these tests, the simplest thing is probably just setting the window 
size to 256x256.  Otherwise, we should probably create the texture with 
an explicit POT size and check that the window is at least that size 
before probing.  See sample patch attached.

-Brian

Patch hide | download patch | download mbox

diff --git a/tests/fbo/fbo-alphatest-formats.c b/tests/fbo/fbo-alphatest-formats.c
index a51d66c..f21a31a 100644
--- a/tests/fbo/fbo-alphatest-formats.c
+++ b/tests/fbo/fbo-alphatest-formats.c
@@ -58,6 +58,7 @@  static void alphatest(const float *rect, float alpha, GLenum func, float ref)
 static enum piglit_result test_format(const struct format_desc *format)
 {
 	GLboolean pass = GL_TRUE;
+	GLint texSize = 64;
 	GLuint tex, fb;
 	GLenum status;
 	int r, g, b, l, a, i;
@@ -74,6 +75,9 @@  static enum piglit_result test_format(const struct format_desc *format)
 	float pos6[] = { 0.5,  -1.0, 0.25, 2.0};
 	float pos7[] = { 0.75, -1.0, 0.25, 2.0};
 
+	if (piglit_width < texSize)
+		return PIGLIT_SKIP;  /* window too small */
+
         if (format->base_internal_format == GL_DEPTH_COMPONENT ||
             format->base_internal_format == GL_DEPTH_STENCIL ||
 	    format->base_internal_format == GL_ALPHA)
@@ -85,14 +89,14 @@  static enum piglit_result test_format(const struct format_desc *format)
 
 	glGenFramebuffersEXT(1, &fb);
 	glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, fb);
-	glViewport(0, 0, piglit_width, piglit_height);
+	glViewport(0, 0, texSize, texSize);
 
 	glGenTextures(1, &tex);
 	glBindTexture(GL_TEXTURE_2D, tex);
 	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
 	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
 	glTexImage2D(GL_TEXTURE_2D, 0, format->internalformat,
-		     piglit_width, piglit_height, 0,
+                     texSize, texSize, 0,
 		     GL_RGBA, GL_FLOAT, NULL);
 
 	glGetTexLevelParameteriv(GL_TEXTURE_2D, 0,
@@ -196,35 +200,35 @@  static enum piglit_result test_format(const struct format_desc *format)
 	alphatest(pos6, 0.8, GL_NOTEQUAL, 0.8);
 	alphatest(pos7, 0.3, GL_NEVER, 3);
 
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 1 / 16, 0, cpass, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 1 / 16, 0, cpass, NULL)) {
 		printf("  FAIL when testing FBO result, 1: 0.2 < 0.25.\n");
 		pass = GL_FALSE;
         }
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 3 / 16, 0, cfail, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 3 / 16, 0, cfail, NULL)) {
 		printf("  FAIL when testing FBO result, 2: 0.96 <= 0.92.\n");
 		pass = GL_FALSE;
         }
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 5 / 16, 0, cpass, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 5 / 16, 0, cpass, NULL)) {
 		printf("  FAIL when testing FBO result, 3: 0.6 > 0.55.\n");
 		pass = GL_FALSE;
         }
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 7 / 16, 0, cpass, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 7 / 16, 0, cpass, NULL)) {
 		printf("  FAIL when testing FBO result, 4: 0.9 > 0.1.\n");
 		pass = GL_FALSE;
         }
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 9 / 16, 0, cfail, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 9 / 16, 0, cfail, NULL)) {
 		printf("  FAIL when testing FBO result, 5: 0.35 >= 0.4.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 11 / 16, 0, cpass, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 11 / 16, 0, cpass, NULL)) {
 		printf("  FAIL when testing FBO result, 6: 0.4 == 0.4.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 13 / 16, 0, cfail, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 13 / 16, 0, cfail, NULL)) {
 		printf("  FAIL when testing FBO result, 7: 0.8 != 0.8.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 15 / 16, 0, cfail, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 15 / 16, 0, cfail, NULL)) {
 		printf("  FAIL when testing FBO result, 8: FALSE.\n");
 		pass = GL_FALSE;
 	}
@@ -234,7 +238,6 @@  static enum piglit_result test_format(const struct format_desc *format)
 	 * Now check alpha test using the window buffer.
 	 */
 	glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, piglit_winsys_fbo);
-	glViewport(0, 0, piglit_width, piglit_height);
 
 	glTexEnvi(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_COMBINE);
 	glTexEnvi(GL_TEXTURE_ENV, GL_COMBINE_RGB,   GL_REPLACE);
@@ -279,35 +282,35 @@  static enum piglit_result test_format(const struct format_desc *format)
 		/* same R,G,B,A that we computed above */
 	}
 
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 1 / 16, 0, cpass, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 1 / 16, 0, cpass, NULL)) {
 		printf("  FAIL when testing window result, 1: 0.2 < 0.25.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 3 / 16, 0, cfail, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 3 / 16, 0, cfail, NULL)) {
 		printf("  FAIL when testing window result, 2: 0.96 <= 0.92.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 5 / 16, 0, cpass, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 5 / 16, 0, cpass, NULL)) {
 		printf("  FAIL when testing window result, 3: 0.6 > 0.55.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 7 / 16, 0, cpass, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 7 / 16, 0, cpass, NULL)) {
 		printf("  FAIL when testing window result, 4: 0.9 > 0.1.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 9 / 16, 0, cfail, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 9 / 16, 0, cfail, NULL)) {
 		printf("  FAIL when testing window result, 5: 0.35 >= 0.4.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 11 / 16, 0, cpass, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 11 / 16, 0, cpass, NULL)) {
 		printf("  FAIL when testing window result, 6: 0.4 == 0.4.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 13 / 16, 0, cfail, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 13 / 16, 0, cfail, NULL)) {
 		printf("  FAIL when testing window result, 7: 0.8 != 0.8.\n");
 		pass = GL_FALSE;
 	}
-	if (!piglit_probe_pixel_rgb_silent(piglit_width * 15 / 16, 0, cfail, NULL)) {
+	if (!piglit_probe_pixel_rgb_silent(texSize * 15 / 16, 0, cfail, NULL)) {
 		printf("  FAIL when testing window result, 8: FALSE.\n");
 		pass = GL_FALSE;
 	}

Comments

On Fri, Aug 15, 2014 at 11:19 AM, Brian Paul <brianp@vmware.com> wrote:
> On 08/15/2014 08:52 AM, Ilia Mirkin wrote:
>>
>> Brian,
>>
>> You recently went on a campaign against setting .window_width/height
>> in piglit tests, in order to fix issues on Windows I guess? I noticed
>> that a bunch of failures on a NV17 card (GeForce4 MX 440) are due to
>> the fact that the window sizes are now 160x160 (NPOT) and the tests
>> use that in order to size their textures.
>>
>> What is the proper way to resolve this? Temporarily I've changed the
>> default window width/height to 128, but that's not going to work more
>> generally for the same reason you increased it to 160x160. Should I
>> just do so on tests I identify as runnable on nouveau_vieux and that
>> also have this issue? Perhaps increase to 256x256? Something else?
>>
>> You can find such tests with
>>
>> git grep -C2 piglit_width tests/fbo | grep glTexImage
>>
>> Not all of them run on nouveau_vieux, but some do.
>>
>> Thanks for any advice,
>
>
> If a test like fbo-alphatest-formats.c creates a texture equal in size to
> the window and the window size is not explicit and the test doesn't check if
> NPOT textures are supported, I'd say the test is broken.  I didn't see any
> regressions when I removed the original window size because llvmpipe and
> more recent nvidia hardware support NPOT textures.
>
> For these tests, the simplest thing is probably just setting the window size
> to 256x256.  Otherwise, we should probably create the texture with an
> explicit POT size and check that the window is at least that size before
> probing.  See sample patch attached.

I think the path of least resistance may be to just increase the
window size to 256x256 for those tests. I've set it to a POT size
globally for now, will compare results and send a patch updating the
tests that end up being executable with nouveau_vieux. (Although I
guess even GeForce FX 5000 series doesn't have NPOT support, at least
not with the nv30 driver.)