On 03/28/2016 10:38 AM, Ilia Mirkin wrote:
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?
Yes, thanks.
-Brian
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
