[Mesa-dev] [PATCH] mesa: enable GL_TEXTURE_LOD_BIAS set/get v2

[Kenneth Graunke]

On 11/20/2013 03:27 AM, Tapani Pälli wrote:
> Earlier comments suggest this was removed from GL core spec but it is
> still there. Enabling makes 'texture_lod_bias_getter' Khronos
> conformance tests pass, also removes some errors from Metro Last Light
> game which is using this API.
> 
> v2: leave NOTE comment (Ian)
> 
> Cc: "9.0 9.1 9.2 10.0" <mesa-stable at lists.freedesktop.org>
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  src/mesa/main/texparam.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> index d56b7d9..f77e7f6 100644
> --- a/src/mesa/main/texparam.c
> +++ b/src/mesa/main/texparam.c
> @@ -684,11 +684,8 @@ set_tex_parameterf(struct gl_context *ctx,
>        return GL_FALSE;
>  
>     case GL_TEXTURE_LOD_BIAS:
> -      /* NOTE: this is really part of OpenGL 1.4, not EXT_texture_lod_bias.
> -       * It was removed in core-profile, and it has never existed in OpenGL
> -       * ES.
> -       */
> -      if (ctx->API != API_OPENGL_COMPAT)
> +      /* NOTE: this is really part of OpenGL 1.4, not EXT_texture_lod_bias. */
> +      if (_mesa_is_gles(ctx))
>           goto invalid_pname;
>  
>        if (!target_allows_setting_sampler_parameters(texObj->Target))
> @@ -1513,7 +1510,7 @@ _mesa_GetTexParameterfv( GLenum target, GLenum pname, GLfloat *params )
>           *params = (GLfloat) obj->DepthMode;
>           break;
>        case GL_TEXTURE_LOD_BIAS:
> -         if (ctx->API != API_OPENGL_COMPAT)
> +         if (_mesa_is_gles(ctx))
>              goto invalid_pname;
>  
>           *params = obj->Sampler.LodBias;
> @@ -1701,10 +1698,13 @@ _mesa_GetTexParameteriv( GLenum target, GLenum pname, GLint *params )
>           *params = (GLint) obj->DepthMode;
>           break;
>        case GL_TEXTURE_LOD_BIAS:
> -         if (ctx->API != API_OPENGL_COMPAT)
> +         if (_mesa_is_gles(ctx))
>              goto invalid_pname;
>  
> -         *params = (GLint) obj->Sampler.LodBias;
> +         /* GL spec 'Data Conversions' section specifies that floating-point
> +          * value in integer Get function is rounded to nearest integer
> +          */
> +         *params = (GLint) roundf(obj->Sampler.LodBias);

Yikes!  I missed this change in the original patch.

The call to roundf here is definitely wrong.  If you look above, you'll
find...

void GLAPIENTRY
_mesa_GetTexLevelParameterfv( GLenum target, GLint level,
                              GLenum pname, GLfloat *params )
{
   GLint iparam;
   _mesa_GetTexLevelParameteriv( target, level, pname, &iparam );
   *params = (GLfloat) iparam;
}

...so, your change means that we'll always round to the nearest integer,
even for GetTexLevelParameterfv.

I think you should drop the roundf change from this patch, making it
entirely about exposing the values.  Later patches can fix the values.

The other thing that concerns me is that there are other floating point
values being returned, and none of them have this roundf treatment.
Assuming the justification in your comment is correct (I haven't
checked), then they'd all be wrong, and all need to be fixed.

>           break;
>        case GL_TEXTURE_CROP_RECT_OES:
>           if (ctx->API != API_OPENGLES || !ctx->Extensions.OES_draw_texture)
>