[Mesa-dev] [PATCH 1/6] mesa: GetTexLevelParameter{if}v changes for OpenGL ES 3.1

Tapani Pälli tapani.palli at intel.com
Sun Aug 23 22:07:03 PDT 2015



On 08/21/2015 05:14 PM, Ilia Mirkin wrote:
> On Fri, Aug 21, 2015 at 3:22 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> Patch refactors existing parameters check to first check common enums
>> between desktop GL and GLES 3.1 and modifies get_tex_level_parameter_image
>> to be compatible with enums specified in 3.1.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/main/texparam.c | 34 +++++++++++++++++++++++-----------
>>   1 file changed, 23 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
>> index 16739f1..947a2a1 100644
>> --- a/src/mesa/main/texparam.c
>> +++ b/src/mesa/main/texparam.c
>> @@ -1208,20 +1208,34 @@ static GLboolean
>>   legal_get_tex_level_parameter_target(struct gl_context *ctx, GLenum target,
>>                                        bool dsa)
>>   {
>> +   /* Common targets for desktop GL and GLES 3.1. */
>>      switch (target) {
>> -   case GL_TEXTURE_1D:
>> -   case GL_PROXY_TEXTURE_1D:
>>      case GL_TEXTURE_2D:
>> -   case GL_PROXY_TEXTURE_2D:
>>      case GL_TEXTURE_3D:
>> -   case GL_PROXY_TEXTURE_3D:
>>         return GL_TRUE;
>> +   case GL_TEXTURE_2D_ARRAY_EXT:
>> +      return (_mesa_is_gles31(ctx) || ctx->Extensions.EXT_texture_array);
>>      case GL_TEXTURE_CUBE_MAP_POSITIVE_X_ARB:
>>      case GL_TEXTURE_CUBE_MAP_NEGATIVE_X_ARB:
>>      case GL_TEXTURE_CUBE_MAP_POSITIVE_Y_ARB:
>>      case GL_TEXTURE_CUBE_MAP_NEGATIVE_Y_ARB:
>>      case GL_TEXTURE_CUBE_MAP_POSITIVE_Z_ARB:
>>      case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z_ARB:
>> +      return (_mesa_is_gles31(ctx) || ctx->Extensions.ARB_texture_cube_map);
>> +   case GL_TEXTURE_2D_MULTISAMPLE:
>> +      return (_mesa_is_gles31(ctx) || ctx->Extensions.ARB_texture_multisample);
>> +   }
>> +
>> +   if (!_mesa_is_desktop_gl(ctx))
>> +      return GL_FALSE;
>> +
>> +   /* Rest of the desktop GL targets. */
>> +   switch (target) {
>> +   case GL_TEXTURE_1D:
>> +   case GL_PROXY_TEXTURE_1D:
>> +   case GL_PROXY_TEXTURE_2D:
>> +   case GL_PROXY_TEXTURE_3D:
>> +      return GL_TRUE;
>>      case GL_PROXY_TEXTURE_CUBE_MAP_ARB:
>>         return ctx->Extensions.ARB_texture_cube_map;
>>      case GL_TEXTURE_CUBE_MAP_ARRAY_ARB:
>> @@ -1232,7 +1246,6 @@ legal_get_tex_level_parameter_target(struct gl_context *ctx, GLenum target,
>>         return ctx->Extensions.NV_texture_rectangle;
>>      case GL_TEXTURE_1D_ARRAY_EXT:
>>      case GL_PROXY_TEXTURE_1D_ARRAY_EXT:
>> -   case GL_TEXTURE_2D_ARRAY_EXT:
>>      case GL_PROXY_TEXTURE_2D_ARRAY_EXT:
>>         return ctx->Extensions.EXT_texture_array;
>>      case GL_TEXTURE_BUFFER:
>> @@ -1254,7 +1267,6 @@ legal_get_tex_level_parameter_target(struct gl_context *ctx, GLenum target,
>>          * "target may also be TEXTURE_BUFFER, indicating the texture buffer."
>>          */
>>         return ctx->API == API_OPENGL_CORE && ctx->Version >= 31;
>> -   case GL_TEXTURE_2D_MULTISAMPLE:
>>      case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
>>      case GL_PROXY_TEXTURE_2D_MULTISAMPLE:
>>      case GL_PROXY_TEXTURE_2D_MULTISAMPLE_ARRAY:
>> @@ -1381,8 +1393,8 @@ get_tex_level_parameter_image(struct gl_context *ctx,
>>            *params = _mesa_get_format_bits(texFormat, pname);
>>            break;
>>         case GL_TEXTURE_SHARED_SIZE:
>> -         if (ctx->Version < 30 &&
>> -             !ctx->Extensions.EXT_texture_shared_exponent)
>> +         if (!_mesa_is_gles31(ctx) && (ctx->Version < 30 &&
>> +             !ctx->Extensions.EXT_texture_shared_exponent))
>>               goto invalid_pname;
>>            *params = texFormat == MESA_FORMAT_R9G9B9E5_FLOAT ? 5 : 0;
>>            break;
>> @@ -1415,7 +1427,7 @@ get_tex_level_parameter_image(struct gl_context *ctx,
>>         case GL_TEXTURE_BLUE_TYPE_ARB:
>>         case GL_TEXTURE_ALPHA_TYPE_ARB:
>>         case GL_TEXTURE_DEPTH_TYPE_ARB:
>> -         if (!ctx->Extensions.ARB_texture_float)
>> +         if (!_mesa_is_gles31(ctx) && !ctx->Extensions.ARB_texture_float)
>>               goto invalid_pname;
>>           if (_mesa_base_format_has_channel(img->_BaseFormat, pname))
>>              *params = _mesa_get_format_datatype(texFormat);
>> @@ -1425,13 +1437,13 @@ get_tex_level_parameter_image(struct gl_context *ctx,
>>
>>         /* GL_ARB_texture_multisample */
>>         case GL_TEXTURE_SAMPLES:
>> -         if (!ctx->Extensions.ARB_texture_multisample)
>> +         if (!_mesa_is_gles31(ctx) && !ctx->Extensions.ARB_texture_multisample)
>>               goto invalid_pname;
>>            *params = img->NumSamples;
>>            break;
>>
>>         case GL_TEXTURE_FIXED_SAMPLE_LOCATIONS:
>> -         if (!ctx->Extensions.ARB_texture_multisample)
>> +         if (!_mesa_is_gles31(ctx) && !ctx->Extensions.ARB_texture_multisample)
>
> I think you guys have gone a little nuts with sticking _mesa_is_gles31
> all over the place... This is accounting for drivers which want to
> expose gles3.1 but don't have ARB_texture_multisample supported. Are
> there any such drivers? Are all these changes really worthwhile?

This particular patch was to address

"glGetTexLevelParameter[fi]v - needs updates to restrict to GLES enums"

in docs/GL3.txt GLES3.1 section.

I do understand your worry about sticking is_gles31() here and there but 
remember that most (if not all) these changes will apply also for 
gles32. In Mesa there are 232 instances of is_gles(), 156 of is_gles3() 
and only 34 instances of is_gles31() so I think the overall changes are 
still quite small, not so intrusive as it may look.

> The original argument was that you wanted to pass some CTS tests
> before you actually had compute/etc shaders going... which seems
> reasonable. This all feels like going overboard though.

For the question if changes are worthwhile, we have gone from 0 passing 
CTS tests (+ a few crashes) to > 200 passing tests, personally I think 
this is good progress, thanks to you and others who have corrected 
errors in our changes.

Our goal is to pass the whole suite when rest of the missing extensions 
are integrated. There are still things to do, API differences here and 
there. Nothing major but important to get done.



>    -ilia
>
>>               goto invalid_pname;
>>            *params = img->FixedSampleLocations;
>>            break;
>> --
>> 2.4.3
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


// Tapani


More information about the mesa-dev mailing list