On Sun 16 Nov 2014, Jordan Justen wrote:
On 2014-11-15 12:47:05, Emil Velikov wrote:
Hi Dylan,
On 14/11/14 17:39, Dylan Baker wrote:
> v2: - Don't print for gles1, since gles1 doesn't have a shading language
>       and will always return FLINFO_GL_ERROR
>
> Signed-off-by: Dylan Baker <dylanx.c.ba...@intel.com>
> ---
>
> This addresses Jordan's comments, but from the point of view of having a
> parsable output not printing the string 'OpenGL shading language...'
> seems suboptimal. Would it be better to add an additional block to the
> assignment if and look for fixed-functions versions and set the value of
> shading langauge to 'Fixed Function' or 'None' instead?
>
I'm not sure how Chad and Jordan feel about it but making gles1's shader
langugage version return "None" sounds good imho.`

I'd rather not give output for things that are not applicable based on
the platform or api.

Maybe ... N/A for situation like this would make sense in verbose mode
though.

If wflinfo does print anything for the GLES1 GLSL version, I prefer it to be "None". If it printed "N/A", I predict that some user will get confused by over-interpreting the meaning of "N/A". On the contrary, "None" is straightforward and asks for no further interpretation.

Like Emil and Dylan, I have a slight preference that wflinfo print "None" for the GLES1 GLSL version rather than be silent. But it's only a slight preference; it doesn't matter much to me.

Dylan, why in the original patch did you decide to print anything at all for the GLES1 GLSL version? You vaguely mention parseability. Can you say more about any parsing issues you forsee, and how those issues might affect Piglit?

I don't want this thread to devolve into a bikeshed. I just want know if
you have concrete concerns about the silence vs "None" decision before any patch gets committed.

> @@ -561,6 +567,10 @@ print_wflinfo(const struct options *opts)
>      printf("OpenGL vendor string: %s\n", vendor);
>      printf("OpenGL renderer string: %s\n", renderer);
>      printf("OpenGL version string: %s\n", version_str);
> +     // Do not print WFLINFO_GL_ERROR for gles1, there is not GLSL for GL ES 
1.x
> +     if (strcmp(api, "gles1") != 0) {
> +             printf("OpenGL shading language version string: %s\n", 
language_str);
> +     }

How about a new function, and call it like this:
   if (opts->context_api != WAFFLE_CONTEXT_OPENGL_ES1) {
       print_glsl_info();
   }

That would avoid calling glGetString(GL_SHADING_LANGUAGE_VERSION) for
gles1.

Agreed. If wflinfo prints no GLES1 GLSL info in the final patch, then wflinfo shouldn't call glGetString(GL_SHADING_LANGUAGE_VERSION) at all for GLES1. It's just a good idea to avoid producing GL errors when possible.
_______________________________________________
waffle mailing list
waffle@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/waffle

Reply via email to