On Wed, Aug 26, 2015 at 5:03 AM, Tapani Pälli <tapani.pa...@intel.com> wrote: > On 08/26/2015 08:26 AM, Ilia Mirkin wrote: >> >> 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. > > > Not ignored, Ian stated that we have to see case by case and most of the > cases I've seen require checks. We should not be able to query this enum > using GLES 3.0. Doing the '_es31 way' we make sure that these exist only for > 3.1. We changed to this model when somebody pointed out that in earlier > model we exposed enums for GLES 2.0 apps.
Yeah, that somebody was me, if you recall. And I strongly argued against the _es31 stuff when it was first coming in too. But then I looked away, and this stuff got pushed. Same as with the ARB_dsa stuff when I kept saying that the piglits had to be core and then eventually gave up, and it caused a giant thing at mesa 10.6 release time. >>> 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). > > > OK. Maybe we've been too protective then if it works and I don't think we > have any 'negative tests' to for example use ES 3.1 enums in a GLES 2.0 > context. Program would need to strange anyway to either include ES 3.1 > headers or use hardcoded values in the calls. I don't have a strong opinion, > I'm somewhat disappointed that we are changing this once again. Remember, > first we did this loosely by exposing the enums for anyone. Then we made > more strict check by adding _es31 so that we would not expose enums in wrong > contexts and now we are back again giving them for everyone? I think from your next email you've come around to understanding what all I've been saying, but just in case, let me restate: Old check: ARB_texture_multisample || es31 es31 is set based on either a cmdline override (in which case I care less about compliance) or based on the condition in compute_version_es2. This condition will only be true if the various extensions are there, so logically es31 is an && of the various exts. [And also whether an actual ES31 context was selected.] Therefore the || es31 does nothing. Literally nothing. You can't *exclude* GLES2.0 by adding an || condition to the availability, and in this case, ES31 *implies* that ARB_texture_multisample will have been set, since no driver will have an ES31 context but not have ARB_texture_multisample. The only thing it can do is *add*, and it doesn't increase the number of states that will pass through the condition for any practical scenario. In case you're still unconvinced, come up with a scenario and show me which line of code is executed with the _es31 thing but not without. Outside of using cmdline overrides on a driver that has no business exposing ES3.1 in the first place, I don't think that will ever happen. -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
