On 06/09/2015 11:25 AM, Kenneth Graunke wrote:
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;
}
OK, thanks for checking this. I'll send another version following the style.
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;
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
