On Mon, Mar 28, 2016 at 12:31 PM, Brian Paul <bri...@vmware.com> wrote:
> On 03/28/2016 10:20 AM, Ilia Mirkin wrote:
>>
>> Thanks! On the off chance you're in the mood for reviewing the rest of
>> the series but it's lost in your inbox, you can find it here:
>>
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.freedesktop.org_series_3887_&d=BQIFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=0xRgcNjzm9L4CxMt1DG9coPwumjYve81_bkYgcQA6-0&s=_fp9_BM7DpwjSOJAM3dQIDF02eQ7DCBbJxs8Yl8PsxE&e=
>>
>> The later patches actually pipe through GL_EXT/OES_texture_buffer
>> support so that the dEQP tests can run on mesa. (All the current dEQP
>> tests are around AEP, so they like the EXT variants.)
>
>
>
> Overall, the series looks OK to me. But I have a question.
> GL_OES/EXT_texture_buffer is only for ES, right? In the GLSL code, don't we
> have to check for that, in addition to the GLSL version? I'm looking at
> your new texture_buffer() helper function in
> src/compiler/glsl/builtin_functions.cpp
>
> Reviewed-by: Brian Paul <bri...@vmware.com>
>
Presumably you're referring to
static bool
+texture_buffer(const _mesa_glsl_parse_state *state)
+{
+ return state->is_version(140, 320) ||
+ state->EXT_texture_buffer_enable ||
+ state->OES_texture_buffer_enable;
+}
and the question is whether this shouldn't be more like
state->es_shader && state->EXT_foo_enable?
If so, I don't think so. Those exts are marked as ES-only in the
_mesa_glsl_supported_extensions (glsl_parser_extras.cpp), which means
that they should only be enable-able from an ES shader. So if you're
in a non-ES shader, those two enables should always be false (or else
we have larger problems).
Does that address your concern?
I guess there's a question here of what happens if you write a ESSL
1.0 shader and enable those exts (which require ESSL 3.10)... I think
it's supposed to fail, but mesa will be none the wiser and expose the
relevant functionality. The ext definitions in glsl_parser_extras
should probably have minimum GL/GLES versions, but those don't exist
right now. IMHO this isn't such a big deal.
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
