[Piglit] [PATCH] cl-api-enqueue-read_write-buffer: Fix GCC format-security warning.

2014-01-27 Thread Vinson Lee
This patch fixes this GCC format-security warning.

enqueue-read_write-buffer.c: In function ‘piglit_cl_test’:
enqueue-read_write-buffer.c:319:8: warning: format not a string literal and no 
format arguments [-Wformat-security]
fprintf(stderr, test_str_read);
^

Signed-off-by: Vinson Lee 
---
 tests/cl/api/enqueue-read_write-buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/cl/api/enqueue-read_write-buffer.c 
b/tests/cl/api/enqueue-read_write-buffer.c
index bf88d5e..f98125c 100644
--- a/tests/cl/api/enqueue-read_write-buffer.c
+++ b/tests/cl/api/enqueue-read_write-buffer.c
@@ -316,7 +316,7 @@ piglit_cl_test(const int argc,
sprintf(test_str_read,
"Data read from 
buffer is not the same as data written to buffer using 0x%X as memory flags",
(unsigned 
int)mixed_mem_flags);
-   fprintf(stderr, 
test_str_read);
+   fprintf(stderr, "%s", 
test_str_read);
 
printf("  Device: %s\n  
  mem_flags: 0x%x, offset: %d, bytes: %d \n",
   device_name,
-- 
1.8.3.2

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


[Piglit] [PATCH] cl-custom-buffer-flags: Remove unused label.

2014-01-27 Thread Vinson Lee
This patch fixes this GCC unused-label warning.

buffer-flags.c: In function ‘piglit_cl_test’:
buffer-flags.c:222:1: warning: label ‘out’ defined but not used [-Wunused-label]
 out:
 ^

Signed-off-by: Vinson Lee 
---
 tests/cl/custom/buffer-flags.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/cl/custom/buffer-flags.c b/tests/cl/custom/buffer-flags.c
index 321d75f..6bf988f 100644
--- a/tests/cl/custom/buffer-flags.c
+++ b/tests/cl/custom/buffer-flags.c
@@ -219,7 +219,6 @@ piglit_cl_test(const int argc,
piglit_merge_result(&ret, part_ret);
}
 
-out:
clReleaseProgram(program);
piglit_cl_release_context(context);
return ret;
-- 
1.8.3.2

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


[Piglit] [PATCH] util-cl: Fix GCC format-security warning.

2014-01-27 Thread Vinson Lee
This patch fixes this GCC warning.

piglit-framework-cl-program.c: In function ‘piglit_cl_program_test_run’:
piglit-framework-cl-program.c:218:3: warning: format not a string literal and 
no format arguments [-Wformat-security]
   sprintf(build_options+strlen(old), config->build_options);
   ^

Signed-off-by: Vinson Lee 
---
 tests/util/piglit-framework-cl-program.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/util/piglit-framework-cl-program.c 
b/tests/util/piglit-framework-cl-program.c
index 417405d..374296f 100644
--- a/tests/util/piglit-framework-cl-program.c
+++ b/tests/util/piglit-framework-cl-program.c
@@ -215,7 +215,7 @@ piglit_cl_program_test_run(const int argc,
char* old = build_options;
build_options = malloc((strlen(old) + 
strlen(config->build_options) + 1) * sizeof(char));
strcpy(build_options, old);
-   sprintf(build_options+strlen(old), config->build_options);
+   sprintf(build_options+strlen(old), "%s", config->build_options);
free(old);
}
if(version > 10) {
-- 
1.8.3.2

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


Re: [Piglit] How to use Piglit ?

2014-01-27 Thread Ian Romanick
On 01/26/2014 01:16 AM, Maxence Le Doré wrote:
> Hello,
> 
> I would like to start contributing to Piglit. But I'm not able to
> launch a test except the sanity one given in the README. I'm sure this
> is because I'm not familiar at all with python.
> Could you give me a bunch a illustrating commands/actions that you
> usely do while testing/hacking piglit. It will make me gain a lot of
> time instead of wasting hours.

Also, individual tests are installed in ./bin.  They're just programs,
and you can run them... especially if they're tests you've written.

> Thanks
> ___
> 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


Re: [Piglit] [PATCH] Test that scalar ops in different BBs aren't combined.

2014-01-27 Thread Anuj Phogat
On Mon, Jan 27, 2014 at 11:10 AM, Matt Turner  wrote:
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74113
> ---
>  ...l-vs-vectorize-between-basic-blocks.shader_test | 41 
> ++
>  1 file changed, 41 insertions(+)
>  create mode 100644 
> tests/shaders/glsl-vs-vectorize-between-basic-blocks.shader_test
>
> diff --git a/tests/shaders/glsl-vs-vectorize-between-basic-blocks.shader_test 
> b/tests/shaders/glsl-vs-vectorize-between-basic-blocks.shader_test
> new file mode 100644
> index 000..10f0502
> --- /dev/null
> +++ b/tests/shaders/glsl-vs-vectorize-between-basic-blocks.shader_test
> @@ -0,0 +1,41 @@
> +# Tests that scalar operations in different basic blocks
> +# are not combined incorrectly into a vector operation.
> +#
> +# Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74113
> +
> +[require]
> +GLSL >= 1.10
> +
> +[vertex shader]
> +uniform float x, y;
> +uniform vec2 v;
> +
> +attribute vec4 piglit_vertex;
> +varying vec2 color;
> +
> +void main()
> +{
> +color = vec2(0.0);
> +
> +if (x == y)
> +color.x += v.x;
> +else
> +color.y += v.y;
> +
> +gl_Position = piglit_vertex;
> +}
> +
> +[fragment shader]
> +varying vec2 color;
> +
> +void main()
> +{
> +  gl_FragColor = vec4(color, 0.0, 1.0);
> +}
> +
> +[test]
> +uniform float x 3.0
> +uniform float y 3.0
> +uniform vec2 v 0.25 0.25
> +draw rect -1 -1 2 2
> +probe all rgba 0.25 0 0 1
> --
> 1.8.3.2
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit

Reviewed-by: Anuj Phogat 
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] Test that scalar ops in different BBs aren't combined.

2014-01-27 Thread Matt Turner
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74113
---
 ...l-vs-vectorize-between-basic-blocks.shader_test | 41 ++
 1 file changed, 41 insertions(+)
 create mode 100644 
tests/shaders/glsl-vs-vectorize-between-basic-blocks.shader_test

diff --git a/tests/shaders/glsl-vs-vectorize-between-basic-blocks.shader_test 
b/tests/shaders/glsl-vs-vectorize-between-basic-blocks.shader_test
new file mode 100644
index 000..10f0502
--- /dev/null
+++ b/tests/shaders/glsl-vs-vectorize-between-basic-blocks.shader_test
@@ -0,0 +1,41 @@
+# Tests that scalar operations in different basic blocks
+# are not combined incorrectly into a vector operation.
+#
+# Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74113
+
+[require]
+GLSL >= 1.10
+
+[vertex shader]
+uniform float x, y;
+uniform vec2 v;
+
+attribute vec4 piglit_vertex;
+varying vec2 color;
+
+void main()
+{
+color = vec2(0.0);
+
+if (x == y)
+color.x += v.x;
+else
+color.y += v.y;
+
+gl_Position = piglit_vertex;
+}
+
+[fragment shader]
+varying vec2 color;
+
+void main()
+{
+  gl_FragColor = vec4(color, 0.0, 1.0);
+}
+
+[test]
+uniform float x 3.0
+uniform float y 3.0
+uniform vec2 v 0.25 0.25
+draw rect -1 -1 2 2
+probe all rgba 0.25 0 0 1
-- 
1.8.3.2

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


Re: [Piglit] [PATCH V2] Add test to verify 'centroid' qualifier is ignored in case of persample shading

2014-01-27 Thread Anuj Phogat
On Mon, Jan 27, 2014 at 10:34 AM, Paul Berry  wrote:
> On 23 January 2014 15:56, Anuj Phogat  wrote:
>>
>> Here is what it tests:
>> Enable sample shading for the whole fragment shader using OpenGL API
>> glEnable(GL_SAMPLE_SHADING);
>> glMinSampleShading(1.0);
>>
>> fragment shader:
>> centroid in vec4 b;
>> main()
>> {
>>   ...
>> }
>>
>> Variable 'b' should be interpolated at sample's location.
>>
>> V2: Use just one 'in' variable with the 'centroid' qualifier.
>> Another test already covers the interpolation of 'in' variable
>> without any 'qualifier'. This change also captures the mesa bug
>> reported on i965 driver:
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73915
>>
>> Signed-off-by: Anuj Phogat 
>> Cc: Chris Forbes 
>> ---
>>  tests/all.py   |   5 +
>>  .../arb_sample_shading/execution/CMakeLists.gl.txt |   1 +
>>  .../execution/ignore-centroid-qualifier.cpp| 193
>> +
>>  3 files changed, 199 insertions(+)
>>  create mode 100644
>> tests/spec/arb_sample_shading/execution/ignore-centroid-qualifier.cpp
>>
>> diff --git a/tests/all.py b/tests/all.py
>> index 459bcca..5fceff9 100644
>> --- a/tests/all.py
>> +++ b/tests/all.py
>> @@ -1789,6 +1789,11 @@ for num_samples in MSAA_SAMPLE_COUNTS:
>>  executable = 'arb_sample_shading-{0} -auto'.format(test_name)
>>  arb_sample_shading[test_name] = PlainExecTest(executable)
>>
>> +for num_samples in MSAA_SAMPLE_COUNTS:
>> +test_name = 'ignore-centroid-qualifier {0}'.format(num_samples)
>> +executable = 'arb_sample_shading-{0} -auto'.format(test_name)
>> +arb_sample_shading[test_name] = PlainExecTest(executable)
>> +
>>  import_glsl_parser_tests(spec['ARB_sample_shading'],
>>   os.path.join(testsDir, 'spec',
>> 'arb_sample_shading'),
>>   ['compiler'])
>> diff --git a/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt
>> b/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt
>> index 9a72439..c3690e9 100644
>> --- a/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt
>> +++ b/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt
>> @@ -16,4 +16,5 @@ piglit_add_executable
>> (arb_sample_shading-builtin-gl-sample-id builtin-gl-sample
>>  piglit_add_executable (arb_sample_shading-builtin-gl-sample-mask
>> builtin-gl-sample-mask.cpp)
>>  piglit_add_executable (arb_sample_shading-builtin-gl-sample-position
>> builtin-gl-sample-position.cpp)
>>  piglit_add_executable (arb_sample_shading-interpolate-at-sample-position
>> interpolate-at-sample-position.cpp)
>> +piglit_add_executable (arb_sample_shading-ignore-centroid-qualifier
>> ignore-centroid-qualifier.cpp)
>>  # vim: ft=cmake:
>> diff --git
>> a/tests/spec/arb_sample_shading/execution/ignore-centroid-qualifier.cpp
>> b/tests/spec/arb_sample_shading/execution/ignore-centroid-qualifier.cpp
>> new file mode 100644
>> index 000..cb54655
>> --- /dev/null
>> +++
>> b/tests/spec/arb_sample_shading/execution/ignore-centroid-qualifier.cpp
>> @@ -0,0 +1,193 @@
>> +/*
>> + * Copyright (c) 2014 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining
>> a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the
>> next
>> + * paragraph) shall be included in all copies or substantial portions of
>> the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +
>> +/**
>> + * @file ignore-centroid-qualifier.cpp
>> + *
>> + * Tests that all 'in' variables in fragment shader are interpolated at
>> sample
>> + * positions when using per sample shading. Ignore the 'centroid'
>> qualifier if
>> + * used with 'in' variable.
>> + *
>> + */
>> +#include "piglit-util-gl-common.h"
>> +#include "piglit-fbo.h"
>> +
>> +using namespace piglit_util_fbo;
>> +const int pattern_width = 128; const int pattern_height = 128;
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +   config.supports_gl_compat_version = 21;
>> +   config.supports_gl_core_version = 31;
>> +config.window_width = 2 * pattern_width;
>> 

Re: [Piglit] [PATCH V2] Add test to verify 'centroid' qualifier is ignored in case of persample shading

2014-01-27 Thread Paul Berry
On 23 January 2014 15:56, Anuj Phogat  wrote:

> Here is what it tests:
> Enable sample shading for the whole fragment shader using OpenGL API
> glEnable(GL_SAMPLE_SHADING);
> glMinSampleShading(1.0);
>
> fragment shader:
> centroid in vec4 b;
> main()
> {
>   ...
> }
>
> Variable 'b' should be interpolated at sample's location.
>
> V2: Use just one 'in' variable with the 'centroid' qualifier.
> Another test already covers the interpolation of 'in' variable
> without any 'qualifier'. This change also captures the mesa bug
> reported on i965 driver:
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73915
>
> Signed-off-by: Anuj Phogat 
> Cc: Chris Forbes 
> ---
>  tests/all.py   |   5 +
>  .../arb_sample_shading/execution/CMakeLists.gl.txt |   1 +
>  .../execution/ignore-centroid-qualifier.cpp| 193
> +
>  3 files changed, 199 insertions(+)
>  create mode 100644
> tests/spec/arb_sample_shading/execution/ignore-centroid-qualifier.cpp
>
> diff --git a/tests/all.py b/tests/all.py
> index 459bcca..5fceff9 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -1789,6 +1789,11 @@ for num_samples in MSAA_SAMPLE_COUNTS:
>  executable = 'arb_sample_shading-{0} -auto'.format(test_name)
>  arb_sample_shading[test_name] = PlainExecTest(executable)
>
> +for num_samples in MSAA_SAMPLE_COUNTS:
> +test_name = 'ignore-centroid-qualifier {0}'.format(num_samples)
> +executable = 'arb_sample_shading-{0} -auto'.format(test_name)
> +arb_sample_shading[test_name] = PlainExecTest(executable)
> +
>  import_glsl_parser_tests(spec['ARB_sample_shading'],
>   os.path.join(testsDir, 'spec',
> 'arb_sample_shading'),
>   ['compiler'])
> diff --git a/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt
> b/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt
> index 9a72439..c3690e9 100644
> --- a/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt
> +++ b/tests/spec/arb_sample_shading/execution/CMakeLists.gl.txt
> @@ -16,4 +16,5 @@ piglit_add_executable
> (arb_sample_shading-builtin-gl-sample-id builtin-gl-sample
>  piglit_add_executable (arb_sample_shading-builtin-gl-sample-mask
> builtin-gl-sample-mask.cpp)
>  piglit_add_executable (arb_sample_shading-builtin-gl-sample-position
> builtin-gl-sample-position.cpp)
>  piglit_add_executable (arb_sample_shading-interpolate-at-sample-position
> interpolate-at-sample-position.cpp)
> +piglit_add_executable (arb_sample_shading-ignore-centroid-qualifier
> ignore-centroid-qualifier.cpp)
>  # vim: ft=cmake:
> diff --git
> a/tests/spec/arb_sample_shading/execution/ignore-centroid-qualifier.cpp
> b/tests/spec/arb_sample_shading/execution/ignore-centroid-qualifier.cpp
> new file mode 100644
> index 000..cb54655
> --- /dev/null
> +++ b/tests/spec/arb_sample_shading/execution/ignore-centroid-qualifier.cpp
> @@ -0,0 +1,193 @@
> +/*
> + * Copyright (c) 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> next
> + * paragraph) shall be included in all copies or substantial portions of
> the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +/**
> + * @file ignore-centroid-qualifier.cpp
> + *
> + * Tests that all 'in' variables in fragment shader are interpolated at
> sample
> + * positions when using per sample shading. Ignore the 'centroid'
> qualifier if
> + * used with 'in' variable.
> + *
> + */
> +#include "piglit-util-gl-common.h"
> +#include "piglit-fbo.h"
> +
> +using namespace piglit_util_fbo;
> +const int pattern_width = 128; const int pattern_height = 128;
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +   config.supports_gl_compat_version = 21;
> +   config.supports_gl_core_version = 31;
> +config.window_width = 2 * pattern_width;
> +config.window_height = pattern_height;
> +   config.window_visual = PIGLIT_GL_VISUAL_DOUBLE |
> PIGLIT_GL_VISUAL_RGBA;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static Fbo multisamp

Re: [Piglit] [PATCH] arb_texture_view-formats: Silence uninitialized variable warning.

2014-01-27 Thread Jon Ashburn

Looks good.
Reviewed-by: Jon Ashburn 

On 1/25/14, 12:05 AM, Vinson Lee wrote:

This patch fixes this GCC maybe-uninitialized warning.

formats.c: In function 'test_format_errors':
formats.c:269:27: warning: 'numFormats' may be used uninitialized in this 
function [-Wmaybe-uninitialized]
   pass = check_format_array(GL_NO_ERROR, numFormats, legalFormats,
^

Signed-off-by: Vinson Lee 
---
  tests/spec/arb_texture_view/formats.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/spec/arb_texture_view/formats.c 
b/tests/spec/arb_texture_view/formats.c
index d971c3b..78d0869 100644
--- a/tests/spec/arb_texture_view/formats.c
+++ b/tests/spec/arb_texture_view/formats.c
@@ -256,6 +256,7 @@ test_format_errors(GLenum format_class)
break;
default:
assert(!"Invalid format_class\n");
+   numFormats = 0;
}
  
  	if (!piglit_check_gl_error(GL_NO_ERROR)) {


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


Re: [Piglit] [PATCH] arb_texture_view-targets: Silence uninitialized variable warning.

2014-01-27 Thread Jon Ashburn

Looks fine.

Reviewed-by: Jon Ashburn 

On 1/25/14, 12:32 AM, Vinson Lee wrote:

This patch fixes this GCC maybe-uninitialized warning.

targets.c: In function 'test_target_errors':
targets.c:198:35: warning: 'numTargets' may be used uninitialized in this 
function [-Wmaybe-uninitialized]
   pass = pass && check_target_array(GL_NO_ERROR, numTargets, legalTargets,
^

Signed-off-by: Vinson Lee 
---
  tests/spec/arb_texture_view/targets.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/spec/arb_texture_view/targets.c 
b/tests/spec/arb_texture_view/targets.c
index 60bff5b..9dd0858 100644
--- a/tests/spec/arb_texture_view/targets.c
+++ b/tests/spec/arb_texture_view/targets.c
@@ -184,6 +184,7 @@ test_target_errors(GLenum target)
break;
default:
assert(0);
+   numTargets = 0;
break;
}
  


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


[Piglit] Fwd: [PATCH 00/10] Allow building and running EGL+GLES tests without GLX

2014-01-27 Thread Daniel Kurtz
On Jan 20, 2014 4:47 AM, "Chad Versace"  wrote:
>
> On Wed, Jan 15, 2014 at 07:09:55PM +0800, Daniel Kurtz wrote:
> > With the following patches I am able to build and run piglit tests on a 
> > system
> > with no OpenGL and no GLX (ie, no glproto, no libGL.so and no 
> > /usr/include/GL).
> >
> > The X11, GBM and wayland patches are really 'bug reports', it seems like 
> > they
> > too should be probed rather than assumed.
>
> I tested building and running this series (minus the last dlsym patch)
> on a system with no libraries nor headers for X11 and OpenGL. Instead
> the system had GLES1, GLES2, GLES3, Wayland and GBM. Unsurprisingly,
> Piglit failed to build and the series needed a few touch-ups. Rather
> than asking you to set up such a system, fix the bugs, and resubmit the
> series, I went ahead and applied the fixes myself and committed the
> series. It was painful enough for me to set up an X-free GL-free system
> that I didn't want to make you go through the pain too :)
>
> Thanks, really, thanks for getting the ball rolling on breaking Piglit's
> dependency on X11.
>
> About the fixes I applied...
>
> Using my X-free GL-free test system, I discovered that, even with your
> series, Piglit refused to build. I had to reorder a few of your patches
> and add two more to get Piglit to work my test system. My extra two are:
>
> cmake: Fix libpiglitutil on systems without X11
> egl_khr_create_context: Fix X11 and GL requirements in CMake
>
> I fixed a bug in your patch "cmake: GLX is not always present on Linux".
> The patch turned off PIGLIT_HAS_GLX too eagerly. PIGLIT_HAS_GLX doesn't
> actually require glproto, but PIGLIT_BUILD_GLX_TEST does not

I guess there is something about GLX and glproto that I'm missing. I
was assuming that glproto defined the GLX protocol, so a system "has
GLX" iff it has glproto installed.

> Your bugreport patch about probing Wayland support, I completed it by
> probing for wayland-client.pc and wayland-egl.pc.
>
> I also expanded or clarified a few of the commit messages where you
> stated questions or uncertainty in the message.
>
> So... Patches 1-9 of your series are now committed to master. The few
> fixes and expanded commit messages I did are documented with a v2 tag.
>
> I successfully ran Piglit with PIGLIT_PLATFORM=gbm after applying the
> fixes. Honestly, I was surprised that it worked! The system had GBM,
> GLES1, GLES2, and GLES3. The result summary is below. The full summary,
> with all skipped tests removed, is attached.

Thanks for all of the reviewing and testing!

>
> summary:
> pass: 106
> fail: 3
> crash: 1
> skip: 15455
> warn: 0
> dmesg-warn: 0
> dmesg-fail: 0
> total: 15565
>
> I also did a regression test against Piglit master on Intel Sandybridge
> system with GLX and OpenGL installed, as well as GLES1,2,3. There were
> no *real* regressions; just the usual sporadic failures due to Python
> bugs and whatnot.
>
> > I'm really not sure about that last patch. It works for my situation, since
> > my libEGL.so, libGLESv2.so and libGLESv1_CM.so are all exactly the same 
> > library
> > (they are all just symlinks to vendor .so). But I don't think it will work 
> > in
> > the general case where these are really separate libraries, with potentially
> > conflicting core functions.
>
> I'm also uncertain about your last patch.
>
> Are you aware of any shipping EGL implementation that we wish to run
> Piglit on in the short-term and where eglGetProcAddress to fails to work
> for core functions? Mesa doesn't suffer from that problem; it behaves
> as if it supported EGL_KHR_get_all_proc_addresses even though it does
> not yet expose the extension string.

Yes, the mali libEGL is "to spec": eglGetProcAddress does not return
valid function pointers for GL core functions.
For example, using eglGetProcAddress() to fetch glGetString returns NULL:
piglit # shader_runner_gles3
/usr/local/piglit/tests/spec/glsl-es-3.00/execution/varying-struct-copy-out-vs.shader_test
-auto
get_ext_proc_address: eglGetProcAddress(glGetString) => (nil)
GetProcAddress failed for "glGetString"

That is a major stumbling block for me. It also makes it difficult to
use Eric Anholt's libepoxy, which suffers from a similar set of
libGL/GLX-dependency issues.
After hacking up epoxy locally to get gles-only core function dispatch
working, I am now more confident with using dlsym.

> If the problem is only theoretical, and Piglit will not encounter such
> an implementation in the short-term, then I propose we drop the dlsym
> patch. Otherwise, we need to design the patch to work reliably in the
> general case where distinct EGL, GL, and GLES 1-3 libraries are
> installed, which is the case for Mesa.
>

The current hack I have just does dlsym on the executable, which will
follow some default path to find the requested GL client library
symbol. However, I think we agree that to be 100% correct we should
dispatch client library core symbols by doing dlsym on the specific
client library. AFAICT, this 

Re: [Piglit] [PATCH 1/8] layered-rendering/blit: use color other than the default red

2014-01-27 Thread Pohjolainen, Topi
On Sun, Jan 26, 2014 at 11:29:45PM -0800, Kenneth Graunke wrote:
> On 01/26/2014 01:34 AM, Topi Pohjolainen wrote:
> > Passes on nvidia and on IVB using blt-engine and mesa fallback
> > path but fails on IVB using blorp (both before and after the
> > recent refactoring):
> > 
> > Probe color at (32,0)
> >   Expected: 0.50 0.40 0.30
> >   Observed: 0.250980 0.160784 0.090196
> > Probe color at (96,0)
> >   Expected: 0.50 0.40 0.30
> >   Observed: 0.250980 0.160784 0.090196
> > Probe color at (32,64)
> >   Expected: 0.50 0.40 0.30
> >   Observed: 0.250980 0.160784 0.090196
> > Probe color at (96,64)
> >   Expected: 0.50 0.40 0.30
> >   Observed: 0.250980 0.160784 0.090196
> 
> This looks a lot like botched SRGB handling.  I saw BlitFramebuffers
> with a source format of XRGB and a destination format of SRGB8.

This is odd, I tried the latest mesa-master and for me the destination
is either MESA_FORMAT_XRGB (RGB-texture) or MESA_FORMAT_ARGB
(the framebuffer).

> Commenting out the A -> 1.0 override in gen6_blorp_emit_blend_state
> makes the test pass, so something's probably broken around there...

As we discussed offline, I'll try a patch which effectively reverts:

commit c0554141a9b831b4e614747104dcbbe0fe489b9d
Author: Kenneth Graunke 
Date:   Mon Jan 28 22:03:18 2013 -0800

i965/blorp: Support overriding destination alpha to 1.0.

Currently, Blorp requires the source and destination formats to be
equal.  However, we'd really like to be able to blit between XRGB and
ARGB formats; our BLT engine paths have supported this for a long time.

For ARGB -> XRGB, nothing needs to occur: the missing alpha is already
interpreted as 1.0.  For XRGB -> ARGB, we need to smash the alpha
channel to 1.0 when writing the destination colors.  This is fairly
straightforward with blending.

For now, this code is never used, as the source and destination formats
still must be equal.  The next patch will relax that restriction.

NOTE: This is a candidate for the 9.1 branch.

Signed-off-by: Kenneth Graunke 
Reviewed-by: Ian Romanick 
Tested-by: Martin Steigerwald 

diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp b/src/mesa/drivers/dri/i96
index eb61898..3834ae2 100644
--- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
@@ -283,6 +283,25 @@ gen6_blorp_emit_blend_state(struct brw_context *brw,
blend->blend1.write_disable_b = false;
blend->blend1.write_disable_a = false;
 
+   /* When blitting from an XRGB source to a ARGB destination, we need to
+* interpret the missing channel as 1.0.  Blending can do that for us:
+* we simply use the RGB values from the fragment shader ("source RGB"),
+* but smash the alpha channel to 1.
+*/
+   if (_mesa_get_format_bits(params->dst.mt->format, GL_ALPHA_BITS) > 0 &&
+   _mesa_get_format_bits(params->src.mt->format, GL_ALPHA_BITS) == 0) {
+  blend->blend0.blend_enable = 1;
+  blend->blend0.ia_blend_enable = 1;
+
+  blend->blend0.blend_func = BRW_BLENDFUNCTION_ADD;
+  blend->blend0.ia_blend_func = BRW_BLENDFUNCTION_ADD;
+
+  blend->blend0.source_blend_factor = BRW_BLENDFACTOR_SRC_COLOR;
+  blend->blend0.dest_blend_factor = BRW_BLENDFACTOR_ZERO;
+  blend->blend0.ia_source_blend_factor = BRW_BLENDFACTOR_ONE;
+  blend->blend0.ia_dest_blend_factor = BRW_BLENDFACTOR_ZERO;
+   }
+
return cc_blend_state_offset;
 }


--
Our blorp logic samples the XRGB normally and gets alpha channel set
to 1.0 automatically by the sampler engine. This is simply copied in the
blorp blit program directly to the payload of the render target write
message and hence we shouldn't need any additional blending support from
the pixel processing pipeline.

> 
> Good luck!
> 


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