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

[Gurchetan Singh]

Reviewed-by: Gurchetan Singh <gurchetansingh at chromium.org>
On Mon, May 28, 2018 at 2:13 AM Gert Wollny <gert.wollny at collabora.com> wrote:
>
> On GLES >= 3.1 all the functionality that is currently relevant for
> attribute binding in virglrenderer is available. However, GLES 3.1 does
> not fully implement GL_ARB_vertex_attrib_binding. Therefore, rename the
> flag "have_vertex_attrib_binding" to "have_gles31_vertex_attrib_binding",
> and set it when the host implements GL_ARB_vertex_attrib_binding, is
> GL >= 4.3 or GLES >= 3.1. As a result with these configuration the new
> code path vrend_draw_bind_vertex_binding is used.
>
> Fixes on GLES 3.1 hosts:
>   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 GLES 3.1 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" (i.e. when moving towards the clip
> volume boundary a wide point would "naturally" move out). The OpenGL standard
> defines that points should be clipped the moment the centre is outside the
> clip volume (i.e. points will "pop" in and out of the view when moving across
> the border). Hence, for a host implementation following the standard to the
> letter these tests should actually fail.
>
> v2: - rename have_vertex_attrib_binding to have_gles31_vertex_attrib_binding
>     - clarify the commit message describing the wide point clipping problem
>
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
>  src/vrend_renderer.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index cc69257..8cbb9e0 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -112,7 +112,7 @@ struct global_renderer_state {
>     bool have_nv_prim_restart;
>     bool have_gl_prim_restart;
>     bool have_bit_encoding;
> -   bool have_vertex_attrib_binding;
> +   bool have_gles31_vertex_attrib_binding;
>     bool have_tf2;
>     bool have_stencil_texturing;
>     bool have_sample_shading;
> @@ -932,7 +932,7 @@ static struct vrend_linked_shader_program *add_shader_program(struct vrend_conte
>     } else
>        sprog->dual_src_linked = false;
>
> -   if (vrend_state.have_vertex_attrib_binding) {
> +   if (vrend_state.have_gles31_vertex_attrib_binding) {
>        uint32_t mask = vs->sel->sinfo.attrib_input_mask;
>        while (mask) {
>           i = u_bit_scan(&mask);
> @@ -1035,7 +1035,7 @@ static struct vrend_linked_shader_program *add_shader_program(struct vrend_conte
>           sprog->const_locs[id] = NULL;
>     }
>
> -   if (!vrend_state.have_vertex_attrib_binding) {
> +   if (!vrend_state.have_gles31_vertex_attrib_binding) {
>        if (vs->sel->sinfo.num_inputs) {
>           sprog->attrib_locs = calloc(vs->sel->sinfo.num_inputs, sizeof(uint32_t));
>           if (sprog->attrib_locs) {
> @@ -1224,7 +1224,7 @@ static void vrend_destroy_vertex_elements_object(void *obj_ptr)
>  {
>     struct vrend_vertex_element_array *v = obj_ptr;
>
> -   if (vrend_state.have_vertex_attrib_binding) {
> +   if (vrend_state.have_gles31_vertex_attrib_binding) {
>        glDeleteVertexArrays(1, &v->id);
>     }
>     FREE(v);
> @@ -1837,7 +1837,7 @@ int vrend_create_vertex_elements_state(struct vrend_context *ctx,
>           v->elements[i].nr_chan = desc->nr_channels;
>     }
>
> -   if (vrend_state.have_vertex_attrib_binding) {
> +   if (vrend_state.have_gles31_vertex_attrib_binding) {
>        glGenVertexArrays(1, &v->id);
>        glBindVertexArray(v->id);
>        for (i = 0; i < num_elements; i++) {
> @@ -2715,7 +2715,7 @@ static void vrend_draw_bind_vertex_legacy(struct vrend_context *ctx,
>           continue;
>        }
>
> -      if (vrend_state.use_explicit_locations || vrend_state.have_vertex_attrib_binding) {
> +      if (vrend_state.use_explicit_locations || vrend_state.have_gles31_vertex_attrib_binding) {
>           loc = i;
>        } else {
>           if (ctx->sub->prog->attrib_locs) {
> @@ -3064,7 +3064,7 @@ void vrend_draw_vbo(struct vrend_context *ctx,
>        }
>     }
>
> -   if (vrend_state.have_vertex_attrib_binding)
> +   if (vrend_state.have_gles31_vertex_attrib_binding)
>        vrend_draw_bind_vertex_binding(ctx, ctx->sub->ve);
>     else
>        vrend_draw_bind_vertex_legacy(ctx, ctx->sub->ve);
> @@ -4233,8 +4233,9 @@ 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"))
> -      vrend_state.have_vertex_attrib_binding = true;
> +   if (gl_ver >= 43 ||  (gles && gl_ver >= 31) ||
> +       epoxy_has_gl_extension("GL_ARB_vertex_attrib_binding"))
> +      vrend_state.have_gles31_vertex_attrib_binding = true;
>     if (gl_ver >= 33 || epoxy_has_gl_extension("GL_ARB_sampler_objects"))
>        vrend_state.have_samplers = true;
>     if (gl_ver >= 33 || epoxy_has_gl_extension("GL_ARB_shader_bit_encoding"))
> @@ -4343,7 +4344,7 @@ static void vrend_destroy_sub_context(struct vrend_sub_context *sub)
>
>     glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
>
> -   if (!vrend_state.have_vertex_attrib_binding) {
> +   if (!vrend_state.have_gles31_vertex_attrib_binding) {
>        while (sub->enabled_attribs_bitmask) {
>           i = u_bit_scan(&sub->enabled_attribs_bitmask);
>
> @@ -7331,7 +7332,7 @@ void vrend_renderer_create_sub_ctx(struct vrend_context *ctx, int sub_ctx_id)
>
>     sub->sub_ctx_id = sub_ctx_id;
>
> -   if (!vrend_state.have_vertex_attrib_binding) {
> +   if (!vrend_state.have_gles31_vertex_attrib_binding) {
>        glGenVertexArrays(1, &sub->vaoid);
>        glBindVertexArray(sub->vaoid);
>     }
> --
> 2.16.1
>
> _______________________________________________
> virglrenderer-devel mailing list
> virglrenderer-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel