<div dir="ltr">On 29 July 2013 11:17, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 07/28/2013 11:03 PM, Paul Berry wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+/**<br>
+ * Checks if the context supports geometry shaders.<br>
+ */<br>
+static inline GLboolean<br>
+_mesa_has_geometry_shaders(const struct gl_context *ctx)<br>
+{<br>
+ return _mesa_is_desktop_gl(ctx) &&<br>
+ (ctx->Version >= 32 || ctx->Extensions.ARB_geometry_shader4);<br>
+}<br>
</blockquote></div></div><div><div class="h5">
<br></div></div>
I might change this to:<br>
<br>
return _mesa_is_desktop_gl(ctx) &&<br>
(ctx->Const.GLSLVersion >= 150 || ctx->Extensions.ARB_geometry_<u></u>shader4);<div><div class="h5"><br></div></div></blockquote><div><br></div><div>I have a minor preference for keeping this as is, since it's conceivable that we might one day want to support GLSL 1.50 on some platforms that don't support GL 3.2 (much as Chris Forbes is currently doing to support GLSL 1.30 on Gen4-5). The places where _mesa_has_geometry_shaders() is used are for enabling and disabling API functionality (e.g. determining whether LINES_ADJACENCY is a valid primitive mode, or whether glFramebufferTexture() is allowed to be called), and I think that in this hypothetical platform that supports GLSL 1.50 but not GL 3.2, those pieces of functionality should be disabled. But I admit I'm straying pretty far into thought experiment territory at this point.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+<br>
#ifdef __cplusplus<br>
}<br>
#endif<br>
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c<br>
index a29f1ab..f0f59a6 100644<br>
--- a/src/mesa/main/fbobject.c<br>
+++ b/src/mesa/main/fbobject.c<br>
@@ -2472,7 +2472,7 @@ _mesa_FramebufferTexture(<u></u>GLenum target, GLenum attachment,<br>
{<br>
GET_CURRENT_CONTEXT(ctx);<br>
<br>
- if (ctx->Version >= 32 || ctx->Extensions.ARB_geometry_<u></u>shader4) {<br>
+ if (_mesa_has_geometry_shaders(<u></u>ctx)) {<br>
framebuffer_texture(ctx, "Layer", target, attachment, 0, texture,<br>
level, 0, GL_TRUE);<br>
} else {<br>
diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c<br>
index 0b33fa4..09b008a 100644<br>
--- a/src/mesa/main/get.c<br>
+++ b/src/mesa/main/get.c<br>
@@ -989,7 +989,7 @@ check_extra(struct gl_context *ctx, const char *func, const struct value_desc *d<br>
case EXTRA_EXT_UBO_GS4:<br>
api_check = GL_TRUE;<br>
api_found = (ctx->Extensions.ARB_uniform_<u></u>buffer_object &&<br>
- ctx->Extensions.ARB_geometry_<u></u>shader4);<br>
+ _mesa_has_geometry_shaders(<u></u>ctx));<br>
break;<br>
case EXTRA_END:<br>
break;<br>
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h<br>
index aba7d84..efa2d39 100644<br>
--- a/src/mesa/main/mtypes.h<br>
+++ b/src/mesa/main/mtypes.h<br>
@@ -2978,6 +2978,14 @@ struct gl_constants<br>
GLint MaxColorTextureSamples;<br>
GLint MaxDepthTextureSamples;<br>
GLint MaxIntegerSamples;<br>
+<br>
+ /**<br>
+ * True if the implementation supports GLSL 1.50 style geometry shaders.<br>
+ * This boolean is distinct from gl_extensions::ARB_geometry_<u></u>shader4 so<br>
+ * that we can expose GLSL 1.50 (and GL 3.2) functionality without exposing<br>
+ * {ARB,EXT}_geometry_shader4.<br>
+ */<br>
+ GLboolean GeometryShaders150;<br>
};<br>
</blockquote>
<br></div></div>
I don't really like this new flag. In my mind, ctx->Const.GLSLVersion >= 150 is sufficient, since I believe geometry shaders are required to expose 1.50.<br>
<br>
ctx->Const.GLSLVersion is already used to compute the GL version.<div><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c<br>
index 9c8af87..e8303c8 100644<br>
--- a/src/mesa/main/shaderapi.c<br>
+++ b/src/mesa/main/shaderapi.c<br>
@@ -177,7 +177,7 @@ validate_shader_target(const struct gl_context *ctx, GLenum type)<br>
case GL_VERTEX_SHADER:<br>
return ctx->Extensions.ARB_vertex_<u></u>shader;<br>
case GL_GEOMETRY_SHADER_ARB:<br>
- return _mesa_is_desktop_gl(ctx) && ctx->Extensions.ARB_geometry_<u></u>shader4;<br>
+ return _mesa_has_geometry_shaders(<u></u>ctx);<br>
default:<br>
return false;<br>
}<br>
@@ -476,8 +476,7 @@ get_programiv(struct gl_context *ctx, GLuint program, GLenum pname, GLint *param<br>
<br>
/* Are geometry shaders available in this context?<br>
*/<br>
- const bool has_gs =<br>
- _mesa_is_desktop_gl(ctx) && ctx->Extensions.ARB_geometry_<u></u>shader4;<br>
+ const bool has_gs = _mesa_has_geometry_shaders(<u></u>ctx);<br>
<br>
/* Are uniform buffer objects available in this context?<br>
*/<br>
diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c<br>
index ab9b14c..0e836cc 100644<br>
--- a/src/mesa/main/version.c<br>
+++ b/src/mesa/main/version.c<br>
@@ -262,7 +262,7 @@ compute_version(struct gl_context *ctx)<br>
ctx->Extensions.ARB_depth_<u></u>clamp &&<br>
ctx->Extensions.ARB_draw_<u></u>elements_base_vertex &&<br>
ctx->Extensions.ARB_fragment_<u></u>coord_conventions &&<br>
- ctx->Extensions.ARB_geometry_<u></u>shader4 &&<br>
</blockquote>
<br></div></div>
Removing ARB_geometry_shader4 here makes a lot of sense to me. IMO, the GLSLVersion >= 150 check above (but not shown in the diff) is already sufficient.<div class="HOEnZb"><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ ctx->Const.GeometryShaders150 &&<br>
ctx->Extensions.EXT_provoking_<u></u>vertex &&<br>
ctx->Extensions.ARB_seamless_<u></u>cube_map &&<br>
ctx->Extensions.ARB_sync &&<br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br></div></div>