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

[Tapani Pälli]

On 11/21/2013 12:04 AM, Kenneth Graunke wrote:

On 11/20/2013 09:45 AM, Ian Romanick wrote:

On 11/20/2013 09:08 AM, Kenneth Graunke wrote:

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-sta...@lists.freedesktop.org>
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
Signed-off-by: Tapani Pälli <tapani.pa...@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.

But his patch modifies GetTexParameter.  There are already separate
paths for GetTexParameterfv and GetTexParameteriv.  Note the last two
hunks of the patch.

You're right; I totally read the wrong thing...so my concern is invalid,
and this patch is probably fine.

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.

I don't see anything in _mesa_GetTexParameteriv that's incorrectly
handling floats.  The only other floats that I noticed were colors or
other values that get returned as fixed-point.  Did you notice anything
specific?  Or were you looking at GetTexLevelParameteriv? :)

GL_TEXTURE_MAX_ANISOTROPY is also a float.  I think that's the only one,
though, so maybe that can just get fixed separately.  *shrug*

ok, I'll add this to the patch.

--Ken

// Tapani

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev