[Mesa-dev] [PATCH 01/12] mesa/es: Validate FBO attachment enum in Mesa code rather than the ES wrapper

[Brian Paul]

On 10/02/2011 04:44 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 |   27 ---------------------------
>   src/mesa/main/fbobject.c  |    7 ++++++-
>   2 files changed, 6 insertions(+), 28 deletions(-)
>
> diff --git a/src/mesa/main/APIspec.xml b/src/mesa/main/APIspec.xml
> index 0f82d0a..2b277a0 100644
> --- a/src/mesa/main/APIspec.xml
> +++ b/src/mesa/main/APIspec.xml
> @@ -3217,15 +3217,6 @@
>   		<value name="GL_FRAMEBUFFER" category="GLES2.0"/>
>   	</desc>
>
> -	<desc name="attachment">
> -		<value name="GL_COLOR_ATTACHMENT0_OES" category="OES_framebuffer_object"/>
> -		<value name="GL_DEPTH_ATTACHMENT_OES" category="OES_framebuffer_object"/>
> -		<value name="GL_STENCIL_ATTACHMENT_OES" category="OES_framebuffer_object"/>
> -		<value name="GL_COLOR_ATTACHMENT0" category="GLES2.0"/>
> -		<value name="GL_DEPTH_ATTACHMENT" category="GLES2.0"/>
> -		<value name="GL_STENCIL_ATTACHMENT" category="GLES2.0"/>
> -	</desc>
> -
>   	<desc name="renderbuffertarget">
>   		<value name="GL_RENDERBUFFER_OES" category="OES_framebuffer_object"/>
>   		<value name="GL_RENDERBUFFER" category="GLES2.0"/>
> @@ -3247,15 +3238,6 @@
>   		<value name="GL_FRAMEBUFFER" category="GLES2.0"/>
>   	</desc>
>
> -	<desc name="attachment">
> -		<value name="GL_COLOR_ATTACHMENT0_OES" category="OES_framebuffer_object"/>
> -		<value name="GL_DEPTH_ATTACHMENT_OES" category="OES_framebuffer_object"/>
> -		<value name="GL_STENCIL_ATTACHMENT_OES" category="OES_framebuffer_object"/>
> -		<value name="GL_COLOR_ATTACHMENT0" category="GLES2.0"/>
> -		<value name="GL_DEPTH_ATTACHMENT" category="GLES2.0"/>
> -		<value name="GL_STENCIL_ATTACHMENT" category="GLES2.0"/>
> -	</desc>
> -
>   	<desc name="textarget" error="GL_INVALID_OPERATION">
>   		<value name="GL_TEXTURE_2D"/>
>   		<value name="GL_TEXTURE_CUBE_MAP_POSITIVE_X" category="GLES2.0"/>
> @@ -3292,15 +3274,6 @@
>   		<value name="GL_FRAMEBUFFER" category="GLES2.0"/>
>   	</desc>
>
> -	<desc name="attachment">
> -		<value name="GL_COLOR_ATTACHMENT0_OES" category="OES_framebuffer_object"/>
> -		<value name="GL_DEPTH_ATTACHMENT_OES" category="OES_framebuffer_object"/>
> -		<value name="GL_STENCIL_ATTACHMENT_OES" category="OES_framebuffer_object"/>
> -		<value name="GL_COLOR_ATTACHMENT0" category="GLES2.0"/>
> -		<value name="GL_DEPTH_ATTACHMENT" category="GLES2.0"/>
> -		<value name="GL_STENCIL_ATTACHMENT" category="GLES2.0"/>
> -	</desc>
> -
>   	<desc name="textarget" error="GL_INVALID_OPERATION">
>   		<value name="GL_TEXTURE_3D_OES" category="OES_texture_3D"/>
>   	</desc>
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 139ff03..2e0af97 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -239,17 +239,22 @@ _mesa_get_attachment(struct gl_context *ctx, struct gl_framebuffer *fb,
>      case GL_COLOR_ATTACHMENT14_EXT:
>      case GL_COLOR_ATTACHMENT15_EXT:
>         i = attachment - GL_COLOR_ATTACHMENT0_EXT;
> -      if (i>= ctx->Const.MaxColorAttachments) {
> +      if (i>= ctx->Const.MaxColorAttachments
> +	  || (i>  0&&  ctx->API != API_OPENGL)) {
>   	 return NULL;
>         }

Couldn't we just set ctx->Const.MaxColorAttachments = 1 for ES 
contexts to avoid changing this test (and get short loops elsewhere)?

Otherwise, these changes look good to me.

Reviewed-by: Brian Paul <brianp at vmware.com>