[Mesa-dev] [PATCH 03/26] mesa/es: Remove redundant texture target validation
Ian Romanick
idr at freedesktop.org
Mon Aug 20 10:04:28 PDT 2012
On 08/20/2012 12:15 AM, Kenneth Graunke wrote:
> On 08/19/2012 11:28 PM, Kenneth Graunke wrote:
>> On 08/17/2012 08:11 PM, Ian Romanick wrote:
>>> From: Ian Romanick <ian.d.romanick at intel.com>
>>>
>>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>>> ---
>>> src/mesa/main/APIspec.xml | 12 ---------
>>> src/mesa/main/es1_conversion.c | 51 ----------------------------------------
>>> 2 files changed, 0 insertions(+), 63 deletions(-)
>>>
>>> diff --git a/src/mesa/main/APIspec.xml b/src/mesa/main/APIspec.xml
>>> index 68e6535..b957db4 100644
>>> --- a/src/mesa/main/APIspec.xml
>>> +++ b/src/mesa/main/APIspec.xml
>>> @@ -2345,10 +2345,6 @@
>>> <param name="q" type="GLtype"/>
>>> </vector>
>>> </proto>
>>> -
>>> - <desc name="texture">
>>> - <range base="GL_TEXTURE" from="0" to="31"/>
>>> - </desc>
>>> </template>
>>>
>>> <template name="CompressedTexImage3D">
>>> @@ -2404,10 +2400,6 @@
>>> <return type="void"/>
>>> <param name="texture" type="GLenum"/>
>>> </proto>
>>> -
>>> - <desc name="texture">
>>> - <range base="GL_TEXTURE" from="0" to="31"/>
>>> - </desc>
>>> </template>
>>>
>>> <template name="ClientActiveTexture">
>>> @@ -2415,10 +2407,6 @@
>>> <return type="void"/>
>>> <param name="texture" type="GLenum"/>
>>> </proto>
>>> -
>>> - <desc name="texture">
>>> - <range base="GL_TEXTURE" from="0" to="31"/>
>>> - </desc>
>>> </template>
>>>
>>> <template name="SampleCoverage">
>>> diff --git a/src/mesa/main/es1_conversion.c b/src/mesa/main/es1_conversion.c
>>> index 16e3f57..32dda67 100644
>>> --- a/src/mesa/main/es1_conversion.c
>>> +++ b/src/mesa/main/es1_conversion.c
>>> @@ -801,47 +801,6 @@ _es_MultMatrixx(const GLfixed *m)
>>> void GL_APIENTRY
>>> _es_MultiTexCoord4x(GLenum texture, GLfixed s, GLfixed t, GLfixed r, GLfixed q)
>>> {
>>> - switch(texture) {
>>> - case GL_TEXTURE0:
>>> - case GL_TEXTURE1:
>>> - case GL_TEXTURE2:
>>> - case GL_TEXTURE3:
>>> - case GL_TEXTURE4:
>>> - case GL_TEXTURE5:
>>> - case GL_TEXTURE6:
>>> - case GL_TEXTURE7:
>>> - case GL_TEXTURE8:
>>> - case GL_TEXTURE9:
>>> - case GL_TEXTURE10:
>>> - case GL_TEXTURE11:
>>> - case GL_TEXTURE12:
>>> - case GL_TEXTURE13:
>>> - case GL_TEXTURE14:
>>> - case GL_TEXTURE15:
>>> - case GL_TEXTURE16:
>>> - case GL_TEXTURE17:
>>> - case GL_TEXTURE18:
>>> - case GL_TEXTURE19:
>>> - case GL_TEXTURE20:
>>> - case GL_TEXTURE21:
>>> - case GL_TEXTURE22:
>>> - case GL_TEXTURE23:
>>> - case GL_TEXTURE24:
>>> - case GL_TEXTURE25:
>>> - case GL_TEXTURE26:
>>> - case GL_TEXTURE27:
>>> - case GL_TEXTURE28:
>>> - case GL_TEXTURE29:
>>> - case GL_TEXTURE30:
>>> - case GL_TEXTURE31:
>>> - break;
>>> - default:
>>> - _mesa_error(_mesa_get_current_context(), GL_INVALID_ENUM,
>>> - "glMultiTexCoord4x(texture=0x%x)", texture);
>>> - return;
>>> - }
>>> -
>>> -
>>> _es_MultiTexCoord4f(texture,
>>> (GLfloat) (s / 65536.0f),
>>> (GLfloat) (t / 65536.0f),
>>> @@ -1041,11 +1000,6 @@ _es_TexEnvx(GLenum target, GLenum pname, GLfixed param)
>>> case GL_SRC0_ALPHA:
>>> case GL_SRC1_ALPHA:
>>> case GL_SRC2_ALPHA:
>>> - if (param != GL_TEXTURE && param != GL_CONSTANT && param != GL_PRIMARY_COLOR && param != GL_PREVIOUS && param != GL_TEXTURE0 && param != GL_TEXTURE1 && param != GL_TEXTURE2 && param != GL_TEXTURE3 && param != GL_TEXTURE4 && param != GL_TEXTURE5 && param != GL_TEXTURE6 && param != GL_TEXTURE7 && param != GL_TEXTURE8 && param != GL_TEXTURE9 && param != GL_TEXTURE10 && param != GL_TEXTURE11 && param != GL_TEXTURE12 && param != GL_TEXTURE13 && param != GL_TEXTURE14 && param != GL_TEXTURE15 && param != GL_TEXTURE16 && param != GL_TEXTURE17 && param != GL_TEXTURE18 && param != GL_TEXTURE19 && param != GL_TEXTURE20 && param != GL_TEXTURE21 && param != GL_TEXTURE22 && param != GL_TEXTURE23 && param != GL_TEXTURE24 && param != GL_TEXTURE25 && param != GL_TEXTURE26 && param != GL_TEXTURE27 && param != GL_TEXTURE28 && param != GL_TEXTURE29 && param != GL_TEXTURE30 && param != GL_TEXTURE31) {
>>> - _mesa_error(_mesa_get_current_context(), GL_INVALID_ENUM,
>>> - "glTexEnvx(pname=0x%x)", pname);
>>> - return;
>>> - }
>>> convert_param_value = false;
>>> break;
>>> case GL_OPERAND0_RGB:
>>> @@ -1167,11 +1121,6 @@ _es_TexEnvxv(GLenum target, GLenum pname, const GLfixed *params)
>>> case GL_SRC0_ALPHA:
>>> case GL_SRC1_ALPHA:
>>> case GL_SRC2_ALPHA:
>>> - if (params[0] != GL_TEXTURE && params[0] != GL_CONSTANT && params[0] != GL_PRIMARY_COLOR && params[0] != GL_PREVIOUS && params[0] != GL_TEXTURE0 && params[0] != GL_TEXTURE1 && params[0] != GL_TEXTURE2 && params[0] != GL_TEXTURE3 && params[0] != GL_TEXTURE4 && params[0] != GL_TEXTURE5 && params[0] != GL_TEXTURE6 && params[0] != GL_TEXTURE7 && params[0] != GL_TEXTURE8 && params[0] != GL_TEXTURE9 && params[0] != GL_TEXTURE10 && params[0] != GL_TEXTURE11 && params[0] != GL_TEXTURE12 && params[0] != GL_TEXTURE13 && params[0] != GL_TEXTURE14 && params[0] != GL_TEXTURE15 && params[0] != GL_TEXTURE16 && params[0] != GL_TEXTURE17 && params[0] != GL_TEXTURE18 && params[0] != GL_TEXTURE19 && params[0] != GL_TEXTURE20 && params[0] != GL_TEXTURE21 && params[0] != GL_TEXTURE22 && params[0] != GL_TEXTURE23 && params[0] != GL_TEXTURE24 && params[0] != GL_TEXTURE25 && params[0] != GL_TEXTURE26 && params[0] != GL_TEXTURE27 && params[0] != GL_TEXTURE28 && params[0] != GL_TEXTURE29 &&
> pa!
>>> rams[0] != GL_TEXTURE30 && params[0] != GL_TEXTURE31) {
>>> - _mesa_error(_mesa_get_current_context(), GL_INVALID_ENUM,
>>> - "glTexEnvxv(pname=0x%x)", pname);
>>> - return;
>>> - }
>>> convert_params_value = false;
>>> n_params = 1;
>>> break;
>>
>> The irony here is that, for the TexEnv cases, the core Mesa code only
>> accepts GL_TEXTURE0 .. GL_TEXTURE7. (Follow the chain: _es_TexEnvx ->
>> _mesa_TexEnvf -> set_combiner_source) In other words, not only is it
>> redundant, it's unduly lenient.
>>
>> Well...unless the core Mesa code is supposed to accept the full 0..31.
>> Regardless, that would be a separate bug fix.
>>
>> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
> Wait, no, there is a bug: it turns out that MultiTexCoord does
> absolutely /no/ validation on its targets. From vbo_attrib_tmp.h:
>
> static void GLAPIENTRY
> TAG(MultiTexCoord4f)(GLenum target, GLfloat x, GLfloat y, GLfloat z,
> GLfloat w)
> {
> GET_CURRENT_CONTEXT(ctx);
> GLuint attr = (target & 0x7) + VBO_ATTRIB_TEX0;
> ATTR4F(attr, x, y, z, w);
> }
>
> In other words, it happily accepts 0x31337 and calls it GL_TEXTURE7. I
> verified this with a small GL program.
Dare I ask what happens if you pass GL_TEXTURE0+666? Does it just smash
context memory? That seems bad. :(
I think I'm inclined to take this patch and submit a bug for the other
issue (since it already affects desktop OpenGL). We should fix that bug.
> Lame. Though I could see not wanting to waste CPU verifying such
> trivialities.
>
> So, your patch introduces this broken behavior on ES, but at least makes
> it consistent with desktop GL. Up to you if you care to fix it.
>
> For the series:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
More information about the mesa-dev
mailing list