[Mesa-dev] [PATCH 2/3] mesa/st: use ESSL cap top enable gpu_shader5 (v2)

[Rob Clark]

On Thu, Mar 21, 2019 at 2:19 PM Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>
> On Thu, Mar 21, 2019 at 1:45 PM Rob Clark <robdclark at gmail.com> wrote:
> >
> > For GLES2+ contexts, enable EXT_gpu_shader5 if the driver exposes a
> > sufficiently high ESSL feature level, even if the GLSL feature level
> > isn't high enough.
> >
> > This allows drivers to support EXT_gpu_shader5 in GLES contexts before
> > they support all the additional features of ARB_gpu_shader5 in GL
> > contexts.
> >
> > We can also use this cap to enable ARB_ES3_1_compatibility.
>
> I believe this is no longer the case, so probably remove that line.
> Also, all the cool people talk about the OES_* variants of the exts...
> Doesn't really matter, of course. They're identical.
>
> >
> > Signed-off-by: Rob Clark <robdclark at gmail.com>
> > ---
> > This kinda morphed into also fixing OES_viewport_array (which should
> > depend on GLES 3.2)
> >
> >  src/mesa/state_tracker/st_extensions.c | 24 +++++++++++++++++-------
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
> > index c953bfd9f7a..7c3b98c1ba6 100644
> > --- a/src/mesa/state_tracker/st_extensions.c
> > +++ b/src/mesa/state_tracker/st_extensions.c
> > @@ -1056,6 +1056,8 @@ void st_init_extensions(struct pipe_screen *screen,
> >     consts->GLSLVersionCompat =
> >        screen->get_param(screen, PIPE_CAP_GLSL_FEATURE_LEVEL_COMPATIBILITY);
> >
> > +   const unsigned ESSLVersion =
> > +      screen->get_param(screen, PIPE_CAP_ESSL_FEATURE_LEVEL);
> >     const unsigned GLSLVersion =
> >        api == API_OPENGL_COMPAT ? consts->GLSLVersionCompat :
> >                                   consts->GLSLVersion;
> > @@ -1077,6 +1079,13 @@ void st_init_extensions(struct pipe_screen *screen,
> >
> >     consts->AllowGLSLCrossStageInterpolationMismatch = options->allow_glsl_cross_stage_interpolation_mismatch;
> >
> > +   /* Technically we are turning on the EXT_gpu_shader5 extension,
> > +    * ARB_gpu_shader5 does not exist in GLES, but this flag is what
> > +    * switches on EXT_gpu_shader5:
> > +    */
> > +   if (api == API_OPENGLES2 && ESSLVersion >= 320)
> > +      extensions->ARB_gpu_shader5 = GL_TRUE;
> > +
> >     if (GLSLVersion >= 400)
> >        extensions->ARB_gpu_shader5 = GL_TRUE;
> >     if (GLSLVersion >= 410)
> > @@ -1540,16 +1549,12 @@ void st_init_extensions(struct pipe_screen *screen,
> >        extensions->EXT_shader_integer_mix;
> >
> >     extensions->OES_texture_cube_map_array =
> > -      extensions->ARB_ES3_1_compatibility &&
> > +      (extensions->ARB_ES3_1_compatibility || ESSLVersion >= 310) &&
>
> It's a bit more work, but if you get all the drivers to properly
> expose this cap, you could make this *just* ESSLVersion >= x, instead
> of also relying on the ARB_* thing. Totally your call, just wanted to
> make the observation.
>
> >        extensions->OES_geometry_shader &&
> >        extensions->ARB_texture_cube_map_array;
> >
> > -   extensions->OES_viewport_array =
> > -      extensions->ARB_ES3_1_compatibility &&
> > -      extensions->OES_geometry_shader &&
> > -      extensions->ARB_viewport_array;
> > -
> > -   extensions->OES_primitive_bounding_box = extensions->ARB_ES3_1_compatibility;
> > +   extensions->OES_primitive_bounding_box =
> > +      extensions->ARB_ES3_1_compatibility || (ESSLVersion >= 310);
> >     consts->NoPrimitiveBoundingBoxOutput = true;
> >
> >     extensions->ANDROID_extension_pack_es31a =
> > @@ -1590,6 +1595,11 @@ void st_init_extensions(struct pipe_screen *screen,
> >        extensions->ARB_texture_stencil8 &&
> >        extensions->ARB_texture_multisample;
> >
> > +   extensions->OES_viewport_array =
> > +      (extensions->ARB_ES3_2_compatibility || ESSLVersion >= 320) &&
>
> Why is this getting bumped up to needing ES 3.2? Did you mean to leave
> this at ES3_1_compat || ESSL >= 310?
>
> "OpenGL ES 3.2, EXT_geometry_shader or OES_geometry_shader is
> required." (Note the "or". Otherwise it'd make no sense since ES 3.2
> includes geometry shaders as core functionality.)

yeah, moving it was intentional, but maybe based on mis-parsing the
spec dependencies and applying the 'C' order of operator precedence to
english.. :-P

BR,
-R

>
>
> > +      extensions->OES_geometry_shader &&
> > +      extensions->ARB_viewport_array;
> > +
> >     if (screen->get_param(screen, PIPE_CAP_CONSERVATIVE_RASTER_POST_SNAP_TRIANGLES) &&
> >         screen->get_param(screen, PIPE_CAP_CONSERVATIVE_RASTER_POST_SNAP_POINTS_LINES) &&
> >         screen->get_param(screen, PIPE_CAP_CONSERVATIVE_RASTER_POST_DEPTH_COVERAGE)) {
> > --
> > 2.20.1
> >