winsys-framework Default to showing window

Submitted by Alex Goins on Aug. 13, 2015, 5:49 p.m.

Details

Message ID 1439488169-10276-2-git-send-email-agoins@nvidia.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Alex Goins Aug. 13, 2015, 5:49 p.m.
winsys-framework incorrectly assumed that it will always have ownership of the
pixels in its own buffers. If using the default framebuffer (i.e., when Piglit
is not running in FBO mode,) ownership will not be acquired unless the window is
mapped.

While this is not typically a problem because buffers are separate, NVIDIA's
Unified Back Buffer feature makes all windows share a back buffer while still
conforming to the OpenGL spec. If Piglit does not establish pixel ownership, its
output will be clobbered by other windows.

While this problem could be easily fixed by specifying PIGLIT_FORCE_WINDOW=1 or
-fbo, the current default takes advantage of undefined behavior in the OpenGL
spec. A better solution would be to replace PIGLIT_FORCE_WINDOW with
PIGLIT_NO_WINDOW, a flag that allows tests to be run without mapping a window or
using an FBO. The default behavior, the, would be to map a window. Really,
though, if users want to test with offscreen rendering, they should use FBOs
with flag -fbo.

More information on pixel ownership here:
https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node94.html
---
 .../piglit-framework-gl/piglit_winsys_framework.c    | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
index c80e972..fcba91e 100644
--- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
+++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
@@ -53,18 +53,18 @@  run_test(struct piglit_gl_framework *gl_fw,
          int argc, char *argv[])
 {
 	struct piglit_winsys_framework *winsys_fw = piglit_winsys_framework(gl_fw);
-	bool force_window = false;
-	const char *env_force_window = getenv("PIGLIT_FORCE_WINDOW");
+	bool no_window = false;
+	const char *env_no_window = getenv("PIGLIT_NO_WINDOW");
 
 
-	if (env_force_window != NULL) {
-		if (strcmp(env_force_window, "0") == 0) {
-			force_window = false;
-		} else if (strcmp(env_force_window, "1") == 0) {
-			force_window = true;
+	if (env_no_window != NULL) {
+		if (strcmp(env_no_window, "0") == 0) {
+			no_window = true;
+		} else if (strcmp(env_no_window, "1") == 0) {
+			no_window = true;
 		} else {
-			fprintf(stderr, "PIGLIT_FORCE_WINDOW has invalid"
-				" value: %s\n", env_force_window);
+			fprintf(stderr, "PIGLIT_NO_WINDOW has invalid"
+				" value: %s\n", env_no_window);
 			abort();
 		}
 	}
@@ -73,7 +73,7 @@  run_test(struct piglit_gl_framework *gl_fw,
 		gl_fw->test_config->init(argc, argv);
 
 	if (!gl_fw->test_config->requires_displayed_window &&
-	    piglit_automatic && !force_window) {
+	    piglit_automatic && no_window) {
 		enum piglit_result result = PIGLIT_PASS;
 		if (gl_fw->test_config->display)
 			result = gl_fw->test_config->display();

Comments

Interesting. PIGLIT_FORCE_WINDOW was introduced in this change:

commit b797406fb850c5e3c9fdaaebae36127b5e5f8954
Author: Paul Berry <stereotype441@gmail.com>
Date:   Tue Jun 11 16:34:32 2013 -0700

    Add PIGLIT_FORCE_WINDOW environment variable.

    Setting PIGLIT_FORCE_WINDOW=1 causes all piglit windowsystem tests
    (those that weren't invoked with the "-fbo" flag) to show the window
    prior to calling piglit_display().  Setting PIGLIT_FORCE_WINDOW=0 (the
    default) preserves piglit's existing behaviour, which is to only show
    the window when the test is run in "manual" mode (with no "-auto"
    argument supplied).

    Setting PIGLIT_FORCE_WINDOW=1 slows down the piglit run, and produces
    more annoying windows on the screen, but it has been observed to
    produce more repeatable results when testing with the nVidia
    proprietary driver for Linux.

    Reviewed-by: Chad Versace <chad.versace@linux.intel.com>

Which explicitly mentions the nvidia blob driver thing. Having more
windows popping up during piglit runs is going to be annoying... and
slowing things down will also be unwelcome. Is there anything else
that can be done here?

  -ilia

On Thu, Aug 13, 2015 at 1:49 PM, agoins <agoins@nvidia.com> wrote:
> winsys-framework incorrectly assumed that it will always have ownership of the
> pixels in its own buffers. If using the default framebuffer (i.e., when Piglit
> is not running in FBO mode,) ownership will not be acquired unless the window is
> mapped.
>
> While this is not typically a problem because buffers are separate, NVIDIA's
> Unified Back Buffer feature makes all windows share a back buffer while still
> conforming to the OpenGL spec. If Piglit does not establish pixel ownership, its
> output will be clobbered by other windows.
>
> While this problem could be easily fixed by specifying PIGLIT_FORCE_WINDOW=1 or
> -fbo, the current default takes advantage of undefined behavior in the OpenGL
> spec. A better solution would be to replace PIGLIT_FORCE_WINDOW with
> PIGLIT_NO_WINDOW, a flag that allows tests to be run without mapping a window or
> using an FBO. The default behavior, the, would be to map a window. Really,
> though, if users want to test with offscreen rendering, they should use FBOs
> with flag -fbo.
>
> More information on pixel ownership here:
> https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node94.html
> ---
>  .../piglit-framework-gl/piglit_winsys_framework.c    | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> index c80e972..fcba91e 100644
> --- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> +++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> @@ -53,18 +53,18 @@ run_test(struct piglit_gl_framework *gl_fw,
>           int argc, char *argv[])
>  {
>         struct piglit_winsys_framework *winsys_fw = piglit_winsys_framework(gl_fw);
> -       bool force_window = false;
> -       const char *env_force_window = getenv("PIGLIT_FORCE_WINDOW");
> +       bool no_window = false;
> +       const char *env_no_window = getenv("PIGLIT_NO_WINDOW");
>
>
> -       if (env_force_window != NULL) {
> -               if (strcmp(env_force_window, "0") == 0) {
> -                       force_window = false;
> -               } else if (strcmp(env_force_window, "1") == 0) {
> -                       force_window = true;
> +       if (env_no_window != NULL) {
> +               if (strcmp(env_no_window, "0") == 0) {
> +                       no_window = true;
> +               } else if (strcmp(env_no_window, "1") == 0) {
> +                       no_window = true;
>                 } else {
> -                       fprintf(stderr, "PIGLIT_FORCE_WINDOW has invalid"
> -                               " value: %s\n", env_force_window);
> +                       fprintf(stderr, "PIGLIT_NO_WINDOW has invalid"
> +                               " value: %s\n", env_no_window);
>                         abort();
>                 }
>         }
> @@ -73,7 +73,7 @@ run_test(struct piglit_gl_framework *gl_fw,
>                 gl_fw->test_config->init(argc, argv);
>
>         if (!gl_fw->test_config->requires_displayed_window &&
> -           piglit_automatic && !force_window) {
> +           piglit_automatic && no_window) {
>                 enum piglit_result result = PIGLIT_PASS;
>                 if (gl_fw->test_config->display)
>                         result = gl_fw->test_config->display();
> --
> 1.9.1
> New to the project - please commit for me.
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
From NVIDIA's side, I don't think it would be that big of a problem to just keep running our tests with PIGLIT_FORCE_WINDOW=1 or -fbo. However, my reasoning is that Piglit should default to the 'correct' behavior according to the OpenGL spec. While the NVIDIA driver is the only one I know of that actually enforces pixel ownership, any driver theoretically could do so while still adhering to the spec.

I agree that having a window pop up for every test is annoying, but using the default framebuffer without mapping it is undefined behavior. The 'correct' way to do offscreen rendering would be using FBOs which Piglit supports with the -fbo argument. Is there a reason why this can't/shouldn't be used for running Piglit offscreen? I originally removed the ability to render offscreen with the default framebuffer altogether, but I figured some users might still desire this behavior so instead decided to leave it in as a non-default option.

Thanks,
Alex

-----Original Message-----
From: ibmirkin@gmail.com [mailto:ibmirkin@gmail.com] On Behalf Of Ilia Mirkin

Sent: Friday, August 14, 2015 8:07 AM
To: Alexander Goins
Cc: piglit@lists.freedesktop.org
Subject: Re: [Piglit] [PATCH] winsys-framework Default to showing window

Interesting. PIGLIT_FORCE_WINDOW was introduced in this change:

commit b797406fb850c5e3c9fdaaebae36127b5e5f8954
Author: Paul Berry <stereotype441@gmail.com>
Date:   Tue Jun 11 16:34:32 2013 -0700

    Add PIGLIT_FORCE_WINDOW environment variable.

    Setting PIGLIT_FORCE_WINDOW=1 causes all piglit windowsystem tests
    (those that weren't invoked with the "-fbo" flag) to show the window
    prior to calling piglit_display().  Setting PIGLIT_FORCE_WINDOW=0 (the
    default) preserves piglit's existing behaviour, which is to only show
    the window when the test is run in "manual" mode (with no "-auto"
    argument supplied).

    Setting PIGLIT_FORCE_WINDOW=1 slows down the piglit run, and produces
    more annoying windows on the screen, but it has been observed to
    produce more repeatable results when testing with the nVidia
    proprietary driver for Linux.

    Reviewed-by: Chad Versace <chad.versace@linux.intel.com>


Which explicitly mentions the nvidia blob driver thing. Having more windows popping up during piglit runs is going to be annoying... and slowing things down will also be unwelcome. Is there anything else that can be done here?

  -ilia

On Thu, Aug 13, 2015 at 1:49 PM, agoins <agoins@nvidia.com> wrote:
> winsys-framework incorrectly assumed that it will always have 

> ownership of the pixels in its own buffers. If using the default 

> framebuffer (i.e., when Piglit is not running in FBO mode,) ownership 

> will not be acquired unless the window is mapped.

>

> While this is not typically a problem because buffers are separate, 

> NVIDIA's Unified Back Buffer feature makes all windows share a back 

> buffer while still conforming to the OpenGL spec. If Piglit does not 

> establish pixel ownership, its output will be clobbered by other windows.

>

> While this problem could be easily fixed by specifying 

> PIGLIT_FORCE_WINDOW=1 or -fbo, the current default takes advantage of 

> undefined behavior in the OpenGL spec. A better solution would be to 

> replace PIGLIT_FORCE_WINDOW with PIGLIT_NO_WINDOW, a flag that allows 

> tests to be run without mapping a window or using an FBO. The default 

> behavior, the, would be to map a window. Really, though, if users want 

> to test with offscreen rendering, they should use FBOs with flag -fbo.

>

> More information on pixel ownership here:

> https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node94

> .html

> ---

>  .../piglit-framework-gl/piglit_winsys_framework.c    | 20 ++++++++++----------

>  1 file changed, 10 insertions(+), 10 deletions(-)

>

> diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c 

> b/tests/util/piglit-framework-gl/piglit_winsys_framework.c

> index c80e972..fcba91e 100644

> --- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c

> +++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c

> @@ -53,18 +53,18 @@ run_test(struct piglit_gl_framework *gl_fw,

>           int argc, char *argv[])

>  {

>         struct piglit_winsys_framework *winsys_fw = piglit_winsys_framework(gl_fw);

> -       bool force_window = false;

> -       const char *env_force_window = getenv("PIGLIT_FORCE_WINDOW");

> +       bool no_window = false;

> +       const char *env_no_window = getenv("PIGLIT_NO_WINDOW");

>

>

> -       if (env_force_window != NULL) {

> -               if (strcmp(env_force_window, "0") == 0) {

> -                       force_window = false;

> -               } else if (strcmp(env_force_window, "1") == 0) {

> -                       force_window = true;

> +       if (env_no_window != NULL) {

> +               if (strcmp(env_no_window, "0") == 0) {

> +                       no_window = true;

> +               } else if (strcmp(env_no_window, "1") == 0) {

> +                       no_window = true;

>                 } else {

> -                       fprintf(stderr, "PIGLIT_FORCE_WINDOW has invalid"

> -                               " value: %s\n", env_force_window);

> +                       fprintf(stderr, "PIGLIT_NO_WINDOW has invalid"

> +                               " value: %s\n", env_no_window);

>                         abort();

>                 }

>         }

> @@ -73,7 +73,7 @@ run_test(struct piglit_gl_framework *gl_fw,

>                 gl_fw->test_config->init(argc, argv);

>

>         if (!gl_fw->test_config->requires_displayed_window &&

> -           piglit_automatic && !force_window) {

> +           piglit_automatic && no_window) {

>                 enum piglit_result result = PIGLIT_PASS;

>                 if (gl_fw->test_config->display)

>                         result = gl_fw->test_config->display();

> --

> 1.9.1

> New to the project - please commit for me.

>

> _______________________________________________

> Piglit mailing list

> Piglit@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/piglit
Most tests do indeed use an FBO, either directly or via the fbo
winsys, and all will be well. However some don't (glean, probably
others) because they were written in the stone ages. Those are the
ones that have problems.

Perhaps we should just suck up the slower-ness and use that as an
incentive to finally convert those tests over to The New Way (tm).

Unfortunately I'm ill-equipped to either agree or disagree with your
claim that "using the default framebuffer without mapping it is
undefined behavior". Hopefully someone "in the know" can comment?

  -ilia


On Fri, Aug 14, 2015 at 7:27 PM, Alexander Goins <agoins@nvidia.com> wrote:
> From NVIDIA's side, I don't think it would be that big of a problem to just keep running our tests with PIGLIT_FORCE_WINDOW=1 or -fbo. However, my reasoning is that Piglit should default to the 'correct' behavior according to the OpenGL spec. While the NVIDIA driver is the only one I know of that actually enforces pixel ownership, any driver theoretically could do so while still adhering to the spec.
>
> I agree that having a window pop up for every test is annoying, but using the default framebuffer without mapping it is undefined behavior. The 'correct' way to do offscreen rendering would be using FBOs which Piglit supports with the -fbo argument. Is there a reason why this can't/shouldn't be used for running Piglit offscreen? I originally removed the ability to render offscreen with the default framebuffer altogether, but I figured some users might still desire this behavior so instead decided to leave it in as a non-default option.
>
> Thanks,
> Alex
>
> -----Original Message-----
> From: ibmirkin@gmail.com [mailto:ibmirkin@gmail.com] On Behalf Of Ilia Mirkin
> Sent: Friday, August 14, 2015 8:07 AM
> To: Alexander Goins
> Cc: piglit@lists.freedesktop.org
> Subject: Re: [Piglit] [PATCH] winsys-framework Default to showing window
>
> Interesting. PIGLIT_FORCE_WINDOW was introduced in this change:
>
> commit b797406fb850c5e3c9fdaaebae36127b5e5f8954
> Author: Paul Berry <stereotype441@gmail.com>
> Date:   Tue Jun 11 16:34:32 2013 -0700
>
>     Add PIGLIT_FORCE_WINDOW environment variable.
>
>     Setting PIGLIT_FORCE_WINDOW=1 causes all piglit windowsystem tests
>     (those that weren't invoked with the "-fbo" flag) to show the window
>     prior to calling piglit_display().  Setting PIGLIT_FORCE_WINDOW=0 (the
>     default) preserves piglit's existing behaviour, which is to only show
>     the window when the test is run in "manual" mode (with no "-auto"
>     argument supplied).
>
>     Setting PIGLIT_FORCE_WINDOW=1 slows down the piglit run, and produces
>     more annoying windows on the screen, but it has been observed to
>     produce more repeatable results when testing with the nVidia
>     proprietary driver for Linux.
>
>     Reviewed-by: Chad Versace <chad.versace@linux.intel.com>
>
> Which explicitly mentions the nvidia blob driver thing. Having more windows popping up during piglit runs is going to be annoying... and slowing things down will also be unwelcome. Is there anything else that can be done here?
>
>   -ilia
>
> On Thu, Aug 13, 2015 at 1:49 PM, agoins <agoins@nvidia.com> wrote:
>> winsys-framework incorrectly assumed that it will always have
>> ownership of the pixels in its own buffers. If using the default
>> framebuffer (i.e., when Piglit is not running in FBO mode,) ownership
>> will not be acquired unless the window is mapped.
>>
>> While this is not typically a problem because buffers are separate,
>> NVIDIA's Unified Back Buffer feature makes all windows share a back
>> buffer while still conforming to the OpenGL spec. If Piglit does not
>> establish pixel ownership, its output will be clobbered by other windows.
>>
>> While this problem could be easily fixed by specifying
>> PIGLIT_FORCE_WINDOW=1 or -fbo, the current default takes advantage of
>> undefined behavior in the OpenGL spec. A better solution would be to
>> replace PIGLIT_FORCE_WINDOW with PIGLIT_NO_WINDOW, a flag that allows
>> tests to be run without mapping a window or using an FBO. The default
>> behavior, the, would be to map a window. Really, though, if users want
>> to test with offscreen rendering, they should use FBOs with flag -fbo.
>>
>> More information on pixel ownership here:
>> https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node94
>> .html
>> ---
>>  .../piglit-framework-gl/piglit_winsys_framework.c    | 20 ++++++++++----------
>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> index c80e972..fcba91e 100644
>> --- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> +++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> @@ -53,18 +53,18 @@ run_test(struct piglit_gl_framework *gl_fw,
>>           int argc, char *argv[])
>>  {
>>         struct piglit_winsys_framework *winsys_fw = piglit_winsys_framework(gl_fw);
>> -       bool force_window = false;
>> -       const char *env_force_window = getenv("PIGLIT_FORCE_WINDOW");
>> +       bool no_window = false;
>> +       const char *env_no_window = getenv("PIGLIT_NO_WINDOW");
>>
>>
>> -       if (env_force_window != NULL) {
>> -               if (strcmp(env_force_window, "0") == 0) {
>> -                       force_window = false;
>> -               } else if (strcmp(env_force_window, "1") == 0) {
>> -                       force_window = true;
>> +       if (env_no_window != NULL) {
>> +               if (strcmp(env_no_window, "0") == 0) {
>> +                       no_window = true;
>> +               } else if (strcmp(env_no_window, "1") == 0) {
>> +                       no_window = true;
>>                 } else {
>> -                       fprintf(stderr, "PIGLIT_FORCE_WINDOW has invalid"
>> -                               " value: %s\n", env_force_window);
>> +                       fprintf(stderr, "PIGLIT_NO_WINDOW has invalid"
>> +                               " value: %s\n", env_no_window);
>>                         abort();
>>                 }
>>         }
>> @@ -73,7 +73,7 @@ run_test(struct piglit_gl_framework *gl_fw,
>>                 gl_fw->test_config->init(argc, argv);
>>
>>         if (!gl_fw->test_config->requires_displayed_window &&
>> -           piglit_automatic && !force_window) {
>> +           piglit_automatic && no_window) {
>>                 enum piglit_result result = PIGLIT_PASS;
>>                 if (gl_fw->test_config->display)
>>                         result = gl_fw->test_config->display();
>> --
>> 1.9.1
>> New to the project - please commit for me.
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
For those who don't want to click-through, I'll just paste the very 
brief portion of the spec on Pixel Ownership that Alex linked to here:

[1] "The first test is to determine if the pixel at location in the 
framebuffer is currently owned by the GL (more precisely, by this GL 
context). If it is not, the window system decides the fate the incoming 
fragment. Possible results are that the fragment is discarded or that 
some subset of the subsequent per-fragment operations are applied to the 
fragment. This test allows the window system to control the GL's 
behavior, for instance, when a GL window is obscured."

The important part is the last sentence: The GL does not guarantee 
rendering to an obscured pixel will do anything.  Unmapped windows are 
definitely obscured pixels.  I've never heard anyone argue against that 
interpretation, though I've heard some grumbling from people that wish 
this spec language didn't exist.  As long as it exists though, we'll 
take advantage of it in our driver, so I don't think it makes any sense 
for tests to ignore it, at least by default.  Any other driver is free 
to implement the same optimization, but I don't know if any other X11 GL 
drivers do at the moment.

Thanks,
-James

[1] 
https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node94.html

(Also present in the OpenGL 4.5 core spec as section 14.9.1, page 461)
https://www.opengl.org/registry/doc/glspec45.core.pdf

On 08/14/2015 04:33 PM, Ilia Mirkin wrote:
> Most tests do indeed use an FBO, either directly or via the fbo
> winsys, and all will be well. However some don't (glean, probably
> others) because they were written in the stone ages. Those are the
> ones that have problems.
>
> Perhaps we should just suck up the slower-ness and use that as an
> incentive to finally convert those tests over to The New Way (tm).
>
> Unfortunately I'm ill-equipped to either agree or disagree with your
> claim that "using the default framebuffer without mapping it is
> undefined behavior". Hopefully someone "in the know" can comment?
>
>    -ilia
>
>
> On Fri, Aug 14, 2015 at 7:27 PM, Alexander Goins <agoins@nvidia.com> wrote:
>>  From NVIDIA's side, I don't think it would be that big of a problem to just keep running our tests with PIGLIT_FORCE_WINDOW=1 or -fbo. However, my reasoning is that Piglit should default to the 'correct' behavior according to the OpenGL spec. While the NVIDIA driver is the only one I know of that actually enforces pixel ownership, any driver theoretically could do so while still adhering to the spec.
>>
>> I agree that having a window pop up for every test is annoying, but using the default framebuffer without mapping it is undefined behavior. The 'correct' way to do offscreen rendering would be using FBOs which Piglit supports with the -fbo argument. Is there a reason why this can't/shouldn't be used for running Piglit offscreen? I originally removed the ability to render offscreen with the default framebuffer altogether, but I figured some users might still desire this behavior so instead decided to leave it in as a non-default option.
>>
>> Thanks,
>> Alex
>>
>> -----Original Message-----
>> From: ibmirkin@gmail.com [mailto:ibmirkin@gmail.com] On Behalf Of Ilia Mirkin
>> Sent: Friday, August 14, 2015 8:07 AM
>> To: Alexander Goins
>> Cc: piglit@lists.freedesktop.org
>> Subject: Re: [Piglit] [PATCH] winsys-framework Default to showing window
>>
>> Interesting. PIGLIT_FORCE_WINDOW was introduced in this change:
>>
>> commit b797406fb850c5e3c9fdaaebae36127b5e5f8954
>> Author: Paul Berry <stereotype441@gmail.com>
>> Date:   Tue Jun 11 16:34:32 2013 -0700
>>
>>      Add PIGLIT_FORCE_WINDOW environment variable.
>>
>>      Setting PIGLIT_FORCE_WINDOW=1 causes all piglit windowsystem tests
>>      (those that weren't invoked with the "-fbo" flag) to show the window
>>      prior to calling piglit_display().  Setting PIGLIT_FORCE_WINDOW=0 (the
>>      default) preserves piglit's existing behaviour, which is to only show
>>      the window when the test is run in "manual" mode (with no "-auto"
>>      argument supplied).
>>
>>      Setting PIGLIT_FORCE_WINDOW=1 slows down the piglit run, and produces
>>      more annoying windows on the screen, but it has been observed to
>>      produce more repeatable results when testing with the nVidia
>>      proprietary driver for Linux.
>>
>>      Reviewed-by: Chad Versace <chad.versace@linux.intel.com>
>>
>> Which explicitly mentions the nvidia blob driver thing. Having more windows popping up during piglit runs is going to be annoying... and slowing things down will also be unwelcome. Is there anything else that can be done here?
>>
>>    -ilia
>>
>> On Thu, Aug 13, 2015 at 1:49 PM, agoins <agoins@nvidia.com> wrote:
>>> winsys-framework incorrectly assumed that it will always have
>>> ownership of the pixels in its own buffers. If using the default
>>> framebuffer (i.e., when Piglit is not running in FBO mode,) ownership
>>> will not be acquired unless the window is mapped.
>>>
>>> While this is not typically a problem because buffers are separate,
>>> NVIDIA's Unified Back Buffer feature makes all windows share a back
>>> buffer while still conforming to the OpenGL spec. If Piglit does not
>>> establish pixel ownership, its output will be clobbered by other windows.
>>>
>>> While this problem could be easily fixed by specifying
>>> PIGLIT_FORCE_WINDOW=1 or -fbo, the current default takes advantage of
>>> undefined behavior in the OpenGL spec. A better solution would be to
>>> replace PIGLIT_FORCE_WINDOW with PIGLIT_NO_WINDOW, a flag that allows
>>> tests to be run without mapping a window or using an FBO. The default
>>> behavior, the, would be to map a window. Really, though, if users want
>>> to test with offscreen rendering, they should use FBOs with flag -fbo.
>>>
>>> More information on pixel ownership here:
>>> https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node94
>>> .html
>>> ---
>>>   .../piglit-framework-gl/piglit_winsys_framework.c    | 20 ++++++++++----------
>>>   1 file changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>>> b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>>> index c80e972..fcba91e 100644
>>> --- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>>> +++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>>> @@ -53,18 +53,18 @@ run_test(struct piglit_gl_framework *gl_fw,
>>>            int argc, char *argv[])
>>>   {
>>>          struct piglit_winsys_framework *winsys_fw = piglit_winsys_framework(gl_fw);
>>> -       bool force_window = false;
>>> -       const char *env_force_window = getenv("PIGLIT_FORCE_WINDOW");
>>> +       bool no_window = false;
>>> +       const char *env_no_window = getenv("PIGLIT_NO_WINDOW");
>>>
>>>
>>> -       if (env_force_window != NULL) {
>>> -               if (strcmp(env_force_window, "0") == 0) {
>>> -                       force_window = false;
>>> -               } else if (strcmp(env_force_window, "1") == 0) {
>>> -                       force_window = true;
>>> +       if (env_no_window != NULL) {
>>> +               if (strcmp(env_no_window, "0") == 0) {
>>> +                       no_window = true;
>>> +               } else if (strcmp(env_no_window, "1") == 0) {
>>> +                       no_window = true;
>>>                  } else {
>>> -                       fprintf(stderr, "PIGLIT_FORCE_WINDOW has invalid"
>>> -                               " value: %s\n", env_force_window);
>>> +                       fprintf(stderr, "PIGLIT_NO_WINDOW has invalid"
>>> +                               " value: %s\n", env_no_window);
>>>                          abort();
>>>                  }
>>>          }
>>> @@ -73,7 +73,7 @@ run_test(struct piglit_gl_framework *gl_fw,
>>>                  gl_fw->test_config->init(argc, argv);
>>>
>>>          if (!gl_fw->test_config->requires_displayed_window &&
>>> -           piglit_automatic && !force_window) {
>>> +           piglit_automatic && no_window) {
>>>                  enum piglit_result result = PIGLIT_PASS;
>>>                  if (gl_fw->test_config->display)
>>>                          result = gl_fw->test_config->display();
>>> --
>>> 1.9.1
>>> New to the project - please commit for me.
>>>
>>> _______________________________________________
>>> Piglit mailing list
>>> Piglit@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/piglit
>>
>> -----------------------------------------------------------------------------------
>> This email message is for the sole use of the intended recipient(s) and may contain
>> confidential information.  Any unauthorized review, use, disclosure or distribution
>> is prohibited.  If you are not the intended recipient, please contact the sender by
>> reply email and destroy all copies of the original message.
>> -----------------------------------------------------------------------------------
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
Hi Alex,

The tests which don't specifically test the behavior of the default
framebuffer should use -fbo. I think this is done by marking a test as
"concurrent" in all.py. A lot of tests don't do that, because nobody
had the time to update all.py and check if it doesn't break them and
if it's okay to do that with regard to the purpose of the tests. (i.e.
do they specifically test the default framebuffer or something else?)

Marek


On Sat, Aug 15, 2015 at 1:27 AM, Alexander Goins <agoins@nvidia.com> wrote:
> From NVIDIA's side, I don't think it would be that big of a problem to just keep running our tests with PIGLIT_FORCE_WINDOW=1 or -fbo. However, my reasoning is that Piglit should default to the 'correct' behavior according to the OpenGL spec. While the NVIDIA driver is the only one I know of that actually enforces pixel ownership, any driver theoretically could do so while still adhering to the spec.
>
> I agree that having a window pop up for every test is annoying, but using the default framebuffer without mapping it is undefined behavior. The 'correct' way to do offscreen rendering would be using FBOs which Piglit supports with the -fbo argument. Is there a reason why this can't/shouldn't be used for running Piglit offscreen? I originally removed the ability to render offscreen with the default framebuffer altogether, but I figured some users might still desire this behavior so instead decided to leave it in as a non-default option.
>
> Thanks,
> Alex
>
> -----Original Message-----
> From: ibmirkin@gmail.com [mailto:ibmirkin@gmail.com] On Behalf Of Ilia Mirkin
> Sent: Friday, August 14, 2015 8:07 AM
> To: Alexander Goins
> Cc: piglit@lists.freedesktop.org
> Subject: Re: [Piglit] [PATCH] winsys-framework Default to showing window
>
> Interesting. PIGLIT_FORCE_WINDOW was introduced in this change:
>
> commit b797406fb850c5e3c9fdaaebae36127b5e5f8954
> Author: Paul Berry <stereotype441@gmail.com>
> Date:   Tue Jun 11 16:34:32 2013 -0700
>
>     Add PIGLIT_FORCE_WINDOW environment variable.
>
>     Setting PIGLIT_FORCE_WINDOW=1 causes all piglit windowsystem tests
>     (those that weren't invoked with the "-fbo" flag) to show the window
>     prior to calling piglit_display().  Setting PIGLIT_FORCE_WINDOW=0 (the
>     default) preserves piglit's existing behaviour, which is to only show
>     the window when the test is run in "manual" mode (with no "-auto"
>     argument supplied).
>
>     Setting PIGLIT_FORCE_WINDOW=1 slows down the piglit run, and produces
>     more annoying windows on the screen, but it has been observed to
>     produce more repeatable results when testing with the nVidia
>     proprietary driver for Linux.
>
>     Reviewed-by: Chad Versace <chad.versace@linux.intel.com>
>
> Which explicitly mentions the nvidia blob driver thing. Having more windows popping up during piglit runs is going to be annoying... and slowing things down will also be unwelcome. Is there anything else that can be done here?
>
>   -ilia
>
> On Thu, Aug 13, 2015 at 1:49 PM, agoins <agoins@nvidia.com> wrote:
>> winsys-framework incorrectly assumed that it will always have
>> ownership of the pixels in its own buffers. If using the default
>> framebuffer (i.e., when Piglit is not running in FBO mode,) ownership
>> will not be acquired unless the window is mapped.
>>
>> While this is not typically a problem because buffers are separate,
>> NVIDIA's Unified Back Buffer feature makes all windows share a back
>> buffer while still conforming to the OpenGL spec. If Piglit does not
>> establish pixel ownership, its output will be clobbered by other windows.
>>
>> While this problem could be easily fixed by specifying
>> PIGLIT_FORCE_WINDOW=1 or -fbo, the current default takes advantage of
>> undefined behavior in the OpenGL spec. A better solution would be to
>> replace PIGLIT_FORCE_WINDOW with PIGLIT_NO_WINDOW, a flag that allows
>> tests to be run without mapping a window or using an FBO. The default
>> behavior, the, would be to map a window. Really, though, if users want
>> to test with offscreen rendering, they should use FBOs with flag -fbo.
>>
>> More information on pixel ownership here:
>> https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node94
>> .html
>> ---
>>  .../piglit-framework-gl/piglit_winsys_framework.c    | 20 ++++++++++----------
>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> index c80e972..fcba91e 100644
>> --- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> +++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
>> @@ -53,18 +53,18 @@ run_test(struct piglit_gl_framework *gl_fw,
>>           int argc, char *argv[])
>>  {
>>         struct piglit_winsys_framework *winsys_fw = piglit_winsys_framework(gl_fw);
>> -       bool force_window = false;
>> -       const char *env_force_window = getenv("PIGLIT_FORCE_WINDOW");
>> +       bool no_window = false;
>> +       const char *env_no_window = getenv("PIGLIT_NO_WINDOW");
>>
>>
>> -       if (env_force_window != NULL) {
>> -               if (strcmp(env_force_window, "0") == 0) {
>> -                       force_window = false;
>> -               } else if (strcmp(env_force_window, "1") == 0) {
>> -                       force_window = true;
>> +       if (env_no_window != NULL) {
>> +               if (strcmp(env_no_window, "0") == 0) {
>> +                       no_window = true;
>> +               } else if (strcmp(env_no_window, "1") == 0) {
>> +                       no_window = true;
>>                 } else {
>> -                       fprintf(stderr, "PIGLIT_FORCE_WINDOW has invalid"
>> -                               " value: %s\n", env_force_window);
>> +                       fprintf(stderr, "PIGLIT_NO_WINDOW has invalid"
>> +                               " value: %s\n", env_no_window);
>>                         abort();
>>                 }
>>         }
>> @@ -73,7 +73,7 @@ run_test(struct piglit_gl_framework *gl_fw,
>>                 gl_fw->test_config->init(argc, argv);
>>
>>         if (!gl_fw->test_config->requires_displayed_window &&
>> -           piglit_automatic && !force_window) {
>> +           piglit_automatic && no_window) {
>>                 enum piglit_result result = PIGLIT_PASS;
>>                 if (gl_fw->test_config->display)
>>                         result = gl_fw->test_config->display();
>> --
>> 1.9.1
>> New to the project - please commit for me.
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
Does anyone else want to review this before it goes in?

Thanks,
Alex

-----Original Message-----
From: Marek Olšák [mailto:maraeo@gmail.com] 

Sent: Friday, August 21, 2015 5:09 AM
To: Alexander Goins
Cc: Ilia Mirkin; piglit@lists.freedesktop.org
Subject: Re: [Piglit] [PATCH] winsys-framework Default to showing window

Hi Alex,

The tests which don't specifically test the behavior of the default framebuffer should use -fbo. I think this is done by marking a test as "concurrent" in all.py. A lot of tests don't do that, because nobody had the time to update all.py and check if it doesn't break them and if it's okay to do that with regard to the purpose of the tests. (i.e.
do they specifically test the default framebuffer or something else?)

Marek


On Sat, Aug 15, 2015 at 1:27 AM, Alexander Goins <agoins@nvidia.com> wrote:
> From NVIDIA's side, I don't think it would be that big of a problem to just keep running our tests with PIGLIT_FORCE_WINDOW=1 or -fbo. However, my reasoning is that Piglit should default to the 'correct' behavior according to the OpenGL spec. While the NVIDIA driver is the only one I know of that actually enforces pixel ownership, any driver theoretically could do so while still adhering to the spec.

>

> I agree that having a window pop up for every test is annoying, but using the default framebuffer without mapping it is undefined behavior. The 'correct' way to do offscreen rendering would be using FBOs which Piglit supports with the -fbo argument. Is there a reason why this can't/shouldn't be used for running Piglit offscreen? I originally removed the ability to render offscreen with the default framebuffer altogether, but I figured some users might still desire this behavior so instead decided to leave it in as a non-default option.

>

> Thanks,

> Alex

>

> -----Original Message-----

> From: ibmirkin@gmail.com [mailto:ibmirkin@gmail.com] On Behalf Of Ilia 

> Mirkin

> Sent: Friday, August 14, 2015 8:07 AM

> To: Alexander Goins

> Cc: piglit@lists.freedesktop.org

> Subject: Re: [Piglit] [PATCH] winsys-framework Default to showing 

> window

>

> Interesting. PIGLIT_FORCE_WINDOW was introduced in this change:

>

> commit b797406fb850c5e3c9fdaaebae36127b5e5f8954

> Author: Paul Berry <stereotype441@gmail.com>

> Date:   Tue Jun 11 16:34:32 2013 -0700

>

>     Add PIGLIT_FORCE_WINDOW environment variable.

>

>     Setting PIGLIT_FORCE_WINDOW=1 causes all piglit windowsystem tests

>     (those that weren't invoked with the "-fbo" flag) to show the window

>     prior to calling piglit_display().  Setting PIGLIT_FORCE_WINDOW=0 (the

>     default) preserves piglit's existing behaviour, which is to only show

>     the window when the test is run in "manual" mode (with no "-auto"

>     argument supplied).

>

>     Setting PIGLIT_FORCE_WINDOW=1 slows down the piglit run, and produces

>     more annoying windows on the screen, but it has been observed to

>     produce more repeatable results when testing with the nVidia

>     proprietary driver for Linux.

>

>     Reviewed-by: Chad Versace <chad.versace@linux.intel.com>

>

> Which explicitly mentions the nvidia blob driver thing. Having more windows popping up during piglit runs is going to be annoying... and slowing things down will also be unwelcome. Is there anything else that can be done here?

>

>   -ilia

>

> On Thu, Aug 13, 2015 at 1:49 PM, agoins <agoins@nvidia.com> wrote:

>> winsys-framework incorrectly assumed that it will always have 

>> ownership of the pixels in its own buffers. If using the default 

>> framebuffer (i.e., when Piglit is not running in FBO mode,) ownership 

>> will not be acquired unless the window is mapped.

>>

>> While this is not typically a problem because buffers are separate, 

>> NVIDIA's Unified Back Buffer feature makes all windows share a back 

>> buffer while still conforming to the OpenGL spec. If Piglit does not 

>> establish pixel ownership, its output will be clobbered by other windows.

>>

>> While this problem could be easily fixed by specifying

>> PIGLIT_FORCE_WINDOW=1 or -fbo, the current default takes advantage of 

>> undefined behavior in the OpenGL spec. A better solution would be to 

>> replace PIGLIT_FORCE_WINDOW with PIGLIT_NO_WINDOW, a flag that allows 

>> tests to be run without mapping a window or using an FBO. The default 

>> behavior, the, would be to map a window. Really, though, if users 

>> want to test with offscreen rendering, they should use FBOs with flag -fbo.

>>

>> More information on pixel ownership here:

>> https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node9

>> 4

>> .html

>> ---

>>  .../piglit-framework-gl/piglit_winsys_framework.c    | 20 ++++++++++----------

>>  1 file changed, 10 insertions(+), 10 deletions(-)

>>

>> diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c

>> b/tests/util/piglit-framework-gl/piglit_winsys_framework.c

>> index c80e972..fcba91e 100644

>> --- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c

>> +++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c

>> @@ -53,18 +53,18 @@ run_test(struct piglit_gl_framework *gl_fw,

>>           int argc, char *argv[])

>>  {

>>         struct piglit_winsys_framework *winsys_fw = piglit_winsys_framework(gl_fw);

>> -       bool force_window = false;

>> -       const char *env_force_window = getenv("PIGLIT_FORCE_WINDOW");

>> +       bool no_window = false;

>> +       const char *env_no_window = getenv("PIGLIT_NO_WINDOW");

>>

>>

>> -       if (env_force_window != NULL) {

>> -               if (strcmp(env_force_window, "0") == 0) {

>> -                       force_window = false;

>> -               } else if (strcmp(env_force_window, "1") == 0) {

>> -                       force_window = true;

>> +       if (env_no_window != NULL) {

>> +               if (strcmp(env_no_window, "0") == 0) {

>> +                       no_window = true;

>> +               } else if (strcmp(env_no_window, "1") == 0) {

>> +                       no_window = true;

>>                 } else {

>> -                       fprintf(stderr, "PIGLIT_FORCE_WINDOW has invalid"

>> -                               " value: %s\n", env_force_window);

>> +                       fprintf(stderr, "PIGLIT_NO_WINDOW has invalid"

>> +                               " value: %s\n", env_no_window);

>>                         abort();

>>                 }

>>         }

>> @@ -73,7 +73,7 @@ run_test(struct piglit_gl_framework *gl_fw,

>>                 gl_fw->test_config->init(argc, argv);

>>

>>         if (!gl_fw->test_config->requires_displayed_window &&

>> -           piglit_automatic && !force_window) {

>> +           piglit_automatic && no_window) {

>>                 enum piglit_result result = PIGLIT_PASS;

>>                 if (gl_fw->test_config->display)

>>                         result = gl_fw->test_config->display();

>> --

>> 1.9.1

>> New to the project - please commit for me.

>>

>> _______________________________________________

>> Piglit mailing list

>> Piglit@lists.freedesktop.org

>> http://lists.freedesktop.org/mailman/listinfo/piglit

> _______________________________________________

> Piglit mailing list

> Piglit@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/piglit
On 21 August 2015 at 22:08, Marek Olšák <maraeo@gmail.com> wrote:
> Hi Alex,
>
> The tests which don't specifically test the behavior of the default
> framebuffer should use -fbo. I think this is done by marking a test as
> "concurrent" in all.py. A lot of tests don't do that, because nobody
> had the time to update all.py and check if it doesn't break them and
> if it's okay to do that with regard to the purpose of the tests. (i.e.
> do they specifically test the default framebuffer or something else?)


Just FYI classic swrast suffers from this issue, and since I added front buffer
support to softpipe/llvmpipe they now also.

We try to call XGetImage when no window has been mapped, and the X server
correctly throws BadMatch. I'm not sure how to detect this is going to
fail in this
case, as I don't think inside libGL I can detect the unmapped front buffer.

Dave.
On 09/11/15 05:19, Dave Airlie wrote:
> On 21 August 2015 at 22:08, Marek Olšák <maraeo@gmail.com> wrote:
>> Hi Alex,
>>
>> The tests which don't specifically test the behavior of the default
>> framebuffer should use -fbo. I think this is done by marking a test as
>> "concurrent" in all.py. A lot of tests don't do that, because nobody
>> had the time to update all.py and check if it doesn't break them and
>> if it's okay to do that with regard to the purpose of the tests. (i.e.
>> do they specifically test the default framebuffer or something else?)
>
> Just FYI classic swrast suffers from this issue, and since I added front buffer
> support to softpipe/llvmpipe they now also.
>
> We try to call XGetImage when no window has been mapped, and the X server
> correctly throws BadMatch. I'm not sure how to detect this is going to
> fail in this
> case, as I don't think inside libGL I can detect the unmapped front buffer.
>
> Dave.

Dave, since you see a problem on swrast too, are you willing to give 
your R-b on this change?

I guess this will force us to spend time on moving tests to using fbo, 
to speed things up and be less annoying, but we should not rely on 
unspecified behaviour anyway...

Martin
Hi Alex,

You're right on the theory, but your patch seems to have an issue (read 
on).

On 2015-08-13 19:49, agoins wrote:
> winsys-framework incorrectly assumed that it will always have ownership 
> of the
> pixels in its own buffers. If using the default framebuffer (i.e., when 
> Piglit
> is not running in FBO mode,) ownership will not be acquired unless the 
> window is
> mapped.
> 
> While this is not typically a problem because buffers are separate, 
> NVIDIA's
> Unified Back Buffer feature makes all windows share a back buffer while 
> still
> conforming to the OpenGL spec. If Piglit does not establish pixel 
> ownership, its
> output will be clobbered by other windows.
> 
> While this problem could be easily fixed by specifying 
> PIGLIT_FORCE_WINDOW=1 or
> -fbo, the current default takes advantage of undefined behavior in the 
> OpenGL
> spec. A better solution would be to replace PIGLIT_FORCE_WINDOW with
> PIGLIT_NO_WINDOW, a flag that allows tests to be run without mapping a 
> window or
> using an FBO. The default behavior, the, would be to map a window. 
> Really,
> though, if users want to test with offscreen rendering, they should use 
> FBOs
> with flag -fbo.
> 
> More information on pixel ownership here:
> https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node94.html
> ---
>  .../piglit-framework-gl/piglit_winsys_framework.c    | 20 
> ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> index c80e972..fcba91e 100644
> --- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> +++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> @@ -53,18 +53,18 @@ run_test(struct piglit_gl_framework *gl_fw,
>           int argc, char *argv[])
>  {
>  	struct piglit_winsys_framework *winsys_fw = 
> piglit_winsys_framework(gl_fw);
> -	bool force_window = false;
> -	const char *env_force_window = getenv("PIGLIT_FORCE_WINDOW");
> +	bool no_window = false;
> +	const char *env_no_window = getenv("PIGLIT_NO_WINDOW");
> 
> 
> -	if (env_force_window != NULL) {
> -		if (strcmp(env_force_window, "0") == 0) {
> -			force_window = false;
> -		} else if (strcmp(env_force_window, "1") == 0) {
> -			force_window = true;
> +	if (env_no_window != NULL) {
> +		if (strcmp(env_no_window, "0") == 0) {
> +			no_window = true;
> +		} else if (strcmp(env_no_window, "1") == 0) {
> +			no_window = true;

So no_window is set to TRUE if the variable is "0" and if it is "1". 
That doesn't seem right.
Can you fix that?
Woops, yeah, typo. Sure enough, the first one should be false.

-----Original Message-----
From: Arthur Huillet [mailto:arthur.huillet@free.fr] 
Sent: Tuesday, November 17, 2015 3:10 AM
To: Alexander Goins
Cc: piglit@lists.freedesktop.org; Arthur Huillet
Subject: Re: [Piglit] [PATCH] winsys-framework Default to showing window

Hi Alex,

You're right on the theory, but your patch seems to have an issue (read on).

On 2015-08-13 19:49, agoins wrote:
> winsys-framework incorrectly assumed that it will always have 
> ownership of the pixels in its own buffers. If using the default 
> framebuffer (i.e., when Piglit is not running in FBO mode,) ownership 
> will not be acquired unless the window is mapped.
> 
> While this is not typically a problem because buffers are separate, 
> NVIDIA's Unified Back Buffer feature makes all windows share a back 
> buffer while still conforming to the OpenGL spec. If Piglit does not 
> establish pixel ownership, its output will be clobbered by other 
> windows.
> 
> While this problem could be easily fixed by specifying
> PIGLIT_FORCE_WINDOW=1 or
> -fbo, the current default takes advantage of undefined behavior in the 
> OpenGL spec. A better solution would be to replace PIGLIT_FORCE_WINDOW 
> with PIGLIT_NO_WINDOW, a flag that allows tests to be run without 
> mapping a window or using an FBO. The default behavior, the, would be 
> to map a window.
> Really,
> though, if users want to test with offscreen rendering, they should 
> use FBOs with flag -fbo.
> 
> More information on pixel ownership here:
> https://www.opengl.org/documentation/specs/version1.1/glspec1.1/node94
> .html
> ---
>  .../piglit-framework-gl/piglit_winsys_framework.c    | 20 
> ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> index c80e972..fcba91e 100644
> --- a/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> +++ b/tests/util/piglit-framework-gl/piglit_winsys_framework.c
> @@ -53,18 +53,18 @@ run_test(struct piglit_gl_framework *gl_fw,
>           int argc, char *argv[])
>  {
>  	struct piglit_winsys_framework *winsys_fw = 
> piglit_winsys_framework(gl_fw);
> -	bool force_window = false;
> -	const char *env_force_window = getenv("PIGLIT_FORCE_WINDOW");
> +	bool no_window = false;
> +	const char *env_no_window = getenv("PIGLIT_NO_WINDOW");
> 
> 
> -	if (env_force_window != NULL) {
> -		if (strcmp(env_force_window, "0") == 0) {
> -			force_window = false;
> -		} else if (strcmp(env_force_window, "1") == 0) {
> -			force_window = true;
> +	if (env_no_window != NULL) {
> +		if (strcmp(env_no_window, "0") == 0) {
> +			no_window = true;
> +		} else if (strcmp(env_no_window, "1") == 0) {
> +			no_window = true;

So no_window is set to TRUE if the variable is "0" and if it is "1". 
That doesn't seem right.
Can you fix that?

--
A. Huillet
NVIDIA Linux graphics