On Tuesday, June 09, 2015 09:53:42 AM Tapani Pälli wrote: > Mesa supports EXT_texture_rg and OES_texture_float. This patch adds > support for using unsized enums GL_RED and GL_RG for floating point > targets and writes proper checks for internalformat when format is > GL_RED or GL_RG and type is of GL_FLOAT or GL_HALF_FLOAT. > > Later, internalformat will get adjusted by adjust_for_oes_float_texture > after these checks. > > v2: simplify to check vs supported enums > > Signed-off-by: Tapani Pälli <[email protected]> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90748 > --- > src/mesa/main/glformats.c | 43 +++++++++++++++++++++++++++++++++++-------- > 1 file changed, 35 insertions(+), 8 deletions(-) >
This looks right to me. A couple comments inline...
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index 057a5d1..119956e 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -2295,20 +2295,32 @@ _mesa_es3_error_check_format_and_type(const struct
> gl_context *ctx,
> return GL_INVALID_OPERATION;
> break;
>
> + case GL_HALF_FLOAT_OES:
> case GL_HALF_FLOAT:
> - if (internalFormat != GL_RG16F)
> - return GL_INVALID_OPERATION;
> - break;
> + switch (internalFormat) {
> + case GL_RG:
> + if (!ctx->Extensions.ARB_texture_rg ||
> + !ctx->Extensions.OES_texture_half_float)
> + break;
> + case GL_RG16F:
> + return GL_NO_ERROR;
> + }
> +
> + return GL_INVALID_OPERATION;
>
> case GL_FLOAT:
> switch (internalFormat) {
> + case GL_RG:
> + if (!ctx->Extensions.ARB_texture_rg ||
> + !ctx->Extensions.OES_texture_float)
> + break;
> case GL_RG16F:
> case GL_RG32F:
> + return GL_NO_ERROR;
> break;
> default:
> return GL_INVALID_OPERATION;
> }
> - break;
The rest of the function breaks out of the switch statement when a
format is allowed, and falls through to the default in order to return
GL_INVALID_OPERATION. This doesn't fit that style.
I think you could instead do:
case GL_FLOAT:
switch (internalFormat) {
case GL_RG:
if (!ctx->Extensions.ARB_texture_rg ||
!ctx->Extensions.OES_texture_float)
return GL_INVALID_OPERATION;
break;
case GL_RG16F:
case GL_RG32F:
break;
default:
return GL_INVALID_OPERATION;
}
which just adds a new case and doesn't alter the rest of the control
flow. Alternatively, you could do this, which fits the existing style
pretty well:
case GL_FLOAT:
switch (internalFormat) {
case GL_RG16F:
case GL_RG32F:
break;
case GL_RG:
if (ctx->Extensions.ARB_texture_rg &&
ctx->Extensions.OES_texture_float)
break;
/* fallthrough */
default:
return GL_INVALID_OPERATION;
}
>
> default:
> return GL_INVALID_OPERATION;
> @@ -2365,19 +2377,34 @@ _mesa_es3_error_check_format_and_type(const struct
> gl_context *ctx,
> break;
>
> case GL_HALF_FLOAT:
> - if (internalFormat != GL_R16F)
> - return GL_INVALID_OPERATION;
> - break;
> + case GL_HALF_FLOAT_OES:
> +
> + switch (internalFormat) {
> + case GL_RG:
> + case GL_RED:
> + if (!ctx->Extensions.ARB_texture_rg ||
> + !ctx->Extensions.OES_texture_half_float)
> + break;
> + case GL_R16F:
> + return GL_NO_ERROR;
> + }
> +
> + return GL_INVALID_OPERATION;
>
> case GL_FLOAT:
> +
> switch (internalFormat) {
> + case GL_RED:
> + if (!ctx->Extensions.ARB_texture_rg ||
> + !ctx->Extensions.OES_texture_float)
> + break;
> case GL_R16F:
> case GL_R32F:
> + return GL_NO_ERROR;
> break;
> default:
> return GL_INVALID_OPERATION;
> }
> - break;
>
> default:
> return GL_INVALID_OPERATION;
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
