On 11/22/2011 07:54 AM, Marek Olšák wrote:
On Tue, Nov 22, 2011 at 4:05 PM, Jose Fonseca<jfons...@vmware.com> wrote:
Marek,
I think we should take one of two approaches here:
- aim for a short-term workaround, without gallium interface changes:
- e.g., provide to GLSL compiler infinite/huge limits, while advertising to
the app the pipe driver ones
- or detect the wine process and advertise bigger limits in the r300 driver
- aim for accurately describing the pipe driver compiling abilities
- e.g., allow the state tracker to create shaders that exceed abilities, and
force a trial generation and compilation of the TGSI shaders when the GLSL
shader is first linked
But the hard limit caps interface change you propose below doesn't quite align
to neither approach: it is an interface change which doesn't truly help
describing the pipe driver compiler abilities any better (the actual maximum
constants/inputs depends on the shader and is not a global constant), so it
merely provides a short term relief.
All of this discussion is largely moot. The failure that you're so
angry about was caused by a bug in the check, not by the check itself.
That bug has already been fixed (commit 151867b).
The exact same check was previously performed in st_glsl_to_tgsi (or
ir_to_mesa), and the exact same set of shaders would have been rejected.
The check is now done in the linker instead.
I would gladly commit this patch:
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 3527088..4e1e648 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1813,6 +1813,9 @@ assign_varying_locations(struct gl_context *ctx,
}
}
+ /* Enable this once the GLSL compiler can properly count in just
+ * active array elements, not whole arrays. */
+#if 0
if (ctx->API == API_OPENGLES2 || prog->Version == 100) {
if (varying_vectors> ctx->Const.MaxVarying) {
linker_error(prog, "shader uses too many varying vectors "
@@ -1829,6 +1832,7 @@ assign_varying_locations(struct gl_context *ctx,
return false;
}
}
+#endif
return true;
}
@@ -1959,10 +1963,16 @@ check_resources(struct gl_context *ctx, struct
gl_shader_program *prog)
shader_names[i]);
}
+ /* Enable this once the GLSL compiler can properly count in just
+ * active array elements, not whole arrays. */
+#if 0
if (sh->num_uniform_components> max_uniform_components[i]) {
linker_error(prog, "Too many %s shader uniform components",
shader_names[i]);
}
+#else
+ (void) max_uniform_components;
+#endif
}
return prog->LinkStatus;
BUT I had a very informative discussion with one Intel developer last
night and he said we can't change the linker because he's on the ARB.
:D
Now seriously. I do really care about users. And I'd like to release a
product which works best for them. Some people obviously don't want me
to release such a product.
For every shader these patches would allow to run correctly, I can find
a shader that it will allow that won't (and can't) run correctly. I can
also find another driver that advertises the same limits where shaders
in the first group already don't run.
For example, these patches would allow the following shader, which
cannot possibly work correctly:
#version 120
uniform float a[gl_MaxUniformComponents + 10];
uniform int i = gl_MaxUniformComponents + 5;
void main()
{
gl_Position = vec4(a[i]);
}
This shader cannot work, and we've lost the ability to tell the user
that it cannot work. Instead of an nice error message about exceeding
limits, the user gets incorrect rendering, an assertion failure, or a
GPU hang. None of these is correct (for any definition of correct) or
what the user wants. They're also really hard for anyone (driver
developer or app developer) to debug or fix.
I'm fairly sure that such a change would cause at least one or two
OpenGL ES 2.0 conformance tests to fail. There are a lot of tests there
that poke at various corner cases of limits.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
