On Wed, Aug 26, 2015 at 1:19 AM, Tapani Pälli <tapani.pa...@intel.com> wrote: > On 08/24/2015 04:18 PM, Ilia Mirkin wrote: >> >> On Fri, Aug 21, 2015 at 3:22 AM, Tapani Pälli <tapani.pa...@intel.com> >> wrote: >>> >>> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com> >>> --- >>> src/mesa/main/get_hash_params.py | 6 +++--- >>> src/mesa/main/texobj.c | 6 ++++-- >>> src/mesa/main/texparam.c | 2 +- >>> 3 files changed, 8 insertions(+), 6 deletions(-) >>> >>> diff --git a/src/mesa/main/get_hash_params.py >>> b/src/mesa/main/get_hash_params.py >>> index 517c391..c06e9b1 100644 >>> --- a/src/mesa/main/get_hash_params.py >>> +++ b/src/mesa/main/get_hash_params.py >>> @@ -434,6 +434,9 @@ descriptor=[ >>> [ "SAMPLE_MASK", "CONTEXT_BOOL(Multisample.SampleMask), >>> extra_ARB_texture_multisample_es31" ], >>> [ "MAX_SAMPLE_MASK_WORDS", "CONST(1), >>> extra_ARB_texture_multisample_es31" ], >>> >>> +# GL_ARB_texture_multisample / ES 3.1 with >>> GL_OES_texture_storage_multisample_2d_array >>> + [ "TEXTURE_BINDING_2D_MULTISAMPLE_ARRAY", "LOC_CUSTOM, TYPE_INT, >>> TEXTURE_2D_MULTISAMPLE_ARRAY_INDEX, extra_ARB_texture_multisample_es31" ], >> >> What does the _es31 add? Should be removed, I think. Should be done in >> a cleanup patch later though. Looks like I wasn't looking carefully >> enough at what was going on the list and this managed to sneak >> through... same goes for most (but probably not all) of the _es31 >> things. > > > I think using the _es31 is technically the best solution leaving no gaps or > 'hidden features' where we enable some GLES functionality through desktop > extension. I think we already went through this discussion.
Yeah, and I pointed this same thing out then too, and was ignored I believe. > For extensions > string enable, I think it's feasible to use desktop feature enables but > anywhere else in the code it hides things from the reader. I must be missing something. Can you describe to me a scenario under which the _es31 thing makes *any* change to the code paths executed? The only one I can imagine is with a driver that doesn't set Extensions.ARB_texture_multisample, but a user has force-enabled ES3.1 via override flags. I don't think that this is a case worth worrying about. In fact I've sent a patch to remove all the _es31 things (except for ARB_cs, where it's an actual scenario). > >>> + >>> # GL_ARB_texture_gather / GLES 3.1 >>> [ "MIN_PROGRAM_TEXTURE_GATHER_OFFSET", >>> "CONTEXT_INT(Const.MinProgramTextureGatherOffset), >>> extra_ARB_texture_gather_es31"], >>> [ "MAX_PROGRAM_TEXTURE_GATHER_OFFSET", >>> "CONTEXT_INT(Const.MaxProgramTextureGatherOffset), >>> extra_ARB_texture_gather_es31"], >>> @@ -740,9 +743,6 @@ descriptor=[ >>> [ "TEXTURE_BUFFER_FORMAT_ARB", "LOC_CUSTOM, TYPE_INT, 0, >>> extra_texture_buffer_object" ], >>> [ "TEXTURE_BUFFER_ARB", "LOC_CUSTOM, TYPE_INT, 0, >>> extra_texture_buffer_object" ], >>> >>> -# GL_ARB_texture_multisample / GL 3.2 >>> - [ "TEXTURE_BINDING_2D_MULTISAMPLE_ARRAY", "LOC_CUSTOM, TYPE_INT, >>> TEXTURE_2D_MULTISAMPLE_ARRAY_INDEX, extra_ARB_texture_multisample" ], >>> - >>> # GL 3.0 >>> [ "CONTEXT_FLAGS", "CONTEXT_INT(Const.ContextFlags), >>> extra_version_30" ], >>> >>> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c >>> index 395e4d3..a3f44c9 100644 >>> --- a/src/mesa/main/texobj.c >>> +++ b/src/mesa/main/texobj.c >>> @@ -217,7 +217,8 @@ _mesa_get_current_tex_object(struct gl_context *ctx, >>> GLenum target) >>> return ctx->Extensions.ARB_texture_multisample >>> ? ctx->Texture.ProxyTex[TEXTURE_2D_MULTISAMPLE_INDEX] : >>> NULL; >>> case GL_TEXTURE_2D_MULTISAMPLE_ARRAY: >>> - return ctx->Extensions.ARB_texture_multisample >>> + return (ctx->Extensions.ARB_texture_multisample || >>> + >>> ctx->Extensions.OES_texture_storage_multisample_2d_array) >> >> And then you don't have to touch this (if you don't add the extra enable) > > > This is used in such a lot of places that I'll need to then add is_gles31() > though. One should not be able to get this using bare gles 2.0. errr... how does the context affect what's in Extensions.*? [hint, it doesn't... if you need those checks now, you also needed them before] > >>> ? texUnit->CurrentTex[TEXTURE_2D_MULTISAMPLE_ARRAY_INDEX] : >>> NULL; >>> case GL_PROXY_TEXTURE_2D_MULTISAMPLE_ARRAY: >>> return ctx->Extensions.ARB_texture_multisample >>> @@ -1612,7 +1613,8 @@ _mesa_tex_target_to_index(const struct gl_context >>> *ctx, GLenum target) >>> return ((_mesa_is_desktop_gl(ctx) && >>> ctx->Extensions.ARB_texture_multisample) || >>> _mesa_is_gles31(ctx)) ? TEXTURE_2D_MULTISAMPLE_INDEX: -1; >>> case GL_TEXTURE_2D_MULTISAMPLE_ARRAY: >>> - return _mesa_is_desktop_gl(ctx) && >>> ctx->Extensions.ARB_texture_multisample >>> + return ((_mesa_is_desktop_gl(ctx) && >>> ctx->Extensions.ARB_texture_multisample) || >>> + ctx->Extensions.OES_texture_storage_multisample_2d_array) >> >> or this > > > Same here, is_gles31() check needed without separate extension enable. Separate enable has got nothing to do with it. It's enabled irrespective of the context. -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
