[Mesa-dev] [PATCH 09/13] tnl: Avoid double promotion.

[Matt Turner]

On Tue, Jul 14, 2015 at 3:53 AM, Iago Toral <itoral at igalia.com> wrote:
> On Mon, 2015-07-13 at 16:22 -0700, Matt Turner wrote:
>> There are a couple of unrelated changes in t_vb_lighttmp.h that I hope
>> you'll excuse -- there's a block of code that's duplicated modulo a few
>> trivial differences that I took the liberty of fixing.
>> ---
>>  src/mesa/tnl/t_draw.c           |  2 +-
>>  src/mesa/tnl/t_rasterpos.c      |  6 +++---
>>  src/mesa/tnl/t_vb_fog.c         |  6 +++---
>>  src/mesa/tnl/t_vb_light.c       | 16 ++++++++--------
>>  src/mesa/tnl/t_vb_lighttmp.h    | 16 +++++++---------
>>  src/mesa/tnl/t_vb_normals.c     |  4 ++--
>>  src/mesa/tnl/t_vertex_generic.c |  2 +-
>>  7 files changed, 25 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/mesa/tnl/t_draw.c b/src/mesa/tnl/t_draw.c
>> index 6adf1dc..713c6a9 100644
>> --- a/src/mesa/tnl/t_draw.c
>> +++ b/src/mesa/tnl/t_draw.c
>> @@ -257,7 +257,7 @@ static GLboolean *_tnl_import_edgeflag( struct gl_context *ctx,
>>     GLuint i;
>>
>>     for (i = 0; i < count; i++) {
>> -      *bptr++ = ((GLfloat *)ptr)[0] == 1.0;
>> +      *bptr++ = ((GLfloat *)ptr)[0] == 1.0F;
>>        ptr += stride;
>>     }
>>
>> diff --git a/src/mesa/tnl/t_rasterpos.c b/src/mesa/tnl/t_rasterpos.c
>> index 7ef50ea..4bd9ac8 100644
>> --- a/src/mesa/tnl/t_rasterpos.c
>> +++ b/src/mesa/tnl/t_rasterpos.c
>> @@ -148,7 +148,7 @@ shade_rastpos(struct gl_context *ctx,
>>        SUB_3V(VP, light->_Position, vertex);
>>           /* d = length(VP) */
>>        d = (GLfloat) LEN_3FV( VP );
>> -      if (d > 1.0e-6) {
>> +      if (d > 1.0e-6F) {
>>              /* normalize VP */
>>           GLfloat invd = 1.0F / d;
>>           SELF_SCALE_SCALAR_3V(VP, invd);
>> @@ -172,7 +172,7 @@ shade_rastpos(struct gl_context *ctx,
>>        }
>>        }
>>
>> -      if (attenuation < 1e-3)
>> +      if (attenuation < 1e-3F)
>>        continue;
>>
>>        n_dot_VP = DOT3( normal, VP );
>> @@ -219,7 +219,7 @@ shade_rastpos(struct gl_context *ctx,
>>           shine = ctx->Light.Material.Attrib[MAT_ATTRIB_FRONT_SHININESS][0];
>>           spec_coef = powf(n_dot_h, shine);
>>
>> -         if (spec_coef > 1.0e-10) {
>> +         if (spec_coef > 1.0e-10F) {
>>                 if (ctx->Light.Model.ColorControl==GL_SEPARATE_SPECULAR_COLOR) {
>>                    ACC_SCALE_SCALAR_3V( specularContrib, spec_coef,
>>                                         light->_MatSpecular[0]);
>> diff --git a/src/mesa/tnl/t_vb_fog.c b/src/mesa/tnl/t_vb_fog.c
>> index 1ca72f8..5489ed6 100644
>> --- a/src/mesa/tnl/t_vb_fog.c
>> +++ b/src/mesa/tnl/t_vb_fog.c
>> @@ -45,8 +45,8 @@ struct fog_stage_data {
>>  #define FOG_STAGE_DATA(stage) ((struct fog_stage_data *)stage->privatePtr)
>>
>>  #define FOG_EXP_TABLE_SIZE 256
>> -#define FOG_MAX (10.0)
>> -#define EXP_FOG_MAX .0006595
>> +#define FOG_MAX (10.0F)
>> +#define EXP_FOG_MAX .0006595F
>>  #define FOG_INCR (FOG_MAX/FOG_EXP_TABLE_SIZE)
>>  static GLfloat exp_table[FOG_EXP_TABLE_SIZE];
>>  static GLfloat inited = 0;
>> @@ -54,7 +54,7 @@ static GLfloat inited = 0;
>>  #if 1
>>  #define NEG_EXP( result, narg )                                              \
>>  do {                                                                 \
>> -   GLfloat f = (GLfloat) (narg * (1.0/FOG_INCR));                    \
>> +   GLfloat f = (GLfloat) (narg * (1.0F / FOG_INCR));                 \
>>     GLint k = (GLint) f;                                                      \
>>     if (k > FOG_EXP_TABLE_SIZE-2)                                     \
>>        result = (GLfloat) EXP_FOG_MAX;                                        \
>> diff --git a/src/mesa/tnl/t_vb_light.c b/src/mesa/tnl/t_vb_light.c
>> index dbd57fa..df9073e 100644
>> --- a/src/mesa/tnl/t_vb_light.c
>> +++ b/src/mesa/tnl/t_vb_light.c
>> @@ -137,23 +137,23 @@ validate_shine_table( struct gl_context *ctx, GLuint side, GLfloat shininess )
>>           break;
>>
>>        m = s->tab;
>> -      m[0] = 0.0;
>> -      if (shininess == 0.0) {
>> +      m[0] = 0.0F;
>> +      if (shininess == 0.0F) {
>>        for (j = 1 ; j <= SHINE_TABLE_SIZE ; j++)
>> -         m[j] = 1.0;
>> +         m[j] = 1.0F;
>>        }
>>        else {
>>        for (j = 1 ; j < SHINE_TABLE_SIZE ; j++) {
>>              GLdouble t, x = j / (GLfloat) (SHINE_TABLE_SIZE - 1);
>
> I think you want to declare x (and probably t as well) with type GLfloat
> here.
>
>> -            if (x < 0.005) /* underflow check */
>> -               x = 0.005;
>> +            if (x < 0.005F) /* underflow check */
>> +               x = 0.005F;
>>              t = pow(x, shininess);
>
> Since the code below casts t to float anyway, is there a reason why you
> did not use powf and declared t as float? Also, there is no point in
> making the change below if you don't do that, right?

Totally right. Thanks for the nice review!

I've made t and x GLfloat, s/pow/powf/, and removed the GLfloat cast from t.