[virglrenderer-devel] [PATCH 2/3] initial feature extension support
Gert Wollny
gert.wollny at collabora.com
Mon Jul 23 07:18:28 UTC 2018
I was also thinking about this, because currently we have two code path
for gles and gl, and this makes it easy to forget to add the feature
test for one code path (In fact, just correcting the feature evaluation
for the gles host I've been able to get gles 3.1 on top of gles 3.1).
I'll send an RFC with my changes to document how the tests can be
joined.
See comments in-line
Am Montag, den 23.07.2018, 12:08 +1000 schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
>
> This adds a mechanism to detect supported GL features on the
> host side and use them to set the caps bits later.
> ---
> src/vrend_renderer.c | 60
> ++++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index 317024a..3643550 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -87,6 +87,26 @@ struct global_error_state {
> enum virgl_errors last_error;
> };
>
> +enum feature_name
> +{
> + FEAT_START = 0,
> + FEAT_TESSELLATION = 0,
> + FEAT_END,
> +};
> +
> +struct features {
> + int gl_ver;
> + int gles_ver;
> + const char *gl_ext;
> + const char *gles_ext;
I wouldn't distinguish between GLES and GL versions, just make it an
array of a reasonable size like MAX_EXT_NAMES=4:
const char *gles_ext[MAX_EXT_NAMES];
To avoid allocating the space for the array, one could also list the
extension names as space-delimited list and parse this when checking
for extensions, but this is probably not worth the trouble.
> +};
> +
I'd suggest to initialize add a define
#define NOT_AVAILABLE INT_MAX
and then (with the above) the definitions become
static const struct features features_list[] = {
[FEAT_TESSELLATION] = { 400, -1, {"GL_ARB_tessellation_shader", NULL}}
[FEAT_COPY_IMAGE] = { 430, -1, {"GL_ARB_copy_image",
"GL_OES_copy_image",
"GL_EXT_copy_image", NULL}}
};
+
> +#define HAVE_FEATURE(feature) (vrend_state.features[feature] ==
> true)
> +
> struct global_renderer_state {
> int gl_major_ver;
> int gl_minor_ver;
> @@ -99,6 +119,7 @@ struct global_renderer_state {
> bool use_gles;
> bool use_core_profile;
>
> + bool features[FEAT_END];
> bool have_debug_cb;
> bool have_mesa_invert;
> bool have_samplers;
> @@ -118,7 +139,6 @@ struct global_renderer_state {
> bool have_texture_buffer_range;
> bool have_polygon_offset_clamp;
> bool have_texture_storage;
> - bool have_tessellation;
> bool have_texture_view;
> bool have_copy_image;
> bool have_ssbo;
> @@ -2565,7 +2585,7 @@ int vrend_create_shader(struct vrend_context
> *ctx,
> if (type > PIPE_SHADER_TESS_EVAL)
> return EINVAL;
>
> - if (!vrend_state.have_tessellation &&
> + if (!HAVE_FEATURE(FEAT_TESSELLATION) &&
> (type == PIPE_SHADER_TESS_CTRL ||
> type == PIPE_SHADER_TESS_EVAL))
> return EINVAL;
> @@ -3443,7 +3463,7 @@ void vrend_draw_vbo(struct vrend_context *ctx,
> else
> glBindBuffer(GL_DRAW_INDIRECT_BUFFER, 0);
>
> - if (info->vertices_per_patch && vrend_state.have_tessellation)
> + if (info->vertices_per_patch && HAVE_FEATURE(FEAT_TESSELLATION))
> glPatchParameteri(GL_PATCH_VERTICES, info-
> >vertices_per_patch);
>
> /* set the vertex state up now on a delay */
> @@ -4526,6 +4546,25 @@ static void vrend_debug_cb(UNUSED GLenum
> source, GLenum type, UNUSED GLuint id,
> fprintf(stderr, "ERROR: %s\n", message);
> }
>
> +static void vrend_features_init(bool gles, int gl_ver)
Here I would pass in gl_ver and gles_ver, depending on the host context
one would be zero, and the other the actual value like
if (vrend_state.use_gles) {
gles_ver = epoxy_gl_version();
gl_ver = 0;
} else {
gles_ver = 0;
gl_ver = epoxy_gl_version();
}
With this and the extension array evaluation the caps would become
static void vrend_features_init(int gles_ver, int gl_ver)
{
for(feat_id...
if (gles_ver >= features_list[feat_id].gles_ver) ||
gl_ver >= features_list[feat_id].gl_ver)
vrend_state.features[feat_id] = true;
else {
for (int i = 0; i < MAX_EXT_NAMES &&
features_list[feat_id].gl_ext[i]; ++i)
if (epoxy_has_gl_extension(features_list[feat_id].gl_ext[i])) {
vrend_state.features[feat_id] = true;
break;
}
}
...} // endfor
}
> +{
> + for (enum feature_name feat_id = FEAT_START; feat_id < FEAT_END;
> feat_id++) {
> + if (gles) {
> + if (features_list[feat_id].gles_ver != -1 && gl_ver >=
> features_list[feat_id].gles_ver)
> + vrend_state.features[feat_id] = true;
> + } else {
> + if (features_list[feat_id].gl_ver != -1 && gl_ver >=
> features_list[feat_id].gl_ver)
> + vrend_state.features[feat_id] = true;
> + }
> + if (vrend_state.features[feat_id])
> + continue;
> +
> + if (!gles)
> + if (epoxy_has_gl_extension(features_list[feat_id].gl_ext))
> + vrend_state.features[feat_id] = true;
> + }
> +}
> +
> int vrend_renderer_init(struct vrend_if_cbs *cbs, uint32_t flags)
> {
> bool gles;
> @@ -4578,6 +4617,8 @@ int vrend_renderer_init(struct vrend_if_cbs
> *cbs, uint32_t flags)
> fprintf(stderr, "gl_version %d - compat profile\n", gl_ver);
> }
>
> + vrend_features_init(gles, gl_ver);
> +
> glGetIntegerv(GL_MAX_DRAW_BUFFERS, (GLint *)
> &vrend_state.max_draw_buffers);
>
> if (epoxy_has_gl_extension("GL_ARB_robustness")) {
> @@ -4629,9 +4670,6 @@ int vrend_renderer_init(struct vrend_if_cbs
> *cbs, uint32_t flags)
> if (gl_ver >= 40 ||
> epoxy_has_gl_extension("GL_ARB_sample_shading"))
> vrend_state.have_sample_shading = true;
>
> - if (gl_ver >= 40 ||
> epoxy_has_gl_extension("GL_ARB_tessellation_shader"))
> - vrend_state.have_tessellation = true;
> -
> if (gl_ver >= 43 ||
> epoxy_has_gl_extension("GL_ARB_texture_buffer_range"))
> vrend_state.have_texture_buffer_range = true;
>
> @@ -6220,7 +6258,7 @@ void vrend_set_min_samples(struct vrend_context
> *ctx, unsigned min_samples)
>
> void vrend_set_tess_state(UNUSED struct vrend_context *ctx, const
> float tess_factors[6])
> {
> - if (vrend_state.have_tessellation) {
> + if (HAVE_FEATURE(FEAT_TESSELLATION)) {
> glPatchParameterfv(GL_PATCH_DEFAULT_OUTER_LEVEL,
> tess_factors);
> glPatchParameterfv(GL_PATCH_DEFAULT_INNER_LEVEL,
> &tess_factors[4]);
> }
> @@ -7583,6 +7621,9 @@ void vrend_renderer_fill_caps(uint32_t set,
> uint32_t version,
> caps->v1.bset.texture_multisample = 1;
> }
>
> + if (HAVE_FEATURE(FEAT_TESSELLATION))
> + caps->v1.bset.has_tessellation_shaders = 1;
> +
> if (gl_ver >= 40) {
> caps->v1.bset.indep_blend_func = 1;
> caps->v1.bset.cube_map_array = 1;
> @@ -7590,7 +7631,6 @@ void vrend_renderer_fill_caps(uint32_t set,
> uint32_t version,
> caps->v1.bset.has_indirect_draw = 1;
> caps->v1.bset.has_sample_shading = 1;
> caps->v1.bset.has_fp64 = 1;
> - caps->v1.bset.has_tessellation_shaders = 1;
> } else {
> if (epoxy_has_gl_extension("GL_ARB_draw_buffers_blend"))
> caps->v1.bset.indep_blend_func = 1;
> @@ -7606,8 +7646,6 @@ void vrend_renderer_fill_caps(uint32_t set,
> uint32_t version,
> if (epoxy_has_gl_extension("GL_ARB_gpu_shader_fp64") &&
> epoxy_has_gl_extension("GL_ARB_gpu_shader5"))
> caps->v1.bset.has_fp64 = 1;
> - if (epoxy_has_gl_extension("GL_ARB_tessellation_shader"))
> - caps->v1.bset.has_tessellation_shaders = 1;
> }
>
> if (gl_ver >= 42) {
> @@ -7716,7 +7754,7 @@ void vrend_renderer_fill_caps(uint32_t set,
> uint32_t version,
> glGetIntegerv(GL_MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS,
> (GLint*)&caps->v2.max_geom_total_output_components);
> }
>
> - if (epoxy_has_gl_extension("GL_ARB_tessellation_shader") ||
> gl_ver >= 40) {
> + if (HAVE_FEATURE(FEAT_TESSELLATION)) {
> glGetIntegerv(GL_MAX_TESS_PATCH_COMPONENTS, &max);
> caps->v2.max_shader_patch_varyings = max / 4;
> } else
More information about the virglrenderer-devel
mailing list