Re: [Mesa-dev] [PATCH 1/2] mesa: fix up GLSL version when computing GL version
On 04/27/2015 07:23 AM, Ilia Mirkin wrote: Some required extension might be missing, and we may expose a GLSL version that does not match up with the GL version. Fix that up when computing the GL version. Signed-off-by: Ilia Mirkin --- src/mesa/main/version.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c index a65ace0..bf0a1cf 100644 --- a/src/mesa/main/version.c +++ b/src/mesa/main/version.c @@ -207,7 +207,7 @@ _mesa_override_glsl_version(struct gl_constants *consts) */ static GLuint compute_version(const struct gl_extensions *extensions, -const struct gl_constants *consts, gl_api api) +struct gl_constants *consts, gl_api api) { GLuint major, minor, version; @@ -330,38 +330,47 @@ compute_version(const struct gl_extensions *extensions, if (ver_4_2) { major = 4; minor = 2; + consts->GLSLVersion = 420; } else if (ver_4_1) { major = 4; minor = 1; + consts->GLSLVersion = 410; } else if (ver_4_0) { major = 4; minor = 0; + consts->GLSLVersion = 400; } else if (ver_3_3) { major = 3; minor = 3; + consts->GLSLVersion = 330; } else if (ver_3_2) { major = 3; minor = 2; + consts->GLSLVersion = 150; } else if (ver_3_1) { major = 3; minor = 1; + consts->GLSLVersion = 140; } else if (ver_3_0) { major = 3; minor = 0; + consts->GLSLVersion = 130; } else if (ver_2_1) { major = 2; minor = 1; + consts->GLSLVersion = 120; } else if (ver_2_0) { major = 2; minor = 0; + consts->GLSLVersion = 110; } else if (ver_1_5) { major = 1; This looks OK, but another approach might be to have a new function which returns the GLSL version for a given GL version (could be encoded in a small table). Then, use that function to clamp/set consts->GLSLVersion. That way we can keep the nice const-ness of the compute_version() parameters. Otherwise, we should at least update the comments to indicate that the gl_constants can change. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: fix up GLSL version when computing GL version
On Mon, Apr 27, 2015 at 9:38 AM, Brian Paul wrote: > On 04/27/2015 07:23 AM, Ilia Mirkin wrote: >> >> Some required extension might be missing, and we may expose a GLSL >> version that does not match up with the GL version. Fix that up when >> computing the GL version. >> >> Signed-off-by: Ilia Mirkin >> --- >> src/mesa/main/version.c | 11 ++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c >> index a65ace0..bf0a1cf 100644 >> --- a/src/mesa/main/version.c >> +++ b/src/mesa/main/version.c >> @@ -207,7 +207,7 @@ _mesa_override_glsl_version(struct gl_constants >> *consts) >>*/ >> static GLuint >> compute_version(const struct gl_extensions *extensions, >> -const struct gl_constants *consts, gl_api api) >> +struct gl_constants *consts, gl_api api) >> { >> GLuint major, minor, version; >> >> @@ -330,38 +330,47 @@ compute_version(const struct gl_extensions >> *extensions, >> if (ver_4_2) { >> major = 4; >> minor = 2; >> + consts->GLSLVersion = 420; >> } >> else if (ver_4_1) { >> major = 4; >> minor = 1; >> + consts->GLSLVersion = 410; >> } >> else if (ver_4_0) { >> major = 4; >> minor = 0; >> + consts->GLSLVersion = 400; >> } >> else if (ver_3_3) { >> major = 3; >> minor = 3; >> + consts->GLSLVersion = 330; >> } >> else if (ver_3_2) { >> major = 3; >> minor = 2; >> + consts->GLSLVersion = 150; >> } >> else if (ver_3_1) { >> major = 3; >> minor = 1; >> + consts->GLSLVersion = 140; >> } >> else if (ver_3_0) { >> major = 3; >> minor = 0; >> + consts->GLSLVersion = 130; >> } >> else if (ver_2_1) { >> major = 2; >> minor = 1; >> + consts->GLSLVersion = 120; >> } >> else if (ver_2_0) { >> major = 2; >> minor = 0; >> + consts->GLSLVersion = 110; >> } >> else if (ver_1_5) { >> major = 1; >> > > This looks OK, but another approach might be to have a new function which > returns the GLSL version for a given GL version (could be encoded in a small > table). Then, use that function to clamp/set consts->GLSLVersion. That way > we can keep the nice const-ness of the compute_version() parameters. Where do you envision the clamping be done? > > Otherwise, we should at least update the comments to indicate that the > gl_constants can change. > > -Brian > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: fix up GLSL version when computing GL version
On 04/27/2015 07:39 AM, Ilia Mirkin wrote: On Mon, Apr 27, 2015 at 9:38 AM, Brian Paul wrote: On 04/27/2015 07:23 AM, Ilia Mirkin wrote: Some required extension might be missing, and we may expose a GLSL version that does not match up with the GL version. Fix that up when computing the GL version. Signed-off-by: Ilia Mirkin --- src/mesa/main/version.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c index a65ace0..bf0a1cf 100644 --- a/src/mesa/main/version.c +++ b/src/mesa/main/version.c @@ -207,7 +207,7 @@ _mesa_override_glsl_version(struct gl_constants *consts) */ static GLuint compute_version(const struct gl_extensions *extensions, -const struct gl_constants *consts, gl_api api) +struct gl_constants *consts, gl_api api) { GLuint major, minor, version; @@ -330,38 +330,47 @@ compute_version(const struct gl_extensions *extensions, if (ver_4_2) { major = 4; minor = 2; + consts->GLSLVersion = 420; } else if (ver_4_1) { major = 4; minor = 1; + consts->GLSLVersion = 410; } else if (ver_4_0) { major = 4; minor = 0; + consts->GLSLVersion = 400; } else if (ver_3_3) { major = 3; minor = 3; + consts->GLSLVersion = 330; } else if (ver_3_2) { major = 3; minor = 2; + consts->GLSLVersion = 150; } else if (ver_3_1) { major = 3; minor = 1; + consts->GLSLVersion = 140; } else if (ver_3_0) { major = 3; minor = 0; + consts->GLSLVersion = 130; } else if (ver_2_1) { major = 2; minor = 1; + consts->GLSLVersion = 120; } else if (ver_2_0) { major = 2; minor = 0; + consts->GLSLVersion = 110; } else if (ver_1_5) { major = 1; This looks OK, but another approach might be to have a new function which returns the GLSL version for a given GL version (could be encoded in a small table). Then, use that function to clamp/set consts->GLSLVersion. That way we can keep the nice const-ness of the compute_version() parameters. Where do you envision the clamping be done? How about _mesa_compute_version()? -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: fix up GLSL version when computing GL version
On Mon, Apr 27, 2015 at 9:58 AM, Brian Paul wrote: > On 04/27/2015 07:39 AM, Ilia Mirkin wrote: >> >> On Mon, Apr 27, 2015 at 9:38 AM, Brian Paul wrote: >>> >>> On 04/27/2015 07:23 AM, Ilia Mirkin wrote: Some required extension might be missing, and we may expose a GLSL version that does not match up with the GL version. Fix that up when computing the GL version. Signed-off-by: Ilia Mirkin --- src/mesa/main/version.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c index a65ace0..bf0a1cf 100644 --- a/src/mesa/main/version.c +++ b/src/mesa/main/version.c @@ -207,7 +207,7 @@ _mesa_override_glsl_version(struct gl_constants *consts) */ static GLuint compute_version(const struct gl_extensions *extensions, -const struct gl_constants *consts, gl_api api) +struct gl_constants *consts, gl_api api) { GLuint major, minor, version; @@ -330,38 +330,47 @@ compute_version(const struct gl_extensions *extensions, if (ver_4_2) { major = 4; minor = 2; + consts->GLSLVersion = 420; } else if (ver_4_1) { major = 4; minor = 1; + consts->GLSLVersion = 410; } else if (ver_4_0) { major = 4; minor = 0; + consts->GLSLVersion = 400; } else if (ver_3_3) { major = 3; minor = 3; + consts->GLSLVersion = 330; } else if (ver_3_2) { major = 3; minor = 2; + consts->GLSLVersion = 150; } else if (ver_3_1) { major = 3; minor = 1; + consts->GLSLVersion = 140; } else if (ver_3_0) { major = 3; minor = 0; + consts->GLSLVersion = 130; } else if (ver_2_1) { major = 2; minor = 1; + consts->GLSLVersion = 120; } else if (ver_2_0) { major = 2; minor = 0; + consts->GLSLVersion = 110; } else if (ver_1_5) { major = 1; >>> >>> This looks OK, but another approach might be to have a new function which >>> returns the GLSL version for a given GL version (could be encoded in a >>> small >>> table). Then, use that function to clamp/set consts->GLSLVersion. That >>> way >>> we can keep the nice const-ness of the compute_version() parameters. >> >> >> Where do you envision the clamping be done? > > > How about _mesa_compute_version()? ... which is the only caller of compute_version(). You just want it split up into a separate switch statement or something? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: fix up GLSL version when computing GL version
On 04/27/2015 08:05 AM, Ilia Mirkin wrote: On Mon, Apr 27, 2015 at 9:58 AM, Brian Paul wrote: On 04/27/2015 07:39 AM, Ilia Mirkin wrote: On Mon, Apr 27, 2015 at 9:38 AM, Brian Paul wrote: On 04/27/2015 07:23 AM, Ilia Mirkin wrote: Some required extension might be missing, and we may expose a GLSL version that does not match up with the GL version. Fix that up when computing the GL version. Signed-off-by: Ilia Mirkin --- src/mesa/main/version.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c index a65ace0..bf0a1cf 100644 --- a/src/mesa/main/version.c +++ b/src/mesa/main/version.c @@ -207,7 +207,7 @@ _mesa_override_glsl_version(struct gl_constants *consts) */ static GLuint compute_version(const struct gl_extensions *extensions, -const struct gl_constants *consts, gl_api api) +struct gl_constants *consts, gl_api api) { GLuint major, minor, version; @@ -330,38 +330,47 @@ compute_version(const struct gl_extensions *extensions, if (ver_4_2) { major = 4; minor = 2; + consts->GLSLVersion = 420; } else if (ver_4_1) { major = 4; minor = 1; + consts->GLSLVersion = 410; } else if (ver_4_0) { major = 4; minor = 0; + consts->GLSLVersion = 400; } else if (ver_3_3) { major = 3; minor = 3; + consts->GLSLVersion = 330; } else if (ver_3_2) { major = 3; minor = 2; + consts->GLSLVersion = 150; } else if (ver_3_1) { major = 3; minor = 1; + consts->GLSLVersion = 140; } else if (ver_3_0) { major = 3; minor = 0; + consts->GLSLVersion = 130; } else if (ver_2_1) { major = 2; minor = 1; + consts->GLSLVersion = 120; } else if (ver_2_0) { major = 2; minor = 0; + consts->GLSLVersion = 110; } else if (ver_1_5) { major = 1; This looks OK, but another approach might be to have a new function which returns the GLSL version for a given GL version (could be encoded in a small table). Then, use that function to clamp/set consts->GLSLVersion. That way we can keep the nice const-ness of the compute_version() parameters. Where do you envision the clamping be done? How about _mesa_compute_version()? ... which is the only caller of compute_version(). Via _mesa_get_version() which also called from st_manager.c which seems to be unconcerned with GLSL stuff. You just want it split up into a separate switch statement or something? As I said, I thought a table would work, but a switch would be fine too. If you feel this is too much work, it's not that big of deal. I just thought it would be cleaner. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] mesa: fix up GLSL version when computing GL version
On Mon, Apr 27, 2015 at 10:22 AM, Brian Paul wrote: > On 04/27/2015 08:05 AM, Ilia Mirkin wrote: >> >> On Mon, Apr 27, 2015 at 9:58 AM, Brian Paul wrote: >>> >>> On 04/27/2015 07:39 AM, Ilia Mirkin wrote: On Mon, Apr 27, 2015 at 9:38 AM, Brian Paul wrote: > > > On 04/27/2015 07:23 AM, Ilia Mirkin wrote: >> >> >> >> Some required extension might be missing, and we may expose a GLSL >> version that does not match up with the GL version. Fix that up when >> computing the GL version. >> >> Signed-off-by: Ilia Mirkin >> --- >> src/mesa/main/version.c | 11 ++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c >> index a65ace0..bf0a1cf 100644 >> --- a/src/mesa/main/version.c >> +++ b/src/mesa/main/version.c >> @@ -207,7 +207,7 @@ _mesa_override_glsl_version(struct gl_constants >> *consts) >> */ >> static GLuint >> compute_version(const struct gl_extensions *extensions, >> -const struct gl_constants *consts, gl_api api) >> +struct gl_constants *consts, gl_api api) >> { >>GLuint major, minor, version; >> >> @@ -330,38 +330,47 @@ compute_version(const struct gl_extensions >> *extensions, >>if (ver_4_2) { >> major = 4; >> minor = 2; >> + consts->GLSLVersion = 420; >>} >>else if (ver_4_1) { >> major = 4; >> minor = 1; >> + consts->GLSLVersion = 410; >>} >>else if (ver_4_0) { >> major = 4; >> minor = 0; >> + consts->GLSLVersion = 400; >>} >>else if (ver_3_3) { >> major = 3; >> minor = 3; >> + consts->GLSLVersion = 330; >>} >>else if (ver_3_2) { >> major = 3; >> minor = 2; >> + consts->GLSLVersion = 150; >>} >>else if (ver_3_1) { >> major = 3; >> minor = 1; >> + consts->GLSLVersion = 140; >>} >>else if (ver_3_0) { >> major = 3; >> minor = 0; >> + consts->GLSLVersion = 130; >>} >>else if (ver_2_1) { >> major = 2; >> minor = 1; >> + consts->GLSLVersion = 120; >>} >>else if (ver_2_0) { >> major = 2; >> minor = 0; >> + consts->GLSLVersion = 110; >>} >>else if (ver_1_5) { >> major = 1; >> > > This looks OK, but another approach might be to have a new function > which > returns the GLSL version for a given GL version (could be encoded in a > small > table). Then, use that function to clamp/set consts->GLSLVersion. > That > way > we can keep the nice const-ness of the compute_version() parameters. Where do you envision the clamping be done? >>> >>> >>> >>> How about _mesa_compute_version()? >> >> >> ... which is the only caller of compute_version(). > > > Via _mesa_get_version() which also called from st_manager.c which seems to > be unconcerned with GLSL stuff. > > >> You just want it >> split up into a separate switch statement or something? > > > As I said, I thought a table would work, but a switch would be fine too. > > If you feel this is too much work, it's not that big of deal. I just > thought it would be cleaner. Nope, not too much work, just trying to tease out what you want me to do. I got confused between _mesa_compute_version and _mesa_get_version. It does seem like a switch is simpler than a table though. (The table would have to be indexed by GL version, which is pretty sparse if you do versions like 20, 33 and 40.) -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev