[Mesa-dev] [PATCH] mesa: add glsl version query (v2)
Vadym Shovkoplias
vadym.shovkoplias at globallogic.com
Thu Feb 8 09:31:53 UTC 2018
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 <brianp at vmware.com> 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 <andriy.khulap at globallogic.com>
>> Signed-off-by: Vadym Shovkoplias <vadim.shovkoplias at gmail.com>
>> ---
>> 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
> mesa-dev at lists.freedesktop.org
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180208/fdee2dea/attachment.html>
More information about the mesa-dev
mailing list