[Mesa-dev] [PATCH 1/6] mesa: GetTexLevelParameter{if}v changes for OpenGL ES 3.1
Ilia Mirkin
imirkin at alum.mit.edu
Sun Aug 23 22:17:24 PDT 2015
On Aug 24, 2015 1:07 AM, "Tapani Pälli" <tapani.palli at intel.com> wrote:
>
>
>
> 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.
I think you may have misunderstood my comment.
Old code: if not texture ms
New code: if not gles31 and not texture ms
Aka "not (gles31 or texture ms)"
I posit that the situation where gles31 and not texture ms will never ever
happen.
So you haven't changed the outcome of that conditional for any reasonable
circumstances. Why are you adding that code?
>
>
>> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150824/f8a2700e/attachment-0001.html>
More information about the mesa-dev
mailing list