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.roman...@intel.com>
>>
>> Signed-off-by: Ian Romanick <ian.d.roman...@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 <kenn...@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.
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 <kenn...@whitecape.org>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
