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.
> 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? :)
>> break;
>> case GL_TEXTURE_CROP_RECT_OES:
>> if (ctx->API != API_OPENGLES || !ctx->Extensions.OES_draw_texture)
>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
