Hi Brian, Thanks for review comments! We've just finished v3 of this patch. Regarding Piglit test I'll try to implement appropriate changes.
Regards, Vadym On Wed, Feb 7, 2018 at 5:58 PM, Brian Paul <[email protected]> wrote: > On 02/07/2018 03:01 AM, Vadym Shovkoplias wrote: > >> Add support for GL_NUM_SHADING_LANGUAGE_VERSIONS >> and glGetStringi for GL_SHADING_LANGUAGE_VERSION >> >> v2: >> - Combine similar functionality into >> _mesa_get_shading_language_version() function. >> - Cahnge GLSL version return mechanism. >> > > "Change" > > > > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104915 >> Signed-off-by: Andriy Khulap <[email protected]> >> Signed-off-by: Vadym Shovkoplias <[email protected]> >> --- >> src/mapi/glapi/gen/GL4x.xml | 1 + >> src/mesa/main/get.c | 4 +++ >> src/mesa/main/get_hash_params.py | 3 ++ >> src/mesa/main/getstring.c | 74 ++++++++++++++++++++++++++++++ >> ++++++++++ >> src/mesa/main/version.h | 5 +++ >> 5 files changed, 87 insertions(+) >> >> diff --git a/src/mapi/glapi/gen/GL4x.xml b/src/mapi/glapi/gen/GL4x.xml >> index cd2e3b831e..2116286b35 100644 >> --- a/src/mapi/glapi/gen/GL4x.xml >> +++ b/src/mapi/glapi/gen/GL4x.xml >> @@ -42,6 +42,7 @@ >> <category name="4.3"> >> <enum name="SHADER_STORAGE_BARRIER_BIT" >> value="0x2000" /> >> + <enum name="NUM_SHADING_LANGUAGE_VERSIONS" value="0x82E9" >> /> >> <enum name="MAX_COMBINED_SHADER_OUTPUT_RESOURCES" >> value="0x8F39" /> >> <enum name="SHADER_STORAGE_BUFFER" >> value="0x90D2"/> >> <enum name="SHADER_STORAGE_BUFFER_BINDING" >> value="0x90D3"/> >> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c >> index 516e8d174c..9a677a18d9 100644 >> --- a/src/mesa/main/get.c >> +++ b/src/mesa/main/get.c >> @@ -1084,6 +1084,10 @@ find_custom_value(struct gl_context *ctx, const >> struct value_desc *d, union valu >> v->value_int = 0; >> } >> break; >> + /* GL 4.3 */ >> + case GL_NUM_SHADING_LANGUAGE_VERSIONS: >> + v->value_int = _mesa_get_shading_language_version(ctx, -1, NULL); >> + break; >> /* GL_ARB_draw_indirect */ >> case GL_DRAW_INDIRECT_BUFFER_BINDING: >> v->value_int = ctx->DrawIndirectBuffer->Name; >> diff --git a/src/mesa/main/get_hash_params.py >> b/src/mesa/main/get_hash_params.py >> index df082af207..be716f6f6e 100644 >> --- a/src/mesa/main/get_hash_params.py >> +++ b/src/mesa/main/get_hash_params.py >> @@ -543,6 +543,9 @@ descriptor=[ >> # GL_ARB_texture_cube_map_array >> [ "TEXTURE_BINDING_CUBE_MAP_ARRAY_ARB", "LOC_CUSTOM, TYPE_INT, >> TEXTURE_CUBE_ARRAY_INDEX, >> extra_ARB_texture_cube_map_array_OES_texture_cube_map_array" >> ], >> + >> + # GL_NUM_SHADING_LANGUAGE_VERSIONS >> + [ "NUM_SHADING_LANGUAGE_VERSIONS", "LOC_CUSTOM, TYPE_INT, 0, >> NO_EXTRA" ], >> ]}, >> # Enums in OpenGL Core profile and ES 3.0 >> diff --git a/src/mesa/main/getstring.c b/src/mesa/main/getstring.c >> index 931f6a476c..1b4ec1193a 100644 >> --- a/src/mesa/main/getstring.c >> +++ b/src/mesa/main/getstring.c >> > > I think this new function should go into version.c (you already have the > prototype in version.h). > > > @@ -32,6 +32,7 @@ >> #include "extensions.h" >> #include "mtypes.h" >> #include "macros.h" >> +#include "version.h" >> /** >> * Return the string for a glGetString(GL_SHADING_LANGUAGE_VERSION) >> query. >> @@ -99,6 +100,68 @@ shading_language_version(struct gl_context *ctx) >> } >> +/** >> + * Get the i-th GLSL version string. If index=0, return the most recent >> + * supported version. >> + * \param ctx context to query >> + * \param index which version string to return, or -1 if none >> + * \param versionOut returns the vesrion string, NULL if index=-1 >> > > "version" And it doesn't look like we really return versionOut=NULL if > index=-1. I think you can just drop that part of the comment. > > > > + * \return total number of shading language versions. >> + */ >> +int >> +_mesa_get_shading_language_version(const struct gl_context *ctx, >> + int index, >> + char **versionOut) >> +{ >> + int n = 0; >> + >> +#define GLSL_VERSION(S) \ >> + if (n++ == index) \ >> + *versionOut = S >> + >> + /* GLSL core */ >> + if (ctx->Const.GLSLVersion >= 460) >> + GLSL_VERSION("460"); >> + if (ctx->Const.GLSLVersion >= 450) >> + GLSL_VERSION("450"); >> + if (ctx->Const.GLSLVersion >= 440) >> + GLSL_VERSION("440"); >> + if (ctx->Const.GLSLVersion >= 430) >> + GLSL_VERSION("430"); >> + if (ctx->Const.GLSLVersion >= 420) >> + GLSL_VERSION("420"); >> + if (ctx->Const.GLSLVersion >= 410) >> + GLSL_VERSION("410"); >> + if (ctx->Const.GLSLVersion >= 400) >> + GLSL_VERSION("400"); >> + if (ctx->Const.GLSLVersion >= 330) >> + GLSL_VERSION("330"); >> + if (ctx->Const.GLSLVersion >= 150) >> + GLSL_VERSION("150"); >> + if (ctx->Const.GLSLVersion >= 140) >> + GLSL_VERSION("140"); >> + if (ctx->Const.GLSLVersion >= 130) >> + GLSL_VERSION("130"); >> + if (ctx->Const.GLSLVersion >= 120) >> + GLSL_VERSION("120"); >> + /* GLSL es */ >> + if ((ctx->API == API_OPENGLES2 && ctx->Version >= 32) || >> + ctx->Extensions.ARB_ES3_2_compatibility) >> + GLSL_VERSION("320 es"); >> + if (_mesa_is_gles31(ctx) || ctx->Extensions.ARB_ES3_1_compatibility) >> + GLSL_VERSION("310 es"); >> + if (_mesa_is_gles3(ctx) || ctx->Extensions.ARB_ES3_compatibility) >> + GLSL_VERSION("300 es"); >> + if (ctx->API == API_OPENGLES2 || ctx->Extensions.ARB_ES2_compat >> ibility) >> + GLSL_VERSION("100"); >> + >> + /* XXX the spec also says to return the empty string for GLSL 1.10 */ >> > > I meant that you should implement this case too. So: > > if (ctx->Const.GLSLVersion >= 110) { > /* the GL spec says to return the empty string for GLSL 1.10 */ > GLSL_VERSION(""); > } > > +#undef GLSL_VERSION >> + >> + return n; >> +} >> + >> + >> /** >> * Query string-valued state. The return value should _not_ be freed by >> * the caller. >> @@ -186,6 +249,17 @@ _mesa_GetStringi(GLenum name, GLuint index) >> return (const GLubyte *) 0; >> } >> return _mesa_get_enabled_extension(ctx, index); >> + case GL_SHADING_LANGUAGE_VERSION: >> + { >> + char *version; >> + int num = _mesa_get_shading_language_version(ctx, index, >> &version); >> + if (index >= num) { >> + _mesa_error(ctx, GL_INVALID_VALUE, >> + "glGetStringi(GL_SHADING_LANGUAGE_VERSION, index=%d", >> index); >> > > Missing the closing parenthesis in the string. > > > + return (const GLubyte *) 0; >> + } >> + return (const GLubyte *) version; >> + } >> default: >> _mesa_error(ctx, GL_INVALID_ENUM, "glGetStringi"); >> return (const GLubyte *) 0; >> diff --git a/src/mesa/main/version.h b/src/mesa/main/version.h >> index 4cb5e5f0fa..adfec6f828 100644 >> --- a/src/mesa/main/version.h >> +++ b/src/mesa/main/version.h >> @@ -53,4 +53,9 @@ _mesa_get_driver_uuid(struct gl_context *ctx, GLint >> *uuid); >> extern void >> _mesa_get_device_uuid(struct gl_context *ctx, GLint *uuid); >> +extern int >> +_mesa_get_shading_language_version(const struct gl_context *ctx, >> + int index, >> + char **versionOut); >> + >> #endif /* VERSION_H */ >> >> > Looks good otherwise. Thanks for working on this. > > Are you going to write/modify a Piglit test to exercise these queries? > > -Brian > > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > -- Vadym Shovkoplias | Software engineer GlobalLogic P +x.xxx.xxx.xxxx M +3.8050.931.7304 S vadym.shovkoplias www.globallogic.com <http://www.globallogic.com/> http://www.globallogic.com/email_disclaimer.txt
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
