[virglrenderer-devel] [PATCH] vrend: Enable vertex_attrib_binding also on Hosts with GLES >= 3.1

[Gurchetan Singh]

On Mon, May 21, 2018 at 8:46 AM Gert Wollny <gert.wollny at collabora.com>
wrote:

> On GLES >= 3.1 the functionality that is currently relevant for attribute
> binding in virglrenderer is available. by enabling this functionality the
> new
> code path vrend_draw_bind_vertex_binding is used.
>
> Fixes:
>   dEQP-GLES3.functional.clipping.point.wide_point_z_clip
>   dEQP-GLES3.functional.clipping.point.wide_point_z_clip_viewport_center
>   dEQP-GLES3.functional.clipping.point.wide_point_z_clip_viewport_corner
>
> On an Intel Kabylake host this also fixes:
>
>   dEQP-GLES3.functional.clipping.point.wide_point_clip
>   dEQP-GLES3.functional.clipping.point.wide_point_clip_viewport_center
>   dEQP-GLES3.functional.clipping.point.wide_point_clip_viewport_corner
>
> but it should be noted that these three tests expect that also points with
> a
> centre outside the clip volume will be partially drawn when they overlap
> with
> the clip volume because of being "wide". The OpenGL standard defines that
> these
> points should be clipped completely.
>

Is this a test problem or a problem with vrend_draw_bind_vertex_legacy?
Would the test pass if the points are completely clipped?


> GLES 3.1 does not completely implement GL_ARB_vertex_attrib_binding, so it
> may
> become necessary to add another flag that would indicate that only GLES
> style
> vertex_attrib_binding is available if virglrenderer would want to use this
> additional functionality. Namely
>   VertexAttribLPointer
> is not available, and a few query constants are missing from GLES 3.1 with
> respect to the extension.
>
> iSigned-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
> PS: obviously the bug in vrend_draw_bind_vertex_legacy that made these
> tests
> fail is still there. If someone wants to investigate: The point size is not
> properly set.

 src/vrend_renderer.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index 8cd9109..464560e 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -4203,7 +4203,8 @@ int vrend_renderer_init(struct vrend_if_cbs *cbs,
> uint32_t flags)
>
>     if (epoxy_has_gl_extension("GL_MESA_pack_invert"))
>        vrend_state.have_mesa_invert = true;
> -   if (gl_ver >= 43 ||
> epoxy_has_gl_extension("GL_ARB_vertex_attrib_binding"))
> +   if (gl_ver >= 43 || (gles && gl_ver >= 31) ||
> +       epoxy_has_gl_extension("GL_ARB_vertex_attrib_binding"))
>

Since GLES31 doesn't fully implement this, we can not advertise this.  You
can also do

   if (vrend_state.have_vertex_attrib_binding || (vrend_state.use_gles &&
vrend_state.gl_major_ver >= 3 &&  vrend_state.gl_minor_ver >= 1)
      vrend_draw_bind_vertex_binding(ctx, ctx->sub->ve);
   else
      vrend_draw_bind_vertex_legacy(ctx, ctx->sub->ve);

It's a little more verbose, but also more correct.


>        vrend_state.have_vertex_attrib_binding = true;
>     if (gl_ver >= 33 || epoxy_has_gl_extension("GL_ARB_sampler_objects"))
>        vrend_state.have_samplers = true;
> --
> 2.17.0
>
> _______________________________________________
> virglrenderer-devel mailing list
> virglrenderer-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/virglrenderer-devel/attachments/20180525/5e782cc0/attachment-0001.html>