Re: [Piglit] [PATCH] arb_compute_variable_group_size: Fix require section.

2016-10-18 Thread Nicolai Hähnle 

On 17.10.2016 22:33, Matt Turner wrote:

On Mon, Oct 17, 2016 at 12:54 PM, Samuel Pitoiset
 wrote:



On 10/17/2016 09:45 PM, Samuel Pitoiset wrote:


Thanks for fixing this.

Reviewed-by: Samuel Pitoiset 



Actually, we need to check for both ARB_compute_shader and
ARB_compute_variable_group_size since
https://cgit.freedesktop.org/mesa/mesa/commit/?id=8785a8ff8948385a913e9bd75e8cdd1092bd750f.


Strange. Shouldn't a [compute shader] section (or requiring
GL_ARB_compute_variable_group_size) be sufficient?

I'll make the change regardless.


They're slightly different things. Your patch made sure that the test is 
skipped when ARB_compute_variable_group_size isn't available. And it's 
true that for the [require] section, you actually don't need to 
explicitly check for ARB_compute_shader on top of that, because a driver 
that advertises ARB_compute_variable_group_size must also advertise 
ARB_compute_shader.


My patch was mostly about adding the `#extension GL_ARB_compute_shader: 
enable` to the shaders themselves. I've pushed that now as well.


Cheers,
Nicolai
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH] arb_compute_variable_group_size: Fix require section.

2016-10-17 Thread Dylan Baker 
Quoting Matt Turner (2016-10-17 13:33:11)
> On Mon, Oct 17, 2016 at 12:54 PM, Samuel Pitoiset
>  wrote:
> >
> >
> > On 10/17/2016 09:45 PM, Samuel Pitoiset wrote:
> >>
> >> Thanks for fixing this.
> >>
> >> Reviewed-by: Samuel Pitoiset 
> >
> >
> > Actually, we need to check for both ARB_compute_shader and
> > ARB_compute_variable_group_size since
> > https://cgit.freedesktop.org/mesa/mesa/commit/?id=8785a8ff8948385a913e9bd75e8cdd1092bd750f.
> 
> Strange. Shouldn't a [compute shader] section (or requiring
> GL_ARB_compute_variable_group_size) be sufficient?

No, [computer shader] doesn't imply the requirement. The way shader runner is
implemented it checks the [require] block, then if everything seems good starts
compiling and linking, and doesn't expect errors. So you either need the GL/GLSL
version to imply the extension or to ask for it explicitly.

The fast skipping code in the python layer has the same limitations

Dylan

> 
> I'll make the change regardless.
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit


signature.asc
Description: signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH] arb_compute_variable_group_size: Fix require section.

2016-10-17 Thread Samuel Pitoiset 



On 10/17/2016 10:33 PM, Matt Turner wrote:

On Mon, Oct 17, 2016 at 12:54 PM, Samuel Pitoiset
 wrote:



On 10/17/2016 09:45 PM, Samuel Pitoiset wrote:


Thanks for fixing this.

Reviewed-by: Samuel Pitoiset 



Actually, we need to check for both ARB_compute_shader and
ARB_compute_variable_group_size since
https://cgit.freedesktop.org/mesa/mesa/commit/?id=8785a8ff8948385a913e9bd75e8cdd1092bd750f.


Strange. Shouldn't a [compute shader] section (or requiring
GL_ARB_compute_variable_group_size) be sufficient?


Doesn't seem like.



I'll make the change regardless.


Nicolai also submitted a patch pretty similar to this one, except that 
he checks for both extensions.





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

Re: [Piglit] [PATCH] arb_compute_variable_group_size: Fix require section.

2016-10-17 Thread Matt Turner 
On Mon, Oct 17, 2016 at 12:54 PM, Samuel Pitoiset
 wrote:
>
>
> On 10/17/2016 09:45 PM, Samuel Pitoiset wrote:
>>
>> Thanks for fixing this.
>>
>> Reviewed-by: Samuel Pitoiset 
>
>
> Actually, we need to check for both ARB_compute_shader and
> ARB_compute_variable_group_size since
> https://cgit.freedesktop.org/mesa/mesa/commit/?id=8785a8ff8948385a913e9bd75e8cdd1092bd750f.

Strange. Shouldn't a [compute shader] section (or requiring
GL_ARB_compute_variable_group_size) be sufficient?

I'll make the change regardless.
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH] arb_compute_variable_group_size: Fix require section.

2016-10-17 Thread Samuel Pitoiset 



On 10/17/2016 09:45 PM, Samuel Pitoiset wrote:

Thanks for fixing this.

Reviewed-by: Samuel Pitoiset 


Actually, we need to check for both ARB_compute_shader and 
ARB_compute_variable_group_size since 
https://cgit.freedesktop.org/mesa/mesa/commit/?id=8785a8ff8948385a913e9bd75e8cdd1092bd750f.




On 10/17/2016 09:39 PM, Matt Turner wrote:

---
 .../linker/mixed_fixed_variable_local_work_size.shader_test
| 2 +-
 .../linker/no_local_size_specified.shader_test
| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git
a/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
b/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test

index d660d56..32088ad 100644
---
a/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test

+++
b/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test

@@ -7,7 +7,7 @@
 [require]
 GL >= 3.3
 GLSL >= 3.30
-GL_ARB_compute_shader
+GL_ARB_compute_variable_group_size

 [compute shader]
 #version 330
diff --git
a/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
b/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test

index 6371c29..92a1646 100644
---
a/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test

+++
b/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test

@@ -7,7 +7,7 @@
 [require]
 GL >= 3.3
 GLSL >= 3.30
-GL_ARB_compute_shader
+GL_ARB_compute_variable_group_size

 [compute shader]
 #version 330


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

Re: [Piglit] [PATCH] arb_compute_variable_group_size: Fix require section.

2016-10-17 Thread Samuel Pitoiset 

Thanks for fixing this.

Reviewed-by: Samuel Pitoiset 

On 10/17/2016 09:39 PM, Matt Turner wrote:

---
 .../linker/mixed_fixed_variable_local_work_size.shader_test | 2 +-
 .../linker/no_local_size_specified.shader_test  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
 
b/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
index d660d56..32088ad 100644
--- 
a/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
+++ 
b/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
@@ -7,7 +7,7 @@
 [require]
 GL >= 3.3
 GLSL >= 3.30
-GL_ARB_compute_shader
+GL_ARB_compute_variable_group_size

 [compute shader]
 #version 330
diff --git 
a/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
 
b/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
index 6371c29..92a1646 100644
--- 
a/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
+++ 
b/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
@@ -7,7 +7,7 @@
 [require]
 GL >= 3.3
 GLSL >= 3.30
-GL_ARB_compute_shader
+GL_ARB_compute_variable_group_size

 [compute shader]
 #version 330


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

Re: [Piglit] [PATCH] arb_compute_variable_group_size: Fix require section.

2016-10-17 Thread Jordan Justen 
Reviewed-by: Jordan Justen 

On 2016-10-17 12:39:18, Matt Turner wrote:
> ---
>  .../linker/mixed_fixed_variable_local_work_size.shader_test | 2 
> +-
>  .../linker/no_local_size_specified.shader_test  | 2 
> +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git 
> a/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
>  
> b/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
> index d660d56..32088ad 100644
> --- 
> a/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
> +++ 
> b/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
> @@ -7,7 +7,7 @@
>  [require]
>  GL >= 3.3
>  GLSL >= 3.30
> -GL_ARB_compute_shader
> +GL_ARB_compute_variable_group_size
>  
>  [compute shader]
>  #version 330
> diff --git 
> a/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
>  
> b/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
> index 6371c29..92a1646 100644
> --- 
> a/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
> +++ 
> b/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
> @@ -7,7 +7,7 @@
>  [require]
>  GL >= 3.3
>  GLSL >= 3.30
> -GL_ARB_compute_shader
> +GL_ARB_compute_variable_group_size
>  
>  [compute shader]
>  #version 330
> -- 
> 2.7.3
> 
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH] arb_compute_variable_group_size: Fix require section.

2016-10-17 Thread Matt Turner 
---
 .../linker/mixed_fixed_variable_local_work_size.shader_test | 2 +-
 .../linker/no_local_size_specified.shader_test  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
 
b/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
index d660d56..32088ad 100644
--- 
a/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
+++ 
b/tests/spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size.shader_test
@@ -7,7 +7,7 @@
 [require]
 GL >= 3.3
 GLSL >= 3.30
-GL_ARB_compute_shader
+GL_ARB_compute_variable_group_size
 
 [compute shader]
 #version 330
diff --git 
a/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
 
b/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
index 6371c29..92a1646 100644
--- 
a/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
+++ 
b/tests/spec/arb_compute_variable_group_size/linker/no_local_size_specified.shader_test
@@ -7,7 +7,7 @@
 [require]
 GL >= 3.3
 GLSL >= 3.30
-GL_ARB_compute_shader
+GL_ARB_compute_variable_group_size
 
 [compute shader]
 #version 330
-- 
2.7.3

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