<div dir="ltr">On 26 August 2013 16:04, Eric Anholt <span dir="ltr"><<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div><div>Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> writes:<br>
<br>
> From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>><br>
><br>
> This will be used in future commits.<br>
><br>
> Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>><br>
> Cc: Matt Turner <<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>><br>
> Cc: Paul Berry <<a href="mailto:stereoytpe441@gmail.com" target="_blank">stereoytpe441@gmail.com</a>><br>
> ---<br>
>  tests/util/piglit-util-gl-common.c | 86 ++++++++++++++++++++++++--------------<br>
>  tests/util/piglit-util-gl-common.h |  3 +-<br>
>  2 files changed, 56 insertions(+), 33 deletions(-)<br>
><br>
> diff --git a/tests/util/piglit-util-gl-common.c b/tests/util/piglit-util-gl-common.c<br>
> index b5e87bf..c097d8f 100644<br>
> --- a/tests/util/piglit-util-gl-common.c<br>
> +++ b/tests/util/piglit-util-gl-common.c<br>
> @@ -599,46 +599,68 @@ required_gl_version_from_glsl_version(unsigned glsl_version)<br>
>   *   float tex[4][2];<br>
>   *<br>
>   * if not NULL.<br>
> + *<br>
> + * \param fixed_function_attribute  Should fixed-function attributes (e.g.,<br>
> + *                                  \c glVertexPointer) be used?  In an OpenGL<br>
> + *                                  core profile this must be \c true.  In<br>
> + *                                  OpenGL ES the value is ignored.<br>
>   */<br>
>  void<br>
> -piglit_draw_rect_from_arrays(const void *verts, const void *tex)<br>
> +piglit_draw_rect_from_arrays(const void *verts, const void *tex,<br>
> +                             bool fixed_function_attributes)<br>
>  {<br>
> +#if defined(PIGLIT_USE_OPENGL_ES1)<br>
> +#define USE_FF(x) true<br>
> +#elif defined(PIGLIT_USE_OPENGL_ES2) ||  defined(PIGLIT_USE_OPENGL_ES3)<br>
> +#define USE_FF(x) false<br>
> +#elif defined(PIGLIT_USE_OPENGL)<br>
> +#define USE_FF(x) x<br>
> +#else<br>
> +#error "don't know how to draw arrays"<br>
> +#endif<br>
> +<br>
>  #if defined(PIGLIT_USE_OPENGL_ES1) || defined(PIGLIT_USE_OPENGL)<br>
> -     if (verts) {<br>
> -             glVertexPointer(4, GL_FLOAT, 0, verts);<br>
> -             glEnableClientState(GL_VERTEX_ARRAY);<br>
> -     }<br>
> +     if (USE_FF(fixed_function_attributes)) {<br>
> +             if (verts) {<br>
> +                     glVertexPointer(4, GL_FLOAT, 0, verts);<br>
> +                     glEnableClientState(GL_VERTEX_ARRAY);<br>
> +             }<br>
<br>
</div></div>One thing I'd been tempted to do when considering writing this patch<br>
myself was glGetIntegerv(GL_CURRENT_PROGRAM) and do patch 7/16 right<br>
here.  I felt bad about putting getters in the draw path, but your patch<br>
6 does that anyway and I think it's reasonable.  Then you wouldn't need<br>
the bool input and the funny conditional use of it here, since it would<br>
be "always use generics on GLES2, and also use them if piglit_vertex is<br>
present".  I'm assuming here that verts will always be non-NULL, which I<br>
think is reasonable.<br></blockquote><div><br></div><div>This would be my preference too (and in fact I seem to remember us agreeing on this plan when we discussed it several months ago).  It has the additional advantage that it allows us to simply call piglit_draw_rect() from any .c test, rather than decide at each call site whether to call piglit_draw_rect() or piglit_draw_rect_shader().<br>
</div></div></div></div>