[Mesa-dev] [PATCH 05/34] main: Allow for the possibility of GL 3.2 without ARB_geometry_shader4.

[Paul Berry]

On 29 July 2013 11:17, Kenneth Graunke <kenneth at whitecape.org> wrote:

> On 07/28/2013 11:03 PM, Paul Berry wrote:
>
>> +/**
>> + * Checks if the context supports geometry shaders.
>> + */
>> +static inline GLboolean
>> +_mesa_has_geometry_shaders(const struct gl_context *ctx)
>> +{
>> +   return _mesa_is_desktop_gl(ctx) &&
>> +      (ctx->Version >= 32 || ctx->Extensions.ARB_geometry_shader4);
>> +}
>>
>
> I might change this to:
>
> return _mesa_is_desktop_gl(ctx) &&
>    (ctx->Const.GLSLVersion >= 150 || ctx->Extensions.ARB_geometry_**
> shader4);
>
>
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.


>
>  +
>> +
>>   #ifdef __cplusplus
>>   }
>>   #endif
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index a29f1ab..f0f59a6 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -2472,7 +2472,7 @@ _mesa_FramebufferTexture(**GLenum target, GLenum
>> attachment,
>>   {
>>      GET_CURRENT_CONTEXT(ctx);
>>
>> -   if (ctx->Version >= 32 || ctx->Extensions.ARB_geometry_**shader4) {
>> +   if (_mesa_has_geometry_shaders(**ctx)) {
>>         framebuffer_texture(ctx, "Layer", target, attachment, 0, texture,
>>                             level, 0, GL_TRUE);
>>      } else {
>> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
>> index 0b33fa4..09b008a 100644
>> --- a/src/mesa/main/get.c
>> +++ b/src/mesa/main/get.c
>> @@ -989,7 +989,7 @@ check_extra(struct gl_context *ctx, const char *func,
>> const struct value_desc *d
>>         case EXTRA_EXT_UBO_GS4:
>>            api_check = GL_TRUE;
>>            api_found = (ctx->Extensions.ARB_uniform_**buffer_object &&
>> -                      ctx->Extensions.ARB_geometry_**shader4);
>> +                      _mesa_has_geometry_shaders(**ctx));
>>            break;
>>         case EXTRA_END:
>>          break;
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index aba7d84..efa2d39 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -2978,6 +2978,14 @@ struct gl_constants
>>      GLint MaxColorTextureSamples;
>>      GLint MaxDepthTextureSamples;
>>      GLint MaxIntegerSamples;
>> +
>> +   /**
>> +    * True if the implementation supports GLSL 1.50 style geometry
>> shaders.
>> +    * This boolean is distinct from gl_extensions::ARB_geometry_**shader4
>> so
>> +    * that we can expose GLSL 1.50 (and GL 3.2) functionality without
>> exposing
>> +    * {ARB,EXT}_geometry_shader4.
>> +    */
>> +   GLboolean GeometryShaders150;
>>   };
>>
>
> 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.
>
> ctx->Const.GLSLVersion is already used to compute the GL version.
>
>
>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index 9c8af87..e8303c8 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -177,7 +177,7 @@ validate_shader_target(const struct gl_context *ctx,
>> GLenum type)
>>      case GL_VERTEX_SHADER:
>>         return ctx->Extensions.ARB_vertex_**shader;
>>      case GL_GEOMETRY_SHADER_ARB:
>> -      return _mesa_is_desktop_gl(ctx) && ctx->Extensions.ARB_geometry_**
>> shader4;
>> +      return _mesa_has_geometry_shaders(**ctx);
>>      default:
>>         return false;
>>      }
>> @@ -476,8 +476,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
>> GLenum pname, GLint *param
>>
>>      /* Are geometry shaders available in this context?
>>       */
>> -   const bool has_gs =
>> -      _mesa_is_desktop_gl(ctx) && ctx->Extensions.ARB_geometry_**
>> shader4;
>> +   const bool has_gs = _mesa_has_geometry_shaders(**ctx);
>>
>>      /* Are uniform buffer objects available in this context?
>>       */
>> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
>> index ab9b14c..0e836cc 100644
>> --- a/src/mesa/main/version.c
>> +++ b/src/mesa/main/version.c
>> @@ -262,7 +262,7 @@ compute_version(struct gl_context *ctx)
>>                                 ctx->Extensions.ARB_depth_**clamp &&
>>                                 ctx->Extensions.ARB_draw_**elements_base_vertex
>> &&
>>                                 ctx->Extensions.ARB_fragment_**coord_conventions
>> &&
>> -                              ctx->Extensions.ARB_geometry_**shader4 &&
>>
>
> 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.
>
>
>  +                              ctx->Const.GeometryShaders150 &&
>>                                 ctx->Extensions.EXT_provoking_**vertex &&
>>                                 ctx->Extensions.ARB_seamless_**cube_map
>> &&
>>                                 ctx->Extensions.ARB_sync &&
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130730/26cfd912/attachment-0001.html>