Re: [Piglit] [PATCH v2] winsys-framework Default to showing window

[Martin Peres]

On 17/11/15 22:01, Dave Airlie wrote:

On 18 November 2015 at 05:49, Arthur Huillet  wrote:

Thanks :)

Reviewed by: Arthur Huillet 

Reviewed-by: Dave Airlie 


Pushed
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH v2] winsys-framework Default to showing window

[Arthur Huillet]

Thanks :)

Reviewed by: Arthur Huillet 

On Tue, 17 Nov 2015 11:33:31 -0800
Alex Goins  wrote:

> From: agoins 
> 
> 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..ff3428a 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 = false;
> + } 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
> 
> 
> ---
> 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.
> ---
> 


-- 
Greetings, 
A. Huillet
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH v2] winsys-framework Default to showing window

[Dave Airlie]

On 18 November 2015 at 05:49, Arthur Huillet  wrote:
> Thanks :)
>
> Reviewed by: Arthur Huillet 

Reviewed-by: Dave Airlie 
>
> On Tue, 17 Nov 2015 11:33:31 -0800
> Alex Goins  wrote:
>
>> From: agoins 
>>
>> 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..ff3428a 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 = false;
>> + } 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
>>
>>
>> ---
>> 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.
>> ---
>>
>
>
> --
> Greetings,
> A. Huillet
> ___
> 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

[Piglit] [PATCH v2] winsys-framework Default to showing window

[Alex Goins]

From: agoins 

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..ff3428a 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 = false;
+   } 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

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit