[Mesa-dev] [PATCH] mesa: Correctly set base and max texture levels in glTexparameterf

[Paul Berry]

On 2 January 2013 07:38, Anuj Phogat <anuj.phogat at gmail.com> wrote:

> gles3 conformance sgis_texture_lod_basic_getter.test expects base
> and max texture levels to be rounded to nearest integer. e.g. round
> 2.3 to 2 and 2.6 to 3. OpenGL 3.0 and OpenGL ES 3.0 specifications
> suggest similar rounding to select level when filtering mode is
> GL_NEAREST.
>
> This patch makes sgis_texture_lod_basic_getter.test pass.
>

It looks to me from reading the spec like this logic should be applied in
all cases where _mesa_TexParameterf converts floats to ints, not just base
and max texture level.  GL 4.3, section 8.10 ("Texture Parameters") simply
says "Data conversions are performed as specified in section 2.2.1.", and
section 2.2.1 ("Data Conversion For State-Setting Commands") says that
"Floating-point values are rounded to the nearest integer."  There's no
mention that this rule should apply only to base and max texture level.

So rather than create a separate case in the switch statement to handle
GL_TEXTURE_BASE_LEVEL and GL_TEXTURE_MAX_LEVEL, I'd recommend just fixing
the existing case to do the rounding properly.

Also, the switch statement as it exists today contains two separate blocks
with identical code (one to handle GL_TEXTURE_SWIZZLE_{R,G,B,A}_EXT and one
to handle all other float-to-int conversions).  That's silly--we should
have just one block.

One other comment below:


>
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  src/mesa/main/texparam.c |   24 ++++++++++++++++++++++--
>  1 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> index ca5a21f..f3f97ca 100644
> --- a/src/mesa/main/texparam.c
> +++ b/src/mesa/main/texparam.c
> @@ -654,8 +654,6 @@ _mesa_TexParameterf(GLenum target, GLenum pname,
> GLfloat param)
>     case GL_TEXTURE_WRAP_S:
>     case GL_TEXTURE_WRAP_T:
>     case GL_TEXTURE_WRAP_R:
> -   case GL_TEXTURE_BASE_LEVEL:
> -   case GL_TEXTURE_MAX_LEVEL:
>     case GL_GENERATE_MIPMAP_SGIS:
>     case GL_TEXTURE_COMPARE_MODE_ARB:
>     case GL_TEXTURE_COMPARE_FUNC_ARB:
> @@ -670,6 +668,28 @@ _mesa_TexParameterf(GLenum target, GLenum pname,
> GLfloat param)
>           need_update = set_tex_parameteri(ctx, texObj, pname, p);
>        }
>        break;
> +   case GL_TEXTURE_BASE_LEVEL:
> +   case GL_TEXTURE_MAX_LEVEL:
> +      {
> +         GLint p[4];
> +         /* Check if param exceeds maximum value an integer can hold */
> +         if (param > 2147483647.0f) {
> +            p[0] = (GLint) 2147483647.0f;
> +         }
> +         else if (param < 0.0f) {
> +            p[0] = 0;
> +        }
>

I'm not convinced that this amount of thoroughness is necessary.  There's
no language in the spec (or test cases that I'm aware of) to indicate that
out-of-range floats need to be translated to INT_MAX or 0.  I think it
would be adequate to just do "p[0] = (int) round(param);" and just let the
behaviour be undefined for out-of-range values.


> +         else {
> +            /* Round float param to nearest integer. e.g round 2.3 to 2
> +             * and 2.8 to 3.
> +             */
> +            p[0] = (GLint) (param + 0.5);
> +         }
> +
> +        p[1] = p[2] = p[3] = 0;
> +         need_update = set_tex_parameteri(ctx, texObj, pname, p);
> +      }
> +      break;
>     case GL_TEXTURE_SWIZZLE_R_EXT:
>     case GL_TEXTURE_SWIZZLE_G_EXT:
>     case GL_TEXTURE_SWIZZLE_B_EXT:
> --
> 1.7.7.6
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130102/b668eec5/attachment.html>