<div dir="ltr"><div><div><div>What we want here is c = cosf( angle * (float)(M_PI / 180.0) ); </div>so the compiler can evaluate the constant expression and use a float representation for the result. </div>I always find it a little awkward deciding between using OpenGL's types like GLfloat or simple language types when working with system functions like cosf that are not signed up for OpenGL's "portable" types. </div>In this case I think a simple cast to float is a better fit. </div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Jul 14, 2015 at 5:39 AM, Iago Toral <<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, 2015-07-14 at 13:19 +0200, Iago Toral wrote: > On Mon, 2015-07-13 at 16:22 -0700, Matt Turner wrote: > > --- > > src/mesa/math/m_clip_tmp.h | 20 ++++++------- > > src/mesa/math/m_matrix.c | 70 +++++++++++++++++++++++----------------------- > > src/mesa/math/m_norm_tmp.h | 2 +- > > 3 files changed, 46 insertions(+), 46 deletions(-) > > > > diff --git a/src/mesa/math/m_clip_tmp.h b/src/mesa/math/m_clip_tmp.h > > index e289be7..60c0004 100644 > > --- a/src/mesa/math/m_clip_tmp.h > > +++ b/src/mesa/math/m_clip_tmp.h > > @@ -194,13 +194,13 @@ static GLvector4f * TAG(cliptest_points3)( GLvector4f *clip_vec, > > STRIDE_LOOP { > > const GLfloat cx = from[0], cy = from[1], cz = from[2]; > > GLubyte mask = 0; > > - if (cx > 1.0) mask |= CLIP_RIGHT_BIT; > > - else if (cx < -1.0) mask |= CLIP_LEFT_BIT; > > - if (cy > 1.0) mask |= CLIP_TOP_BIT; > > - else if (cy < -1.0) mask |= CLIP_BOTTOM_BIT; > > + if (cx > 1.0F) mask |= CLIP_RIGHT_BIT; > > + else if (cx < -1.0F) mask |= CLIP_LEFT_BIT; > > + if (cy > 1.0F) mask |= CLIP_TOP_BIT; > > + else if (cy < -1.0F) mask |= CLIP_BOTTOM_BIT; > > if (viewport_z_clip) { > > - if (cz > 1.0) mask |= CLIP_FAR_BIT; > > - else if (cz < -1.0) mask |= CLIP_NEAR_BIT; > > + if (cz > 1.0F) mask |= CLIP_FAR_BIT; > > + else if (cz < -1.0F) mask |= CLIP_NEAR_BIT; > > } > > clipMask[i] = mask; > > tmpOrMask |= mask; > > @@ -230,10 +230,10 @@ static GLvector4f * TAG(cliptest_points2)( GLvector4f *clip_vec, > > STRIDE_LOOP { > > const GLfloat cx = from[0], cy = from[1]; > > GLubyte mask = 0; > > - if (cx > 1.0) mask |= CLIP_RIGHT_BIT; > > - else if (cx < -1.0) mask |= CLIP_LEFT_BIT; > > - if (cy > 1.0) mask |= CLIP_TOP_BIT; > > - else if (cy < -1.0) mask |= CLIP_BOTTOM_BIT; > > + if (cx > 1.0F) mask |= CLIP_RIGHT_BIT; > > + else if (cx < -1.0F) mask |= CLIP_LEFT_BIT; > > + if (cy > 1.0F) mask |= CLIP_TOP_BIT; > > + else if (cy < -1.0F) mask |= CLIP_BOTTOM_BIT; > > clipMask[i] = mask; > > tmpOrMask |= mask; > > tmpAndMask &= mask; > > diff --git a/src/mesa/math/m_matrix.c b/src/mesa/math/m_matrix.c > > index 6a42c6c..6522200 100644 > > --- a/src/mesa/math/m_matrix.c > > +++ b/src/mesa/math/m_matrix.c > > @@ -380,7 +380,7 @@ static GLboolean invert_matrix_general( GLmatrix *mat ) > > if (fabsf(r3[0])>fabsf(r2[0])) SWAP_ROWS(r3, r2); > > if (fabsf(r2[0])>fabsf(r1[0])) SWAP_ROWS(r2, r1); > > if (fabsf(r1[0])>fabsf(r0[0])) SWAP_ROWS(r1, r0); > > - if (0.0 == r0[0]) return GL_FALSE; > > + if (0.0F == r0[0]) return GL_FALSE; > > > > /* eliminate first variable */ > > m1 = r1[0]/r0[0]; m2 = r2[0]/r0[0]; m3 = r3[0]/r0[0]; > > @@ -388,31 +388,31 @@ static GLboolean invert_matrix_general( GLmatrix *mat ) > > s = r0[2]; r1[2] -= m1 * s; r2[2] -= m2 * s; r3[2] -= m3 * s; > > s = r0[3]; r1[3] -= m1 * s; r2[3] -= m2 * s; r3[3] -= m3 * s; > > s = r0[4]; > > - if (s != 0.0) { r1[4] -= m1 * s; r2[4] -= m2 * s; r3[4] -= m3 * s; } > > + if (s != 0.0F) { r1[4] -= m1 * s; r2[4] -= m2 * s; r3[4] -= m3 * s; } > > s = r0[5]; > > - if (s != 0.0) { r1[5] -= m1 * s; r2[5] -= m2 * s; r3[5] -= m3 * s; } > > + if (s != 0.0F) { r1[5] -= m1 * s; r2[5] -= m2 * s; r3[5] -= m3 * s; } > > s = r0[6]; > > - if (s != 0.0) { r1[6] -= m1 * s; r2[6] -= m2 * s; r3[6] -= m3 * s; } > > + if (s != 0.0F) { r1[6] -= m1 * s; r2[6] -= m2 * s; r3[6] -= m3 * s; } > > s = r0[7]; > > - if (s != 0.0) { r1[7] -= m1 * s; r2[7] -= m2 * s; r3[7] -= m3 * s; } > > + if (s != 0.0F) { r1[7] -= m1 * s; r2[7] -= m2 * s; r3[7] -= m3 * s; } > > > > /* choose pivot - or die */ > > if (fabsf(r3[1])>fabsf(r2[1])) SWAP_ROWS(r3, r2); > > if (fabsf(r2[1])>fabsf(r1[1])) SWAP_ROWS(r2, r1); > > - if (0.0 == r1[1]) return GL_FALSE; > > + if (0.0F == r1[1]) return GL_FALSE; > > > > /* eliminate second variable */ > > m2 = r2[1]/r1[1]; m3 = r3[1]/r1[1]; > > r2[2] -= m2 * r1[2]; r3[2] -= m3 * r1[2]; > > r2[3] -= m2 * r1[3]; r3[3] -= m3 * r1[3]; > > - s = r1[4]; if (0.0 != s) { r2[4] -= m2 * s; r3[4] -= m3 * s; } > > - s = r1[5]; if (0.0 != s) { r2[5] -= m2 * s; r3[5] -= m3 * s; } > > - s = r1[6]; if (0.0 != s) { r2[6] -= m2 * s; r3[6] -= m3 * s; } > > - s = r1[7]; if (0.0 != s) { r2[7] -= m2 * s; r3[7] -= m3 * s; } > > + s = r1[4]; if (0.0F != s) { r2[4] -= m2 * s; r3[4] -= m3 * s; } > > + s = r1[5]; if (0.0F != s) { r2[5] -= m2 * s; r3[5] -= m3 * s; } > > + s = r1[6]; if (0.0F != s) { r2[6] -= m2 * s; r3[6] -= m3 * s; } > > + s = r1[7]; if (0.0F != s) { r2[7] -= m2 * s; r3[7] -= m3 * s; } > > > > /* choose pivot - or die */ > > if (fabsf(r3[2])>fabsf(r2[2])) SWAP_ROWS(r3, r2); > > - if (0.0 == r2[2]) return GL_FALSE; > > + if (0.0F == r2[2]) return GL_FALSE; > > > > /* eliminate third variable */ > > m3 = r3[2]/r2[2]; > > @@ -421,7 +421,7 @@ static GLboolean invert_matrix_general( GLmatrix *mat ) > > r3[7] -= m3 * r2[7]; > > > > /* last check */ > > - if (0.0 == r3[3]) return GL_FALSE; > > + if (0.0F == r3[3]) return GL_FALSE; > > > > s = 1.0F/r3[3]; /* now back substitute row 3 */ > > r3[4] *= s; r3[5] *= s; r3[6] *= s; r3[7] *= s; > > @@ -490,26 +490,26 @@ static GLboolean invert_matrix_3d_general( GLmatrix *mat ) > > */ > > pos = neg = 0.0; > > t = MAT(in,0,0) * MAT(in,1,1) * MAT(in,2,2); > > - if (t >= 0.0) pos += t; else neg += t; > > + if (t >= 0.0F) pos += t; else neg += t; > > > > t = MAT(in,1,0) * MAT(in,2,1) * MAT(in,0,2); > > - if (t >= 0.0) pos += t; else neg += t; > > + if (t >= 0.0F) pos += t; else neg += t; > > > > t = MAT(in,2,0) * MAT(in,0,1) * MAT(in,1,2); > > - if (t >= 0.0) pos += t; else neg += t; > > + if (t >= 0.0F) pos += t; else neg += t; > > > > t = -MAT(in,2,0) * MAT(in,1,1) * MAT(in,0,2); > > - if (t >= 0.0) pos += t; else neg += t; > > + if (t >= 0.0F) pos += t; else neg += t; > > > > t = -MAT(in,1,0) * MAT(in,0,1) * MAT(in,2,2); > > - if (t >= 0.0) pos += t; else neg += t; > > + if (t >= 0.0F) pos += t; else neg += t; > > > > t = -MAT(in,0,0) * MAT(in,2,1) * MAT(in,1,2); > > - if (t >= 0.0) pos += t; else neg += t; > > + if (t >= 0.0F) pos += t; else neg += t; > > > > det = pos + neg; > > > > - if (fabsf(det) < 1e-25) > > + if (fabsf(det) < 1e-25F) > > return GL_FALSE; > > > > det = 1.0F / det; > > @@ -564,7 +564,7 @@ static GLboolean invert_matrix_3d( GLmatrix *mat ) > > MAT(in,0,1) * MAT(in,0,1) + > > MAT(in,0,2) * MAT(in,0,2)); > > > > - if (scale == 0.0) > > + if (scale == 0.0F) > > return GL_FALSE; > > > > scale = 1.0F / scale; > > @@ -799,8 +799,8 @@ _math_matrix_rotate( GLmatrix *mat, > > GLfloat m[16]; > > GLboolean optimized; > > > > - s = (GLfloat) sin( angle * M_PI / 180.0 ); > > - c = (GLfloat) cos( angle * M_PI / 180.0 ); > > + s = sinf( angle * M_PI / 180.0 ); > > 180.0F > > > + c = cosf( angle * M_PI / 180.0 ); > > 180.0F </div></div>I have just realized that M_PI is double so the angle we pass to cosf/sinf is going to be a double in the end... I wonder if that is what we want here since in the end it seems we want a float result... but I guess that is a different topic. In any case, as it is, is there any gain with your changes? Before we had double->float conversion in the result, but now we have a double->float conversion in the argument to sinf/cosf, right? Iago <div class="HOEnZb"><div class="h5"> > I guess we will need wrappers for sinf and cosf. > > Reviewed-by: Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> > > > > > memcpy(m, Identity, sizeof(GLfloat)*16); > > optimized = GL_FALSE; > > @@ -859,7 +859,7 @@ _math_matrix_rotate( GLmatrix *mat, > > if (!optimized) { > > const GLfloat mag = sqrtf(x * x + y * y + z * z); > > > > - if (mag <= 1.0e-4) { > > + if (mag <= 1.0e-4F) { > > /* no rotation, leave mat as-is */ > > return; > > } > > @@ -1070,7 +1070,7 @@ _math_matrix_scale( GLmatrix *mat, GLfloat x, GLfloat y, GLfloat z ) > > m[2] *= x; m[6] *= y; m[10] *= z; > > m[3] *= x; m[7] *= y; m[11] *= z; > > > > - if (fabsf(x - y) < 1e-8 && fabsf(x - z) < 1e-8) > > + if (fabsf(x - y) < 1e-8F && fabsf(x - z) < 1e-8F) > > mat->flags |= MAT_FLAG_UNIFORM_SCALE; > > else > > mat->flags |= MAT_FLAG_GENERAL_SCALE; > > @@ -1206,7 +1206,7 @@ static void analyse_from_scratch( GLmatrix *mat ) > > GLuint i; > > > > for (i = 0 ; i < 16 ; i++) { > > - if (m[i] == 0.0) mask |= (1<<i); > > + if (m[i] == 0.0F) mask |= (1<<i); > > } > > > > if (m[0] == 1.0F) mask |= (1<<16); > > @@ -1240,12 +1240,12 @@ static void analyse_from_scratch( GLmatrix *mat ) > > mat->type = MATRIX_2D; > > > > /* Check for scale */ > > - if (SQ(mm-1) > SQ(1e-6) || > > - SQ(m4m4-1) > SQ(1e-6)) > > + if (SQ(mm-1) > SQ(1e-6F) || > > + SQ(m4m4-1) > SQ(1e-6F)) > > mat->flags |= MAT_FLAG_GENERAL_SCALE; > > > > /* Check for rotation */ > > - if (SQ(mm4) > SQ(1e-6)) > > + if (SQ(mm4) > SQ(1e-6F)) > > mat->flags |= MAT_FLAG_GENERAL_3D; > > else > > mat->flags |= MAT_FLAG_ROTATION; > > @@ -1255,9 +1255,9 @@ static void analyse_from_scratch( GLmatrix *mat ) > > mat->type = MATRIX_3D_NO_ROT; > > > > /* Check for scale */ > > - if (SQ(m[0]-m[5]) < SQ(1e-6) && > > - SQ(m[0]-m[10]) < SQ(1e-6)) { > > - if (SQ(m[0]-1.0) > SQ(1e-6)) { > > + if (SQ(m[0]-m[5]) < SQ(1e-6F) && > > + SQ(m[0]-m[10]) < SQ(1e-6F)) { > > + if (SQ(m[0]-1.0F) > SQ(1e-6F)) { > > mat->flags |= MAT_FLAG_UNIFORM_SCALE; > > } > > } > > @@ -1275,8 +1275,8 @@ static void analyse_from_scratch( GLmatrix *mat ) > > mat->type = MATRIX_3D; > > > > /* Check for scale */ > > - if (SQ(c1-c2) < SQ(1e-6) && SQ(c1-c3) < SQ(1e-6)) { > > - if (SQ(c1-1.0) > SQ(1e-6)) > > + if (SQ(c1-c2) < SQ(1e-6F) && SQ(c1-c3) < SQ(1e-6F)) { > > + if (SQ(c1-1.0F) > SQ(1e-6F)) > > mat->flags |= MAT_FLAG_UNIFORM_SCALE; > > /* else no scale at all */ > > } > > @@ -1285,10 +1285,10 @@ static void analyse_from_scratch( GLmatrix *mat ) > > } > > > > /* Check for rotation */ > > - if (SQ(d1) < SQ(1e-6)) { > > + if (SQ(d1) < SQ(1e-6F)) { > > CROSS3( cp, m, m+4 ); > > SUB_3V( cp, cp, (m+8) ); > > - if (LEN_SQUARED_3FV(cp) < SQ(1e-6)) > > + if (LEN_SQUARED_3FV(cp) < SQ(1e-6F)) > > mat->flags |= MAT_FLAG_ROTATION; > > else > > mat->flags |= MAT_FLAG_GENERAL_3D; > > diff --git a/src/mesa/math/m_norm_tmp.h b/src/mesa/math/m_norm_tmp.h > > index d3ec1c2..6f1db8d 100644 > > --- a/src/mesa/math/m_norm_tmp.h > > +++ b/src/mesa/math/m_norm_tmp.h > > @@ -80,7 +80,7 @@ TAG(transform_normalize_normals)( const GLmatrix *mat, > > } > > } > > else { > > - if (scale != 1.0) { > > + if (scale != 1.0f) { > > m0 *= scale, m4 *= scale, m8 *= scale; > > m1 *= scale, m5 *= scale, m9 *= scale; > > m2 *= scale, m6 *= scale, m10 *= scale; > > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> -- <div class="gmail_signature"> Mike Stroyan - Software Architect LunarG, Inc. - The Graphics Experts Cell: (970) 219-7905 Email: Mike@LunarG.com Website: <a href="http://www.lunarg.com" target="_blank">http://www.lunarg.com</a></div> </div>