On 04/08/2015 01:36 AM, Ian Romanick wrote:
On 04/07/2015 03:22 AM, Francisco Jerez wrote:
Tapani Pälli <tapani.pa...@intel.com> writes:
From: Kalyan Kondapally <kalyan.kondapa...@intel.com>
Dynamic indexing of sampler arrays is prohibited by GLSL ES 3.00.
Earlier versions allow 'constant-index-expression' indexing, where
index can contain a loop induction variable.
Patch allows dynamic indexing for sampler arrays when GLSL ES < 3.00.
This change makes 'sampler-array-index.frag' parser test in Piglit
pass + fishgl.com works when running Chrome on OpenGL ES 2.0 backend.
v2: small change and some more commit message (Tapani)
Signed-off-by: Kalyan Kondapally <kalyan.kondapa...@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84225
Looks good, but did you check what happens now if the shader uses actual
variable indexing (i.e. which lowering cannot turn into a constant) on
an implementation that doesn't support it? Hopefully no crashes or
hangs?
I think we should add a post-link check that no dynamic indexing remains
after all the optimizations are complete. The intention if the ES2
language was to allow cases where the dynamic indexing could be
optimized away. This was redacted in ES3 because each optimizer was
differently capable, so a shader that worked on one driver/GPU might
fail on another... even from the same vendor.
Adding the post-link check should prevent the problems the Curro
(rightly) worried about, and it should still allow the WebGL demo to work.
I was not sure if this is worth the effort since this path has been
active for desktop GLSL < 1.30 for quite a long time, but I can take a
look at adding such check.
---
src/glsl/ast_array_index.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
index ecef651..b2609b6 100644
--- a/src/glsl/ast_array_index.cpp
+++ b/src/glsl/ast_array_index.cpp
@@ -226,7 +226,7 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
* dynamically uniform expression is undefined.
*/
if (array->type->element_type()->is_sampler()) {
- if (!state->is_version(130, 100)) {
+ if (!state->is_version(130, 300)) {
if (state->es_shader) {
_mesa_glsl_warning(&loc, state,
"sampler arrays indexed with non-constant "
It looks like this is what e3ded7f should have made this code.
Looking at the rest of the surrounding code, I don't think this is quite
right... at the very least, it's not easy to follow. You can blame me
and Paul for that. I think this is correct and easier to follow:
if (!state->is_version(400, 0) && !state->ARB_gpu_shader5_enable) {
if (state->is_version(130, 300))
_mesa_glsl_error(&loc, state,
"sampler arrays indexed with non-constant "
"expressions are forbidden in GLSL %s "
"and later"
state->es_shader ? "ES 3.00" : "1.30");
else if (state->es_shader)
_mesa_glsl_warning(&loc, state,
"sampler arrays indexed with non-constant "
"expressions are optional in %s and will "
"be forbidden in GLSL ES 3.00 and later"
state->version_string());
else
_mesa_glsl_warning(&loc, state,
"sampler arrays indexed with non-constant "
"expressions will be forbidden in GLSL "
"1.30 and later");
}
OK, thanks!
--
2.1.0
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
