Re: [Mesa-dev] [PATCH v2 12/31] glsl: allow bindless images to be declared inside structures

2017-04-26 Thread Timothy Arceri 

On 27/04/17 01:01, Nicolai Hähnle wrote:

On 26.04.2017 12:16, Timothy Arceri wrote:



On 26/04/17 20:14, Timothy Arceri wrote:



On 26/04/17 19:50, Timothy Arceri wrote:

On 26/04/17 17:28, Nicolai Hähnle wrote:

On 26.04.2017 04:45, Timothy Arceri wrote:

Reviewed-by: Timothy Arceri 

On 24/04/17 20:35, Samuel Pitoiset wrote:

The spec doesn't clearly state this, but I have got clarifiation
from the spec authors.


Okay, that answers my question from the other email.

BTW, what about samplers? It seems that this code doesn't check for
samplers at all?


That's because this is one of those places where OpenGL is totally
inconsistent. Samplers were already allowed, this was a really pain
for implementing indirect indexing, especially when throwing AoA into
the mix.

Samuel it would be really good to have some piglit tests for images.
e.g. Arrays of structs with arrays of images, etc. There should be
some examples in the ARB_gpu_shader5 and ARB_arrays_of_arrays
execution piglit tests.


  Anyway, this patch:


Reviewed-by: Nicolai Hähnle 



Signed-off-by: Samuel Pitoiset 
---
  src/compiler/glsl/ast_to_hir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index a63f9da912..5ef99bf504 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6944,7 +6944,7 @@
ast_process_struct_or_iface_block_members(exec_list *instructions,
  _mesa_glsl_error(, state, "atomic counter in
structure");
   }
  - if (decl_type->contains_image()) {
+ if (!state->has_bindless() &&
decl_type->contains_image()) {
  /* FINISHME: Same problem as with atomic counters.
   * FINISHME: Request clarification from Khronos and 
add

   * FINISHME: spec quotation here.


I believe there was a spec bug filed for this (maybe internally). We
should probably check if that has been resolved. If so we might just
have to remove this check completely.

 From what I recall there was a spec bug where the outcome said
samplers in structs were allowed and atomics were not. This is what
the FINISHME is talking about. Currently the spec doesn't say images
are not allowed.


I should clarify that this outcome was decided in a separate bug report
from images.


Which bug report was this?


The atomic bug report was 10903. I don't have access to the private 
bugzilla at the moment so I can't lookup the image bug report. I got the 
atomic number from an old git commit.





Cheers,
Nicolai










___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 12/31] glsl: allow bindless images to be declared inside structures

2017-04-26 Thread Nicolai Hähnle 

On 26.04.2017 12:16, Timothy Arceri wrote:



On 26/04/17 20:14, Timothy Arceri wrote:



On 26/04/17 19:50, Timothy Arceri wrote:

On 26/04/17 17:28, Nicolai Hähnle wrote:

On 26.04.2017 04:45, Timothy Arceri wrote:

Reviewed-by: Timothy Arceri 

On 24/04/17 20:35, Samuel Pitoiset wrote:

The spec doesn't clearly state this, but I have got clarifiation
from the spec authors.


Okay, that answers my question from the other email.

BTW, what about samplers? It seems that this code doesn't check for
samplers at all?


That's because this is one of those places where OpenGL is totally
inconsistent. Samplers were already allowed, this was a really pain
for implementing indirect indexing, especially when throwing AoA into
the mix.

Samuel it would be really good to have some piglit tests for images.
e.g. Arrays of structs with arrays of images, etc. There should be
some examples in the ARB_gpu_shader5 and ARB_arrays_of_arrays
execution piglit tests.


  Anyway, this patch:


Reviewed-by: Nicolai Hähnle 



Signed-off-by: Samuel Pitoiset 
---
  src/compiler/glsl/ast_to_hir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index a63f9da912..5ef99bf504 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6944,7 +6944,7 @@
ast_process_struct_or_iface_block_members(exec_list *instructions,
  _mesa_glsl_error(, state, "atomic counter in
structure");
   }
  - if (decl_type->contains_image()) {
+ if (!state->has_bindless() &&
decl_type->contains_image()) {
  /* FINISHME: Same problem as with atomic counters.
   * FINISHME: Request clarification from Khronos and add
   * FINISHME: spec quotation here.


I believe there was a spec bug filed for this (maybe internally). We
should probably check if that has been resolved. If so we might just
have to remove this check completely.

 From what I recall there was a spec bug where the outcome said
samplers in structs were allowed and atomics were not. This is what
the FINISHME is talking about. Currently the spec doesn't say images
are not allowed.


I should clarify that this outcome was decided in a separate bug report
from images.


Which bug report was this?

Cheers,
Nicolai










___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 12/31] glsl: allow bindless images to be declared inside structures

2017-04-26 Thread Timothy Arceri 



On 26/04/17 20:14, Timothy Arceri wrote:



On 26/04/17 19:50, Timothy Arceri wrote:

On 26/04/17 17:28, Nicolai Hähnle wrote:

On 26.04.2017 04:45, Timothy Arceri wrote:

Reviewed-by: Timothy Arceri 

On 24/04/17 20:35, Samuel Pitoiset wrote:

The spec doesn't clearly state this, but I have got clarifiation
from the spec authors.


Okay, that answers my question from the other email.

BTW, what about samplers? It seems that this code doesn't check for 
samplers at all?


That's because this is one of those places where OpenGL is totally 
inconsistent. Samplers were already allowed, this was a really pain 
for implementing indirect indexing, especially when throwing AoA into 
the mix.


Samuel it would be really good to have some piglit tests for images.
e.g. Arrays of structs with arrays of images, etc. There should be 
some examples in the ARB_gpu_shader5 and ARB_arrays_of_arrays 
execution piglit tests.



  Anyway, this patch:


Reviewed-by: Nicolai Hähnle 



Signed-off-by: Samuel Pitoiset 
---
  src/compiler/glsl/ast_to_hir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index a63f9da912..5ef99bf504 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6944,7 +6944,7 @@
ast_process_struct_or_iface_block_members(exec_list *instructions,
  _mesa_glsl_error(, state, "atomic counter in
structure");
   }
  - if (decl_type->contains_image()) {
+ if (!state->has_bindless() && decl_type->contains_image()) {
  /* FINISHME: Same problem as with atomic counters.
   * FINISHME: Request clarification from Khronos and add
   * FINISHME: spec quotation here.


I believe there was a spec bug filed for this (maybe internally). We 
should probably check if that has been resolved. If so we might just 
have to remove this check completely.


 From what I recall there was a spec bug where the outcome said samplers 
in structs were allowed and atomics were not. This is what the FINISHME 
is talking about. Currently the spec doesn't say images are not allowed.


I should clarify that this outcome was decided in a separate bug report 
from images.








___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 12/31] glsl: allow bindless images to be declared inside structures

2017-04-26 Thread Samuel Pitoiset 



On 04/26/2017 12:14 PM, Timothy Arceri wrote:



On 26/04/17 19:50, Timothy Arceri wrote:

On 26/04/17 17:28, Nicolai Hähnle wrote:

On 26.04.2017 04:45, Timothy Arceri wrote:

Reviewed-by: Timothy Arceri 

On 24/04/17 20:35, Samuel Pitoiset wrote:

The spec doesn't clearly state this, but I have got clarifiation
from the spec authors.


Okay, that answers my question from the other email.

BTW, what about samplers? It seems that this code doesn't check for 
samplers at all?


That's because this is one of those places where OpenGL is totally 
inconsistent. Samplers were already allowed, this was a really pain 
for implementing indirect indexing, especially when throwing AoA into 
the mix.


Samuel it would be really good to have some piglit tests for images.
e.g. Arrays of structs with arrays of images, etc. There should be 
some examples in the ARB_gpu_shader5 and ARB_arrays_of_arrays 
execution piglit tests.



  Anyway, this patch:


Reviewed-by: Nicolai Hähnle 



Signed-off-by: Samuel Pitoiset 
---
  src/compiler/glsl/ast_to_hir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index a63f9da912..5ef99bf504 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6944,7 +6944,7 @@
ast_process_struct_or_iface_block_members(exec_list *instructions,
  _mesa_glsl_error(, state, "atomic counter in
structure");
   }
  - if (decl_type->contains_image()) {
+ if (!state->has_bindless() && decl_type->contains_image()) {
  /* FINISHME: Same problem as with atomic counters.
   * FINISHME: Request clarification from Khronos and add
   * FINISHME: spec quotation here.


I believe there was a spec bug filed for this (maybe internally). We 
should probably check if that has been resolved. If so we might just 
have to remove this check completely.


 From what I recall there was a spec bug where the outcome said samplers 
in structs were allowed and atomics were not. This is what the FINISHME 
is talking about. Currently the spec doesn't say images are not allowed.


Right. Would be nice to clarify the situation here.







___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 12/31] glsl: allow bindless images to be declared inside structures

2017-04-26 Thread Timothy Arceri 



On 26/04/17 19:50, Timothy Arceri wrote:

On 26/04/17 17:28, Nicolai Hähnle wrote:

On 26.04.2017 04:45, Timothy Arceri wrote:

Reviewed-by: Timothy Arceri 

On 24/04/17 20:35, Samuel Pitoiset wrote:

The spec doesn't clearly state this, but I have got clarifiation
from the spec authors.


Okay, that answers my question from the other email.

BTW, what about samplers? It seems that this code doesn't check for 
samplers at all?


That's because this is one of those places where OpenGL is totally 
inconsistent. Samplers were already allowed, this was a really pain for 
implementing indirect indexing, especially when throwing AoA into the mix.


Samuel it would be really good to have some piglit tests for images.
e.g. Arrays of structs with arrays of images, etc. There should be some 
examples in the ARB_gpu_shader5 and ARB_arrays_of_arrays execution 
piglit tests.



  Anyway, this patch:


Reviewed-by: Nicolai Hähnle 



Signed-off-by: Samuel Pitoiset 
---
  src/compiler/glsl/ast_to_hir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index a63f9da912..5ef99bf504 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6944,7 +6944,7 @@
ast_process_struct_or_iface_block_members(exec_list *instructions,
  _mesa_glsl_error(, state, "atomic counter in
structure");
   }
  - if (decl_type->contains_image()) {
+ if (!state->has_bindless() && decl_type->contains_image()) {
  /* FINISHME: Same problem as with atomic counters.
   * FINISHME: Request clarification from Khronos and add
   * FINISHME: spec quotation here.


I believe there was a spec bug filed for this (maybe internally). We 
should probably check if that has been resolved. If so we might just 
have to remove this check completely.


From what I recall there was a spec bug where the outcome said samplers 
in structs were allowed and atomics were not. This is what the FINISHME 
is talking about. Currently the spec doesn't say images are not allowed.






___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 12/31] glsl: allow bindless images to be declared inside structures

2017-04-26 Thread Timothy Arceri 

On 26/04/17 17:28, Nicolai Hähnle wrote:

On 26.04.2017 04:45, Timothy Arceri wrote:

Reviewed-by: Timothy Arceri 

On 24/04/17 20:35, Samuel Pitoiset wrote:

The spec doesn't clearly state this, but I have got clarifiation
from the spec authors.


Okay, that answers my question from the other email.

BTW, what about samplers? It seems that this code doesn't check for 
samplers at all?


That's because this is one of those places where OpenGL is totally 
inconsistent. Samplers were already allowed, this was a really pain for 
implementing indirect indexing, especially when throwing AoA into the mix.


Samuel it would be really good to have some piglit tests for images.
e.g. Arrays of structs with arrays of images, etc. There should be some 
examples in the ARB_gpu_shader5 and ARB_arrays_of_arrays execution 
piglit tests.



 Anyway, this patch:


Reviewed-by: Nicolai Hähnle 



Signed-off-by: Samuel Pitoiset 
---
  src/compiler/glsl/ast_to_hir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index a63f9da912..5ef99bf504 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6944,7 +6944,7 @@
ast_process_struct_or_iface_block_members(exec_list *instructions,
  _mesa_glsl_error(, state, "atomic counter in
structure");
   }
  - if (decl_type->contains_image()) {
+ if (!state->has_bindless() && decl_type->contains_image()) {
  /* FINISHME: Same problem as with atomic counters.
   * FINISHME: Request clarification from Khronos and add
   * FINISHME: spec quotation here.


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 12/31] glsl: allow bindless images to be declared inside structures

2017-04-26 Thread Samuel Pitoiset 



On 04/26/2017 09:28 AM, Nicolai Hähnle wrote:

On 26.04.2017 04:45, Timothy Arceri wrote:

Reviewed-by: Timothy Arceri 

On 24/04/17 20:35, Samuel Pitoiset wrote:

The spec doesn't clearly state this, but I have got clarifiation
from the spec authors.


Okay, that answers my question from the other email.


Yep. :)



BTW, what about samplers? It seems that this code doesn't check for 
samplers at all? Anyway, this patch:


Nothing particular to do for samplers. But there is one additional check 
for image types.




Reviewed-by: Nicolai Hähnle 



Signed-off-by: Samuel Pitoiset 
---
  src/compiler/glsl/ast_to_hir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index a63f9da912..5ef99bf504 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6944,7 +6944,7 @@
ast_process_struct_or_iface_block_members(exec_list *instructions,
  _mesa_glsl_error(, state, "atomic counter in
structure");
   }
  - if (decl_type->contains_image()) {
+ if (!state->has_bindless() && decl_type->contains_image()) {
  /* FINISHME: Same problem as with atomic counters.
   * FINISHME: Request clarification from Khronos and add
   * FINISHME: spec quotation here.


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 12/31] glsl: allow bindless images to be declared inside structures

2017-04-26 Thread Nicolai Hähnle 

On 26.04.2017 04:45, Timothy Arceri wrote:

Reviewed-by: Timothy Arceri 

On 24/04/17 20:35, Samuel Pitoiset wrote:

The spec doesn't clearly state this, but I have got clarifiation
from the spec authors.


Okay, that answers my question from the other email.

BTW, what about samplers? It seems that this code doesn't check for 
samplers at all? Anyway, this patch:


Reviewed-by: Nicolai Hähnle 



Signed-off-by: Samuel Pitoiset 
---
  src/compiler/glsl/ast_to_hir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index a63f9da912..5ef99bf504 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6944,7 +6944,7 @@
ast_process_struct_or_iface_block_members(exec_list *instructions,
  _mesa_glsl_error(, state, "atomic counter in
structure");
   }
  - if (decl_type->contains_image()) {
+ if (!state->has_bindless() && decl_type->contains_image()) {
  /* FINISHME: Same problem as with atomic counters.
   * FINISHME: Request clarification from Khronos and add
   * FINISHME: spec quotation here.


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 12/31] glsl: allow bindless images to be declared inside structures

2017-04-25 Thread Timothy Arceri 

Reviewed-by: Timothy Arceri 

On 24/04/17 20:35, Samuel Pitoiset wrote:

The spec doesn't clearly state this, but I have got clarifiation
from the spec authors.

Signed-off-by: Samuel Pitoiset 
---
  src/compiler/glsl/ast_to_hir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index a63f9da912..5ef99bf504 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -6944,7 +6944,7 @@ ast_process_struct_or_iface_block_members(exec_list 
*instructions,
  _mesa_glsl_error(, state, "atomic counter in structure");
   }
  
- if (decl_type->contains_image()) {

+ if (!state->has_bindless() && decl_type->contains_image()) {
  /* FINISHME: Same problem as with atomic counters.
   * FINISHME: Request clarification from Khronos and add
   * FINISHME: spec quotation here.


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev