[virglrenderer-devel] [PATCH v4 4/7] vrend, caps: unify GLSL version evaluation (v4)

[Erik Faye-Lund]

On 01. aug. 2018 12:15, Gert Wollny wrote:
> Since the gles code path is now reduced to just setting the gles
> version, move all the glsl stuff into one place.
>
> v3: remove now unused parameter gl_ver
> v4: - rebase after image and compute shader patches landed
>      - add a check to not override the glsl version when on a
>        gles host by using the minimum gl host based version
>
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> Reviewed-by: Gurchetan Singh <gurchetansingh at chromium.org> (v3)
> ---
>   src/vrend_renderer.c | 28 ++++++++++++----------------
>   1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index 4d807ed..60fa6f5 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -7786,9 +7786,6 @@ static void vrend_renderer_fill_caps_common(union virgl_caps *caps)
>       */
>      caps->v1.bset.occlusion_query = 1;
>   
> -   /* Set an initial level here, will be updated later */
> -   caps->v1.glsl_level = 130;
> -
>      /* Set supported prims here as we now know what shaders we support. */
>      caps->v1.prim_mask = (1 << PIPE_PRIM_POINTS) | (1 << PIPE_PRIM_LINES) |
>                           (1 << PIPE_PRIM_LINE_STRIP) | (1 << PIPE_PRIM_LINE_LOOP) |
> @@ -7846,20 +7843,14 @@ static void vrend_renderer_fill_caps_v2_common(union virgl_caps *caps)
>      glGetIntegerv(GL_UNIFORM_BUFFER_OFFSET_ALIGNMENT, (GLint*)&caps->v2.uniform_buffer_offset_alignment);
>   }
>   
> -static void vrend_renderer_fill_caps_gles(UNUSED bool fill_capset2, int gles_ver,
> -                                          union virgl_caps *caps)
> +static void vrend_fill_caps_glsl_version(int gl_ver, int gles_ver,
> +					  union virgl_caps *caps)
>   {
>      if (gles_ver >= 30) {
>         caps->v1.glsl_level = 130;
>      } else {
>         caps->v1.glsl_level = 120;
>      }
> -}
> -
> -static void vrend_renderer_fill_caps_gl(bool fill_capset2, int gl_ver,
> -					  union virgl_caps *caps)
> -{
> -   GLfloat range[2];
>   
>      if (gl_ver == 31)
>         caps->v1.glsl_level = 140;
> @@ -7875,6 +7866,13 @@ static void vrend_renderer_fill_caps_gl(bool fill_capset2, int gl_ver,
>         caps->v1.glsl_level = 420;
>      else if (gl_ver >= 43)
>         caps->v1.glsl_level = 430;
> +   else if (gl_ver > 0)
> +      caps->v1.glsl_level = 130;

Nit: This reads strange compared to the current code. Why not do this 
instead?

---8<---
-   if (gl_ver == 31)
+   if (gl_ver < 31)
+      caps->v1.glsl_level = 130;
+   else if (gl_ver == 31)
        caps->v1.glsl_level = 140;
     else if (gl_ver == 32)
        caps->v1.glsl_level = 150;
---8<---

This keeps the list sorted from most pessimistic to most optimistic, 
which seems reasonable to me. Dunno... either way:

Reviewed-by: Erik Faye-Lund <erik.faye-lund at collabora.com>

> +}
> +
> +static void vrend_renderer_fill_caps_gl(bool fill_capset2, union virgl_caps *caps)
> +{
> +   GLfloat range[2];
>   
>      if (!vrend_state.use_core_profile) {
>         caps->v1.bset.poly_stipple = 1;
> @@ -7929,12 +7927,10 @@ void vrend_renderer_fill_caps(uint32_t set, UNUSED uint32_t version,
>      }
>   
>      vrend_renderer_fill_caps_common(caps);
> +   vrend_fill_caps_glsl_version(gl_ver, gles_ver, caps);
>   
> -   /* GLES has it's own path */
> -   if (vrend_state.use_gles) {
> -      vrend_renderer_fill_caps_gles(fill_capset2, gles_ver, caps);
> -   } else
> -      vrend_renderer_fill_caps_gl(fill_capset2, gl_ver, caps);
> +   if (!vrend_state.use_gles)
> +      vrend_renderer_fill_caps_gl(fill_capset2, caps);
>   
>      if (caps->v1.glsl_level >= 150) {
>         caps->v1.prim_mask |= (1 << PIPE_PRIM_LINES_ADJACENCY) |