FW: [PATCH] fixed oes compressed etc2 texture miptree failure

Submitted by Guo, Johney on April 23, 2014, 2:41 a.m.

Details

Message ID 60711ADC69ED054383C0ADA87BAE4B5101433D70@SCYBEXDAG01.amd.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Guo, Johney April 23, 2014, 2:41 a.m.
- glut config.window_width should be > 160 so as to avoid warning
     message.
   - fopen ktx texture file should be binary mode
   - viewport need be reset in each display, since it has changed in
     glut reshape() function

Signed-off-by: weijun <weijun.guo@amd.com>
---
 tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c | 3 ++-
 tests/util/piglit_ktx.c                                   | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

--
1.8.4.msysgit.0

Patch hide | download patch | download mbox

diff --git a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
index 59d8748..eeda3cb 100644
--- a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
+++ b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
@@ -289,6 +289,7 @@  piglit_display(void)
 	bool pass = true;
 
 	glClear(GL_COLOR_BUFFER_BIT);
+	glViewport(0, 0, window_width, window_height);
 	for (level = 0; level < num_levels; ++level) {
 		glUniform2f(level_pixel_size_loc,
 		            (float) level_width,
@@ -339,7 +340,7 @@  PIGLIT_GL_TEST_CONFIG_BEGIN
 
 	config.supports_gl_es_version = 30;
 
-	config.window_width = 150;
+	config.window_width = 160;
 	config.window_height = 150;
 	config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;  PIGLIT_GL_TEST_CONFIG_END diff --git a/tests/util/piglit_ktx.c b/tests/util/piglit_ktx.c index b60f737..d844540 100644
--- a/tests/util/piglit_ktx.c
+++ b/tests/util/piglit_ktx.c
@@ -436,7 +436,7 @@  piglit_ktx_read_file(const char *filename)
 	if (self == NULL)
 		goto out_of_memory;
 
-	file = fopen(filename, "r");
+	file = fopen(filename, "rb");
 	if (file == NULL)
 		goto bad_open;
 
@@ -521,7 +521,7 @@  piglit_ktx_write_file(struct piglit_ktx *self, const char *filename)
 	size_t size_written = 0;
 	bool ok = true;
 
-	file = fopen(filename, "w");
+	file = fopen(filename, "wb");
 	if (file == NULL)
 		goto bad_open;
 

Comments

On Tue, Apr 22, 2014 at 7:41 PM, Guo, Johney <Weijun.Guo@amd.com> wrote:
>    - glut config.window_width should be > 160 so as to avoid warning
>      message.
What warning are you seeing for width < 160? Add the details to commit message.

>    - fopen ktx texture file should be binary mode
>    - viewport need be reset in each display, since it has changed in
>      glut reshape() function
This test doesn't use glut reshape() function. I think viewport stays unchanged
in piglit_display() function.

> Signed-off-by: weijun <weijun.guo@amd.com>
> ---
>  tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c | 3 ++-
>  tests/util/piglit_ktx.c                                   | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
> index 59d8748..eeda3cb 100644
> --- a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
> +++ b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
> @@ -289,6 +289,7 @@ piglit_display(void)
>         bool pass = true;
>
>         glClear(GL_COLOR_BUFFER_BIT);
> +       glViewport(0, 0, window_width, window_height);
>         for (level = 0; level < num_levels; ++level) {
>                 glUniform2f(level_pixel_size_loc,
>                             (float) level_width,
> @@ -339,7 +340,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>
>         config.supports_gl_es_version = 30;
>
> -       config.window_width = 150;
> +       config.window_width = 160;
>         config.window_height = 150;
>         config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;  PIGLIT_GL_TEST_CONFIG_END diff --git a/tests/util/piglit_ktx.c b/tests/util/piglit_ktx.c index b60f737..d844540 100644
> --- a/tests/util/piglit_ktx.c
> +++ b/tests/util/piglit_ktx.c
> @@ -436,7 +436,7 @@ piglit_ktx_read_file(const char *filename)
>         if (self == NULL)
>                 goto out_of_memory;
>
> -       file = fopen(filename, "r");
> +       file = fopen(filename, "rb");
>         if (file == NULL)
>                 goto bad_open;
>
> @@ -521,7 +521,7 @@ piglit_ktx_write_file(struct piglit_ktx *self, const char *filename)
>         size_t size_written = 0;
>         bool ok = true;
>
> -       file = fopen(filename, "w");
> +       file = fopen(filename, "wb");
>         if (file == NULL)
>                 goto bad_open;
>
> --
> 1.8.4.msysgit.0
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
See tests/util/piglit-framework-gl/piglit_glut_framework.c:84:
default_reshape_func(int w, int h)
{
	if (piglit_automatic &&
	    (w != piglit_width ||
	     h != piglit_height)) {
		printf("Got spurious window resize in automatic run "
		       "(%d,%d to %d,%d)\n", piglit_width, piglit_height, w, h);
		piglit_report_result(PIGLIT_WARN);
	}

	piglit_width = w;
	piglit_height = h;

	glViewport(0, 0, w, h);
}

My OS is win8. If piglit_width < 160, window manager will force it to 160.
Many piglit tests have set configure width as 150,  then they will all report as "warn".
At least the following,
$ grep -Irne "\<150\>" tests | grep width
tests/fbo/fbo-blit-d24s8.c:45:  config.window_width = 150;
tests/fbo/fbo-blit.c:43:        config.window_width = 150;
tests/fbo/fbo-copypix.c:40:     config.window_width = 150;
tests/fbo/fbo-readdrawpix.c:40: config.window_width = 150;
tests/spec/arb_es2_compatibility/arb_es2_compatibility-depthrangef.c:36:        config.window_width = 150;
tests/texturing/incomplete-texture.c:38:        config.window_width = 150;
tests/texturing/shaders/textureSize.c:55:       config.window_width = 150;
tests/texturing/texsubimage.c:42:       config.window_width = 150;
tests/util/piglit-framework-gl.h:272:                config.window_width = 150;

What is more, 	glViewport(0, 0, w, h)  will change the viewport to (w,h),  so in each glut display() function, glViewPort() should be recalled for validity.


-----Original Message-----
From: Anuj Phogat [mailto:anuj.phogat@gmail.com] 

Sent: 2014年4月24日 7:54
To: Guo, Johney
Cc: Ian Romanick; piglit@lists.freedesktop.org
Subject: Re: [Piglit] FW: [PATCH] fixed oes compressed etc2 texture miptree failure

On Tue, Apr 22, 2014 at 7:41 PM, Guo, Johney <Weijun.Guo@amd.com> wrote:
>    - glut config.window_width should be > 160 so as to avoid warning

>      message.

What warning are you seeing for width < 160? Add the details to commit message.

>    - fopen ktx texture file should be binary mode

>    - viewport need be reset in each display, since it has changed in

>      glut reshape() function

This test doesn't use glut reshape() function. I think viewport stays unchanged in piglit_display() function.

> Signed-off-by: weijun <weijun.guo@amd.com>

> ---

>  tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c | 3 ++-

>  tests/util/piglit_ktx.c                                   | 4 ++--

>  2 files changed, 4 insertions(+), 3 deletions(-)

>

> diff --git a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c 

> b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c

> index 59d8748..eeda3cb 100644

> --- a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c

> +++ b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c

> @@ -289,6 +289,7 @@ piglit_display(void)

>         bool pass = true;

>

>         glClear(GL_COLOR_BUFFER_BIT);

> +       glViewport(0, 0, window_width, window_height);

>         for (level = 0; level < num_levels; ++level) {

>                 glUniform2f(level_pixel_size_loc,

>                             (float) level_width, @@ -339,7 +340,7 @@ 

> PIGLIT_GL_TEST_CONFIG_BEGIN

>

>         config.supports_gl_es_version = 30;

>

> -       config.window_width = 150;

> +       config.window_width = 160;

>         config.window_height = 150;

>         config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | 

> PIGLIT_GL_VISUAL_RGBA;  PIGLIT_GL_TEST_CONFIG_END diff --git 

> a/tests/util/piglit_ktx.c b/tests/util/piglit_ktx.c index 

> b60f737..d844540 100644

> --- a/tests/util/piglit_ktx.c

> +++ b/tests/util/piglit_ktx.c

> @@ -436,7 +436,7 @@ piglit_ktx_read_file(const char *filename)

>         if (self == NULL)

>                 goto out_of_memory;

>

> -       file = fopen(filename, "r");

> +       file = fopen(filename, "rb");

>         if (file == NULL)

>                 goto bad_open;

>

> @@ -521,7 +521,7 @@ piglit_ktx_write_file(struct piglit_ktx *self, const char *filename)

>         size_t size_written = 0;

>         bool ok = true;

>

> -       file = fopen(filename, "w");

> +       file = fopen(filename, "wb");

>         if (file == NULL)

>                 goto bad_open;

>

> --

> 1.8.4.msysgit.0

> _______________________________________________

> Piglit mailing list

> Piglit@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/piglit
On Thu, Apr 24, 2014 at 3:41 AM, Guo, Johney <Weijun.Guo@amd.com> wrote:
> See tests/util/piglit-framework-gl/piglit_glut_framework.c:84:
> default_reshape_func(int w, int h)
> {
>         if (piglit_automatic &&
>             (w != piglit_width ||
>              h != piglit_height)) {
>                 printf("Got spurious window resize in automatic run "
>                        "(%d,%d to %d,%d)\n", piglit_width, piglit_height, w, h);
>                 piglit_report_result(PIGLIT_WARN);
>         }
>
>         piglit_width = w;
>         piglit_height = h;
>
>         glViewport(0, 0, w, h);
> }
Yes. It's getting used for Windows.
>
> My OS is win8. If piglit_width < 160, window manager will force it to 160.
> Many piglit tests have set configure width as 150,  then they will all report as "warn".
There is a comment about setting the default window width and height in
 tests/util/piglit-framework-gl.h:293

/* Default window size.  Note: Win7's min window width */    \
/* seems to be 116 pixels.  When the window size is */       \
/* unexpectedly resized, tests are marked as "WARN". */      \
/* Let's use a larger default to avoid that. */              \
config.window_width = 150;                                   \
config.window_height = 150;                                  \

As you're describing, it seems like win8 changed the minimum width
requirement. But I don't have windows setup to verify that. It would be
nice if someone verifies it before making all the changes in piglit tests.

> At least the following,
> $ grep -Irne "\<150\>" tests | grep width
> tests/fbo/fbo-blit-d24s8.c:45:  config.window_width = 150;
> tests/fbo/fbo-blit.c:43:        config.window_width = 150;
> tests/fbo/fbo-copypix.c:40:     config.window_width = 150;
> tests/fbo/fbo-readdrawpix.c:40: config.window_width = 150;
> tests/spec/arb_es2_compatibility/arb_es2_compatibility-depthrangef.c:36:        config.window_width = 150;
> tests/texturing/incomplete-texture.c:38:        config.window_width = 150;
> tests/texturing/shaders/textureSize.c:55:       config.window_width = 150;
> tests/texturing/texsubimage.c:42:       config.window_width = 150;
> tests/util/piglit-framework-gl.h:272:                config.window_width = 150;
>
> What is more,   glViewport(0, 0, w, h)  will change the viewport to (w,h),  so in each glut display() function, glViewPort() should be recalled for validity.
>
>
> -----Original Message-----
> From: Anuj Phogat [mailto:anuj.phogat@gmail.com]
> Sent: 2014年4月24日 7:54
> To: Guo, Johney
> Cc: Ian Romanick; piglit@lists.freedesktop.org
> Subject: Re: [Piglit] FW: [PATCH] fixed oes compressed etc2 texture miptree failure
>
> On Tue, Apr 22, 2014 at 7:41 PM, Guo, Johney <Weijun.Guo@amd.com> wrote:
>>    - glut config.window_width should be > 160 so as to avoid warning
>>      message.
> What warning are you seeing for width < 160? Add the details to commit message.
>
>>    - fopen ktx texture file should be binary mode
>>    - viewport need be reset in each display, since it has changed in
>>      glut reshape() function
> This test doesn't use glut reshape() function. I think viewport stays unchanged in piglit_display() function.
>
>> Signed-off-by: weijun <weijun.guo@amd.com>
>> ---
>>  tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c | 3 ++-
>>  tests/util/piglit_ktx.c                                   | 4 ++--
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
>> b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
>> index 59d8748..eeda3cb 100644
>> --- a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
>> +++ b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
>> @@ -289,6 +289,7 @@ piglit_display(void)
>>         bool pass = true;
>>
>>         glClear(GL_COLOR_BUFFER_BIT);
>> +       glViewport(0, 0, window_width, window_height);
>>         for (level = 0; level < num_levels; ++level) {
>>                 glUniform2f(level_pixel_size_loc,
>>                             (float) level_width, @@ -339,7 +340,7 @@
>> PIGLIT_GL_TEST_CONFIG_BEGIN
>>
>>         config.supports_gl_es_version = 30;
>>
>> -       config.window_width = 150;
>> +       config.window_width = 160;
>>         config.window_height = 150;
>>         config.window_visual = PIGLIT_GL_VISUAL_DOUBLE |
>> PIGLIT_GL_VISUAL_RGBA;  PIGLIT_GL_TEST_CONFIG_END diff --git
>> a/tests/util/piglit_ktx.c b/tests/util/piglit_ktx.c index
>> b60f737..d844540 100644
>> --- a/tests/util/piglit_ktx.c
>> +++ b/tests/util/piglit_ktx.c
>> @@ -436,7 +436,7 @@ piglit_ktx_read_file(const char *filename)
>>         if (self == NULL)
>>                 goto out_of_memory;
>>
>> -       file = fopen(filename, "r");
>> +       file = fopen(filename, "rb");
>>         if (file == NULL)
>>                 goto bad_open;
>>
>> @@ -521,7 +521,7 @@ piglit_ktx_write_file(struct piglit_ktx *self, const char *filename)
>>         size_t size_written = 0;
>>         bool ok = true;
>>
>> -       file = fopen(filename, "w");
>> +       file = fopen(filename, "wb");
>>         if (file == NULL)
>>                 goto bad_open;
>>
>> --
>> 1.8.4.msysgit.0
>> _______________________________________________
>> Piglit mailing list
>> Piglit@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit
This should be two patches:  one that changes the test and one that
changes piglit_ktx.c.

On 04/22/2014 07:41 PM, Guo, Johney wrote:
>    - glut config.window_width should be > 160 so as to avoid warning
>      message.
>    - fopen ktx texture file should be binary mode
>    - viewport need be reset in each display, since it has changed in
>      glut reshape() function
> 
> Signed-off-by: weijun <weijun.guo@amd.com>
> ---
>  tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c | 3 ++-
>  tests/util/piglit_ktx.c                                   | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
> index 59d8748..eeda3cb 100644
> --- a/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
> +++ b/tests/spec/gles-3.0/oes_compressed_etc2_texture-miptree.c
> @@ -289,6 +289,7 @@ piglit_display(void)
>  	bool pass = true;
>  
>  	glClear(GL_COLOR_BUFFER_BIT);
> +	glViewport(0, 0, window_width, window_height);
>  	for (level = 0; level < num_levels; ++level) {
>  		glUniform2f(level_pixel_size_loc,
>  		            (float) level_width,
> @@ -339,7 +340,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>  
>  	config.supports_gl_es_version = 30;
>  
> -	config.window_width = 150;
> +	config.window_width = 160;
>  	config.window_height = 150;
>  	config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;  PIGLIT_GL_TEST_CONFIG_END diff --git a/tests/util/piglit_ktx.c b/tests/util/piglit_ktx.c index b60f737..d844540 100644
> --- a/tests/util/piglit_ktx.c
> +++ b/tests/util/piglit_ktx.c
> @@ -436,7 +436,7 @@ piglit_ktx_read_file(const char *filename)
>  	if (self == NULL)
>  		goto out_of_memory;
>  
> -	file = fopen(filename, "r");
> +	file = fopen(filename, "rb");
>  	if (file == NULL)
>  		goto bad_open;
>  
> @@ -521,7 +521,7 @@ piglit_ktx_write_file(struct piglit_ktx *self, const char *filename)
>  	size_t size_written = 0;
>  	bool ok = true;
>  
> -	file = fopen(filename, "w");
> +	file = fopen(filename, "wb");
>  	if (file == NULL)
>  		goto bad_open;
>  
> --
> 1.8.4.msysgit.0
>
On 04/24/2014 03:41 AM, Guo, Johney wrote:
> See tests/util/piglit-framework-gl/piglit_glut_framework.c:84:
> default_reshape_func(int w, int h)
> {
> 	if (piglit_automatic &&
> 	    (w != piglit_width ||
> 	     h != piglit_height)) {
> 		printf("Got spurious window resize in automatic run "
> 		       "(%d,%d to %d,%d)\n", piglit_width, piglit_height, w, h);
> 		piglit_report_result(PIGLIT_WARN);
> 	}
> 
> 	piglit_width = w;
> 	piglit_height = h;
> 
> 	glViewport(0, 0, w, h);
> }
> 
> My OS is win8. If piglit_width < 160, window manager will force it to 160.

There was a similar issue a few years ago with Win7.  I believe Brian
Paul fixed this by modifying the framework to enforce a minimum window
size depending on the operating system.  It's better to fix this issue
once for all tests than change each individual test.  This is especially
true since we may add more tests with too-small window sizes (since
we're not on Windows).

Brian, does that sound right?

> Many piglit tests have set configure width as 150,  then they will all report as "warn".
> At least the following,
> $ grep -Irne "\<150\>" tests | grep width
> tests/fbo/fbo-blit-d24s8.c:45:  config.window_width = 150;
> tests/fbo/fbo-blit.c:43:        config.window_width = 150;
> tests/fbo/fbo-copypix.c:40:     config.window_width = 150;
> tests/fbo/fbo-readdrawpix.c:40: config.window_width = 150;
> tests/spec/arb_es2_compatibility/arb_es2_compatibility-depthrangef.c:36:        config.window_width = 150;
> tests/texturing/incomplete-texture.c:38:        config.window_width = 150;
> tests/texturing/shaders/textureSize.c:55:       config.window_width = 150;
> tests/texturing/texsubimage.c:42:       config.window_width = 150;
> tests/util/piglit-framework-gl.h:272:                config.window_width = 150;
> 
> What is more, 	glViewport(0, 0, w, h)  will change the viewport to (w,h),  so in each glut display() function, glViewPort() should be recalled for validity.
On 04/27/2014 12:45 PM, Ian Romanick wrote:
> On 04/24/2014 03:41 AM, Guo, Johney wrote:
>> See tests/util/piglit-framework-gl/piglit_glut_framework.c:84:
>> default_reshape_func(int w, int h)
>> {
>> 	if (piglit_automatic &&
>> 	    (w != piglit_width ||
>> 	     h != piglit_height)) {
>> 		printf("Got spurious window resize in automatic run "
>> 		       "(%d,%d to %d,%d)\n", piglit_width, piglit_height, w, h);
>> 		piglit_report_result(PIGLIT_WARN);
>> 	}
>>
>> 	piglit_width = w;
>> 	piglit_height = h;
>>
>> 	glViewport(0, 0, w, h);
>> }
>>
>> My OS is win8. If piglit_width < 160, window manager will force it to 160.
>
> There was a similar issue a few years ago with Win7.  I believe Brian
> Paul fixed this by modifying the framework to enforce a minimum window
> size depending on the operating system.  It's better to fix this issue
> once for all tests than change each individual test.  This is especially
> true since we may add more tests with too-small window sizes (since
> we're not on Windows).
>
> Brian, does that sound right?

Basically, I changed the default window size to be 150x150 because on 
Windows (7 and maybe XP) if the window size was less than 116 pixels 
wide, it would get bumped up to 116 and that would cause the test to fail.

I did a quick search of git history and found this one:

commit 1894ef34c5e3cc0daefd1241b31a2b2ae32c3347
Author: Brian Paul <brianp@vmware.com>
Date:   Fri Apr 5 12:32:46 2013 -0600

     lodbias: increase window size to fix test on Windows

     Before, the 96x96 window got bumped up to 116x96 by Windows and we
     saw a "Got spurious window resize in automatic run" message and the
     test reported "warn".

So the rule of thumb now is don't set the config.window_width, height 
vars unless you really have to, and be prepared for window resizes if 
you do.

I've rarely tested Windows 8 so I wasn't aware of any problems there 
with the window size.  If width=160 is the new magic number, we may have 
to bump up config.window_width, height again.

Johney, can you try bumping up the default window size in 
tests/util/piglit-framework-gl.h and see if that solve the problem?

I can likewise do that here and see if there's any unexpected 
regressions from changing the window size.


>
>> Many piglit tests have set configure width as 150,  then they will all report as "warn".
>> At least the following,
>> $ grep -Irne "\<150\>" tests | grep width
>> tests/fbo/fbo-blit-d24s8.c:45:  config.window_width = 150;
>> tests/fbo/fbo-blit.c:43:        config.window_width = 150;
>> tests/fbo/fbo-copypix.c:40:     config.window_width = 150;
>> tests/fbo/fbo-readdrawpix.c:40: config.window_width = 150;
>> tests/spec/arb_es2_compatibility/arb_es2_compatibility-depthrangef.c:36:        config.window_width = 150;
>> tests/texturing/incomplete-texture.c:38:        config.window_width = 150;
>> tests/texturing/shaders/textureSize.c:55:       config.window_width = 150;
>> tests/texturing/texsubimage.c:42:       config.window_width = 150;
>> tests/util/piglit-framework-gl.h:272:                config.window_width = 150;
>>
>> What is more, 	glViewport(0, 0, w, h)  will change the viewport to (w,h),  so in each glut display() function, glViewPort() should be recalled for validity
>


The last time I bumped up the default window size I had to fix a bunch 
of tests that had hard-coded dimensions.  Looks like there's a few more 
to tend to...

-Brian
Bumping up to 160 is simple way to fix the "warn"s on Win8.
For further compatibility, it is better to set the default window fix sized to disable resize function.
BTW,Will my patch be accepted for this test?

On 04/27/2014 12:45 PM, Ian Romanick wrote:
> On 04/24/2014 03:41 AM, Guo, Johney wrote:
>> See tests/util/piglit-framework-gl/piglit_glut_framework.c:84:
>> default_reshape_func(int w, int h)
>> {
>> 	if (piglit_automatic &&
>> 	    (w != piglit_width ||
>> 	     h != piglit_height)) {
>> 		printf("Got spurious window resize in automatic run "
>> 		       "(%d,%d to %d,%d)\n", piglit_width, piglit_height, w, h);
>> 		piglit_report_result(PIGLIT_WARN);
>> 	}
>>
>> 	piglit_width = w;
>> 	piglit_height = h;
>>
>> 	glViewport(0, 0, w, h);
>> }
>>
>> My OS is win8. If piglit_width < 160, window manager will force it to 160.
>
> There was a similar issue a few years ago with Win7.  I believe Brian 
> Paul fixed this by modifying the framework to enforce a minimum window 
> size depending on the operating system.  It's better to fix this issue 
> once for all tests than change each individual test.  This is 
> especially true since we may add more tests with too-small window 
> sizes (since we're not on Windows).
>
> Brian, does that sound right?

Basically, I changed the default window size to be 150x150 because on Windows (7 and maybe XP) if the window size was less than 116 pixels wide, it would get bumped up to 116 and that would cause the test to fail.

I did a quick search of git history and found this one:

commit 1894ef34c5e3cc0daefd1241b31a2b2ae32c3347
Author: Brian Paul <brianp@vmware.com>
Date:   Fri Apr 5 12:32:46 2013 -0600

     lodbias: increase window size to fix test on Windows

     Before, the 96x96 window got bumped up to 116x96 by Windows and we
     saw a "Got spurious window resize in automatic run" message and the
     test reported "warn".

So the rule of thumb now is don't set the config.window_width, height vars unless you really have to, and be prepared for window resizes if you do.

I've rarely tested Windows 8 so I wasn't aware of any problems there with the window size.  If width=160 is the new magic number, we may have to bump up config.window_width, height again.

Johney, can you try bumping up the default window size in tests/util/piglit-framework-gl.h and see if that solve the problem?

I can likewise do that here and see if there's any unexpected regressions from changing the window size.


>
>> Many piglit tests have set configure width as 150,  then they will all report as "warn".
>> At least the following,
>> $ grep -Irne "\<150\>" tests | grep width
>> tests/fbo/fbo-blit-d24s8.c:45:  config.window_width = 150;
>> tests/fbo/fbo-blit.c:43:        config.window_width = 150;
>> tests/fbo/fbo-copypix.c:40:     config.window_width = 150;
>> tests/fbo/fbo-readdrawpix.c:40: config.window_width = 150;
>> tests/spec/arb_es2_compatibility/arb_es2_compatibility-depthrangef.c:36:        config.window_width = 150;
>> tests/texturing/incomplete-texture.c:38:        config.window_width = 150;
>> tests/texturing/shaders/textureSize.c:55:       config.window_width = 150;
>> tests/texturing/texsubimage.c:42:       config.window_width = 150;
>> tests/util/piglit-framework-gl.h:272:                config.window_width = 150;
>>
>> What is more, 	glViewport(0, 0, w, h)  will change the viewport to (w,h),  so in each glut display() function, glViewPort() should be recalled for validity
>


The last time I bumped up the default window size I had to fix a bunch 
of tests that had hard-coded dimensions.  Looks like there's a few more 
to tend to...

-Brian
The content of your original patch looks OK, but it should probably be 
split into two patches: one for the ktx change, and another for the 
viewport fix.

Can you please test changing the default window size to 160x160 and 
rerun on Win8 to verify?  I'll do the same here on Linux.

-Brian

On 04/29/2014 05:03 AM, Guo, Johney wrote:
> Bumping up to 160 is simple way to fix the "warn"s on Win8.
> For further compatibility, it is better to set the default window fix sized to disable resize function.
> BTW,Will my patch be accepted for this test?
>
> On 04/27/2014 12:45 PM, Ian Romanick wrote:
>> On 04/24/2014 03:41 AM, Guo, Johney wrote:
>>> See tests/util/piglit-framework-gl/piglit_glut_framework.c:84:
>>> default_reshape_func(int w, int h)
>>> {
>>> 	if (piglit_automatic &&
>>> 	    (w != piglit_width ||
>>> 	     h != piglit_height)) {
>>> 		printf("Got spurious window resize in automatic run "
>>> 		       "(%d,%d to %d,%d)\n", piglit_width, piglit_height, w, h);
>>> 		piglit_report_result(PIGLIT_WARN);
>>> 	}
>>>
>>> 	piglit_width = w;
>>> 	piglit_height = h;
>>>
>>> 	glViewport(0, 0, w, h);
>>> }
>>>
>>> My OS is win8. If piglit_width < 160, window manager will force it to 160.
>>
>> There was a similar issue a few years ago with Win7.  I believe Brian
>> Paul fixed this by modifying the framework to enforce a minimum window
>> size depending on the operating system.  It's better to fix this issue
>> once for all tests than change each individual test.  This is
>> especially true since we may add more tests with too-small window
>> sizes (since we're not on Windows).
>>
>> Brian, does that sound right?
>
> Basically, I changed the default window size to be 150x150 because on Windows (7 and maybe XP) if the window size was less than 116 pixels wide, it would get bumped up to 116 and that would cause the test to fail.
>
> I did a quick search of git history and found this one:
>
> commit 1894ef34c5e3cc0daefd1241b31a2b2ae32c3347
> Author: Brian Paul <brianp@vmware.com>
> Date:   Fri Apr 5 12:32:46 2013 -0600
>
>       lodbias: increase window size to fix test on Windows
>
>       Before, the 96x96 window got bumped up to 116x96 by Windows and we
>       saw a "Got spurious window resize in automatic run" message and the
>       test reported "warn".
>
> So the rule of thumb now is don't set the config.window_width, height vars unless you really have to, and be prepared for window resizes if you do.
>
> I've rarely tested Windows 8 so I wasn't aware of any problems there with the window size.  If width=160 is the new magic number, we may have to bump up config.window_width, height again.
>
> Johney, can you try bumping up the default window size in tests/util/piglit-framework-gl.h and see if that solve the problem?
>
> I can likewise do that here and see if there's any unexpected regressions from changing the window size.
>
>
>>
>>> Many piglit tests have set configure width as 150,  then they will all report as "warn".
>>> At least the following,
>>> $ grep -Irne "\<150\>" tests | grep width
>>> tests/fbo/fbo-blit-d24s8.c:45:  config.window_width = 150;
>>> tests/fbo/fbo-blit.c:43:        config.window_width = 150;
>>> tests/fbo/fbo-copypix.c:40:     config.window_width = 150;
>>> tests/fbo/fbo-readdrawpix.c:40: config.window_width = 150;
>>> tests/spec/arb_es2_compatibility/arb_es2_compatibility-depthrangef.c:36:        config.window_width = 150;
>>> tests/texturing/incomplete-texture.c:38:        config.window_width = 150;
>>> tests/texturing/shaders/textureSize.c:55:       config.window_width = 150;
>>> tests/texturing/texsubimage.c:42:       config.window_width = 150;
>>> tests/util/piglit-framework-gl.h:272:                config.window_width = 150;
>>>
>>> What is more, 	glViewport(0, 0, w, h)  will change the viewport to (w,h),  so in each glut display() function, glViewPort() should be recalled for validity
>>
>
>
> The last time I bumped up the default window size I had to fix a bunch
> of tests that had hard-coded dimensions.  Looks like there's a few more
> to tend to...
>
> -Brian
>
160x160 on win8 is OK. The test passes without "spurious window" warning.
And for the ktx and viewport fix, you will change and commit it directly, right?

Thanks
------
The content of your original patch looks OK, but it should probably be split into two patches: one for the ktx change, and another for the viewport fix.

Can you please test changing the default window size to 160x160 and rerun on Win8 to verify?  I'll do the same here on Linux.

-Brian

On 04/29/2014 05:03 AM, Guo, Johney wrote:
> Bumping up to 160 is simple way to fix the "warn"s on Win8.
> For further compatibility, it is better to set the default window fix sized to disable resize function.
> BTW,Will my patch be accepted for this test?
>
> On 04/27/2014 12:45 PM, Ian Romanick wrote:
>> On 04/24/2014 03:41 AM, Guo, Johney wrote:
>>> See tests/util/piglit-framework-gl/piglit_glut_framework.c:84:
>>> default_reshape_func(int w, int h)
>>> {
>>> 	if (piglit_automatic &&
>>> 	    (w != piglit_width ||
>>> 	     h != piglit_height)) {
>>> 		printf("Got spurious window resize in automatic run "
>>> 		       "(%d,%d to %d,%d)\n", piglit_width, piglit_height, w, h);
>>> 		piglit_report_result(PIGLIT_WARN);
>>> 	}
>>>
>>> 	piglit_width = w;
>>> 	piglit_height = h;
>>>
>>> 	glViewport(0, 0, w, h);
>>> }
>>>
>>> My OS is win8. If piglit_width < 160, window manager will force it to 160.
>>
>> There was a similar issue a few years ago with Win7.  I believe Brian 
>> Paul fixed this by modifying the framework to enforce a minimum 
>> window size depending on the operating system.  It's better to fix 
>> this issue once for all tests than change each individual test.  This 
>> is especially true since we may add more tests with too-small window 
>> sizes (since we're not on Windows).
>>
>> Brian, does that sound right?
>
> Basically, I changed the default window size to be 150x150 because on Windows (7 and maybe XP) if the window size was less than 116 pixels wide, it would get bumped up to 116 and that would cause the test to fail.
>
> I did a quick search of git history and found this one:
>
> commit 1894ef34c5e3cc0daefd1241b31a2b2ae32c3347
> Author: Brian Paul <brianp@vmware.com>
> Date:   Fri Apr 5 12:32:46 2013 -0600
>
>       lodbias: increase window size to fix test on Windows
>
>       Before, the 96x96 window got bumped up to 116x96 by Windows and we
>       saw a "Got spurious window resize in automatic run" message and the
>       test reported "warn".
>
> So the rule of thumb now is don't set the config.window_width, height vars unless you really have to, and be prepared for window resizes if you do.
>
> I've rarely tested Windows 8 so I wasn't aware of any problems there with the window size.  If width=160 is the new magic number, we may have to bump up config.window_width, height again.
>
> Johney, can you try bumping up the default window size in tests/util/piglit-framework-gl.h and see if that solve the problem?
>
> I can likewise do that here and see if there's any unexpected regressions from changing the window size.
>
>
>>
>>> Many piglit tests have set configure width as 150,  then they will all report as "warn".
>>> At least the following,
>>> $ grep -Irne "\<150\>" tests | grep width
>>> tests/fbo/fbo-blit-d24s8.c:45:  config.window_width = 150;
>>> tests/fbo/fbo-blit.c:43:        config.window_width = 150;
>>> tests/fbo/fbo-copypix.c:40:     config.window_width = 150;
>>> tests/fbo/fbo-readdrawpix.c:40: config.window_width = 150;
>>> tests/spec/arb_es2_compatibility/arb_es2_compatibility-depthrangef.c:36:        config.window_width = 150;
>>> tests/texturing/incomplete-texture.c:38:        config.window_width = 150;
>>> tests/texturing/shaders/textureSize.c:55:       config.window_width = 150;
>>> tests/texturing/texsubimage.c:42:       config.window_width = 150;
>>> tests/util/piglit-framework-gl.h:272:                config.window_width = 150;
>>>
>>> What is more, 	glViewport(0, 0, w, h)  will change the viewport to (w,h),  so in each glut display() function, glViewPort() should be recalled for validity
>>
>
>
> The last time I bumped up the default window size I had to fix a bunch 
> of tests that had hard-coded dimensions.  Looks like there's a few 
> more to tend to...
>
> -Brian
>
I've pushed the ktx binary mode change.

I still need to do a full piglit run with the 160x160 default window 
size change so I didn't push the change to 
oes_compressed_etc2_texture-miptree.c yet.

-Brian

On 04/29/2014 09:32 PM, Guo, Johney wrote:
> 160x160 on win8 is OK. The test passes without "spurious window" warning.
> And for the ktx and viewport fix, you will change and commit it directly, right?
>
> Thanks
> ------
> The content of your original patch looks OK, but it should probably be split into two patches: one for the ktx change, and another for the viewport fix.
>
> Can you please test changing the default window size to 160x160 and rerun on Win8 to verify?  I'll do the same here on Linux.
>
> -Brian
>
> On 04/29/2014 05:03 AM, Guo, Johney wrote:
>> Bumping up to 160 is simple way to fix the "warn"s on Win8.
>> For further compatibility, it is better to set the default window fix sized to disable resize function.
>> BTW,Will my patch be accepted for this test?
>>
>> On 04/27/2014 12:45 PM, Ian Romanick wrote:
>>> On 04/24/2014 03:41 AM, Guo, Johney wrote:
>>>> See tests/util/piglit-framework-gl/piglit_glut_framework.c:84:
>>>> default_reshape_func(int w, int h)
>>>> {
>>>> 	if (piglit_automatic &&
>>>> 	    (w != piglit_width ||
>>>> 	     h != piglit_height)) {
>>>> 		printf("Got spurious window resize in automatic run "
>>>> 		       "(%d,%d to %d,%d)\n", piglit_width, piglit_height, w, h);
>>>> 		piglit_report_result(PIGLIT_WARN);
>>>> 	}
>>>>
>>>> 	piglit_width = w;
>>>> 	piglit_height = h;
>>>>
>>>> 	glViewport(0, 0, w, h);
>>>> }
>>>>
>>>> My OS is win8. If piglit_width < 160, window manager will force it to 160.
>>>
>>> There was a similar issue a few years ago with Win7.  I believe Brian
>>> Paul fixed this by modifying the framework to enforce a minimum
>>> window size depending on the operating system.  It's better to fix
>>> this issue once for all tests than change each individual test.  This
>>> is especially true since we may add more tests with too-small window
>>> sizes (since we're not on Windows).
>>>
>>> Brian, does that sound right?
>>
>> Basically, I changed the default window size to be 150x150 because on Windows (7 and maybe XP) if the window size was less than 116 pixels wide, it would get bumped up to 116 and that would cause the test to fail.
>>
>> I did a quick search of git history and found this one:
>>
>> commit 1894ef34c5e3cc0daefd1241b31a2b2ae32c3347
>> Author: Brian Paul <brianp@vmware.com>
>> Date:   Fri Apr 5 12:32:46 2013 -0600
>>
>>        lodbias: increase window size to fix test on Windows
>>
>>        Before, the 96x96 window got bumped up to 116x96 by Windows and we
>>        saw a "Got spurious window resize in automatic run" message and the
>>        test reported "warn".
>>
>> So the rule of thumb now is don't set the config.window_width, height vars unless you really have to, and be prepared for window resizes if you do.
>>
>> I've rarely tested Windows 8 so I wasn't aware of any problems there with the window size.  If width=160 is the new magic number, we may have to bump up config.window_width, height again.
>>
>> Johney, can you try bumping up the default window size in tests/util/piglit-framework-gl.h and see if that solve the problem?
>>
>> I can likewise do that here and see if there's any unexpected regressions from changing the window size.
>>
>>
>>>
>>>> Many piglit tests have set configure width as 150,  then they will all report as "warn".
>>>> At least the following,
>>>> $ grep -Irne "\<150\>" tests | grep width
>>>> tests/fbo/fbo-blit-d24s8.c:45:  config.window_width = 150;
>>>> tests/fbo/fbo-blit.c:43:        config.window_width = 150;
>>>> tests/fbo/fbo-copypix.c:40:     config.window_width = 150;
>>>> tests/fbo/fbo-readdrawpix.c:40: config.window_width = 150;
>>>> tests/spec/arb_es2_compatibility/arb_es2_compatibility-depthrangef.c:36:        config.window_width = 150;
>>>> tests/texturing/incomplete-texture.c:38:        config.window_width = 150;
>>>> tests/texturing/shaders/textureSize.c:55:       config.window_width = 150;
>>>> tests/texturing/texsubimage.c:42:       config.window_width = 150;
>>>> tests/util/piglit-framework-gl.h:272:                config.window_width = 150;
>>>>
>>>> What is more, 	glViewport(0, 0, w, h)  will change the viewport to (w,h),  so in each glut display() function, glViewPort() should be recalled for validity
>>>
>>
>>
>> The last time I bumped up the default window size I had to fix a bunch
>> of tests that had hard-coded dimensions.  Looks like there's a few
>> more to tend to...
>>
>> -Brian
>>
>