[Mesa-dev] [PATCH] mesa: Filter glGetProgramiv pnames based on available extensions

[Ian Romanick]

On 08/24/2012 08:51 AM, Brian Paul wrote:
> On 08/24/2012 09:42 AM, Ian Romanick wrote:
>> From: Ian Romanick<ian.d.romanick at intel.com>
>>
>> Previously you could always glGetProgramiv one of the transform feedback
>> or geometry shader enums even if the extension wasn't supported.
>>
>> In addtion, this reverts part of bda6ad27.  I think the hunks involving
>> GL_PROGRAM_BINARY_LENGTH_OES were spurious.  Mesa has no support for any
>> other part of GL_OES_get_program_binary.
>>
>> v2: Remove redundant return in get_programiv based on review feedback
>> from Matt Turner.
>>
>> v3: Correctly handle UBO related enums.
>>
>> Signed-off-by: Ian Romanick<ian.d.romanick at intel.com>
>> Cc: Brian Paul<brianp at vmware.com>
>> ---
>>   src/mesa/main/shaderapi.c |   66
>> ++++++++++++++++++++++++--------------------
>>   1 files changed, 36 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index caeb965..1e69458 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -481,28 +481,28 @@ get_programiv(struct gl_context *ctx, GLuint
>> program, GLenum pname, GLint *param
>>      switch (pname) {
>>      case GL_DELETE_STATUS:
>>         *params = shProg->DeletePending;
>> -      break;
>> +      return;
>>      case GL_LINK_STATUS:
>>         *params = shProg->LinkStatus;
>> -      break;
>> +      return;
>>      case GL_VALIDATE_STATUS:
>>         *params = shProg->Validated;
>> -      break;
>> +      return;
>>      case GL_INFO_LOG_LENGTH:
>>         *params = shProg->InfoLog ? strlen(shProg->InfoLog) + 1 : 0;
>> -      break;
>> +      return;
>>      case GL_ATTACHED_SHADERS:
>>         *params = shProg->NumShaders;
>> -      break;
>> +      return;
>>      case GL_ACTIVE_ATTRIBUTES:
>>         *params = _mesa_count_active_attribs(shProg);
>> -      break;
>> +      return;
>>      case GL_ACTIVE_ATTRIBUTE_MAX_LENGTH:
>>         *params = _mesa_longest_attribute_name_length(shProg);
>> -      break;
>> +      return;
>>      case GL_ACTIVE_UNIFORMS:
>>         *params = shProg->NumUserUniformStorage;
>> -      break;
>> +      return;
>>      case GL_ACTIVE_UNIFORM_MAX_LENGTH: {
>>         unsigned i;
>>         GLint max_len = 0;
>> @@ -517,41 +517,48 @@ get_programiv(struct gl_context *ctx, GLuint
>> program, GLenum pname, GLint *param
>>         }
>>
>>         *params = max_len;
>> -      break;
>> +      return;
>>      }
>> -   case GL_PROGRAM_BINARY_LENGTH_OES:
>> -      *params = 0;
>> -      break;
>>   #if FEATURE_EXT_transform_feedback
>>      case GL_TRANSFORM_FEEDBACK_VARYINGS:
>> +      if (!ctx->Extensions.EXT_transform_feedback)
>> +         break;
>>         *params = shProg->TransformFeedback.NumVarying;
>> -      break;
>> +      return;
>>      case GL_TRANSFORM_FEEDBACK_VARYING_MAX_LENGTH:
>> +      if (!ctx->Extensions.EXT_transform_feedback)
>> +         break;
>>         *params = longest_feedback_varying_name(shProg) + 1;
>> -      break;
>> +      return;
>>      case GL_TRANSFORM_FEEDBACK_BUFFER_MODE:
>> +      if (!ctx->Extensions.EXT_transform_feedback)
>> +         break;
>>         *params = shProg->TransformFeedback.BufferMode;
>> -      break;
>> +      return;
>>   #endif
>>   #if FEATURE_ARB_geometry_shader4
>>      case GL_GEOMETRY_VERTICES_OUT_ARB:
>> +      if (!ctx->Extensions.ARB_geometry_shader4)
>> +         break;
>>         *params = shProg->Geom.VerticesOut;
>> -      break;
>> +      return;
>>      case GL_GEOMETRY_INPUT_TYPE_ARB:
>> +      if (!ctx->Extensions.ARB_geometry_shader4)
>> +         break;
>>         *params = shProg->Geom.InputType;
>> -      break;
>> +      return;
>>      case GL_GEOMETRY_OUTPUT_TYPE_ARB:
>> +      if (!ctx->Extensions.ARB_geometry_shader4)
>> +         break;
>>         *params = shProg->Geom.OutputType;
>> -      break;
>> +      return;
>>   #endif
>>      case GL_ACTIVE_UNIFORM_BLOCK_MAX_NAME_LENGTH: {
>>         unsigned i;
>>         GLint max_len = 0;
>>
>> -      if (!ctx->Extensions.ARB_uniform_buffer_object) {
>> -         _mesa_error(ctx, GL_INVALID_ENUM, "glGetProgramiv(pname)");
>> -         return;
>> -      }
>> +      if (!ctx->Extensions.ARB_uniform_buffer_object)
>> +         break;
>>
>>         for (i = 0; i<  shProg->NumUniformBlocks; i++) {
>>        /* Add one for the terminating NUL character.
>> @@ -563,20 +570,19 @@ get_programiv(struct gl_context *ctx, GLuint
>> program, GLenum pname, GLint *param
>>         }
>>
>>         *params = max_len;
>> -      break;
>> +      return;
>>      }
>>      case GL_ACTIVE_UNIFORM_BLOCKS:
>> -      if (!ctx->Extensions.ARB_uniform_buffer_object) {
>> -         _mesa_error(ctx, GL_INVALID_ENUM, "glGetProgramiv(pname)");
>> -         return;
>> -      }
>> +      if (!ctx->Extensions.ARB_uniform_buffer_object)
>> +         break;
>>
>>         *params = shProg->NumUniformBlocks;
>> -      break;
>> -   default:
>> -      _mesa_error(ctx, GL_INVALID_ENUM, "glGetProgramiv(pname)");
>>         return;
>> +   default:
>> +      break;
>>      }
>> +
>> +   _mesa_error(ctx, GL_INVALID_ENUM, "glGetProgramiv(pname)");
>
> I'm trying to get into the habit of including the bad enum value in
> error strings like this.  Inevitably, where I don't do this I eventually
> have to go in with gdb to get more info.  And this will probably be more
> useful with the ARB_debug extensions.

Right.  I just moved the existing _mesa_error call down a few lines. 
I'll make this change before pushing.

>
> For your copy & paste convenience:
>
>     _mesa_error(ctx, GL_INVALID_ENUM, "glGetProgramiv(pname=%s)",
>                 _mesa_lookup_enum_by_nr(pname));
>
>
> Otherwise,
> Reviewed-by: Brian Paul <brianp at vmware.com>
>
> -Brian
>
>