[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:25:51 PDT 2015
On 08/24/2015 08:17 AM, Ilia Mirkin wrote:
>
> On Aug 24, 2015 1:07 AM, "Tapani Pälli" <tapani.palli at intel.com
> <mailto: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 <mailto: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
> <mailto: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?
There are some differences between ARB_texture_multisample and GLES 31,
at least multisample array textures. Agreeably this is 'academical' and
may not happen in real life (like many other error checks in Mesa
though). I guess these could be removed if others also feel like this.
> >
> >
> >> 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 <mailto:mesa-dev at lists.freedesktop.org>
> >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
> >
> > // Tapani
>
More information about the mesa-dev
mailing list