[Mesa-dev] [PATCH v2] mesa: add GL_RED, GL_RG support for floating point textures
Tapani Pälli
tapani.palli at intel.com
Tue Jun 9 02:23:43 PDT 2015
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 <tapani.palli at intel.com>
>> 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;
>>
More information about the mesa-dev
mailing list