<div dir="ltr"><div><div> </div>I don't remember the specifics, but I recall some of the old conformance tests being sensitive to float->int conversion in a few areas. Matt, if you have access to the conform tests and can run the old swrast driver with your change, that'd be interesting. </div>-Brian <div><div><div class="gmail_extra"> <div class="gmail_quote">On Fri, Jul 31, 2015 at 7:52 PM, Roland Scheidegger <<a href="mailto:sroland@vmware.com" target="_blank">sroland@vmware.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I have some doubts of this. Given that IROUND was just plain incorrect, I think we should just use _mesa_lroundevenf() and see what happens. lroundf() is quite the shocker, my libm's implementation is totalling 40 instructions (!) for it (not counting the last return, vs 1 for lrintf). Now it's probably not THAT bad (not all these 40 instructions will be executed), it still looks expensive (and gcc won't attempt to use a built-in for that mess, quite understandable). I don't see GL mentioning requiring such explicit rounding in general (though for instance the EXT_texture_shared_exponent does this implicitly, albeit I'm wondering if that was by choice or accident...). Different tie breaker (which we'd get with lrintf) sounds like a win over just bogus rounding (which we got before) any day to me. Roland <div class="HOEnZb"><div class="h5"> Am 01.08.2015 um 01:26 schrieb Matt Turner: > lroundf is the most common replacement. I replaced uses of IROUND() > where there was a comment saying "rounded to nearest integer" with > _mesa_lroundevenf. > > IROUND64 is replaced with llroundf. > --- > src/mesa/main/drawpix.c | 21 +++++++++++---------- > src/mesa/main/eval.c | 14 +++++++------- > src/mesa/main/get.c | 28 ++++++++++++++-------------- > src/mesa/main/imports.h | 18 ------------------ > src/mesa/main/light.c | 8 ++++---- > src/mesa/main/pixel.c | 2 +- > src/mesa/main/pixelstore.c | 3 ++- > src/mesa/main/samplerobj.c | 8 ++++---- > src/mesa/main/texparam.c | 8 ++++---- > src/mesa/main/uniform_query.cpp | 2 +- > src/mesa/swrast/s_blit.c | 2 +- > src/mesa/swrast/s_context.h | 2 +- > 12 files changed, 50 insertions(+), 66 deletions(-) > > diff --git a/src/mesa/main/drawpix.c b/src/mesa/main/drawpix.c > index 720a082..025cf7e 100644 > --- a/src/mesa/main/drawpix.c > +++ b/src/mesa/main/drawpix.c > @@ -22,6 +22,7 @@ > * OTHER DEALINGS IN THE SOFTWARE. > */ > > +#include "c99_math.h" > #include "glheader.h" > #include "imports.h" > #include "bufferobj.h" > @@ -51,14 +52,14 @@ _mesa_DrawPixels( GLsizei width, GLsizei height, > FLUSH_VERTICES(ctx, 0); > > if (MESA_VERBOSE & VERBOSE_API) > - _mesa_debug(ctx, "glDrawPixels(%d, %d, %s, %s, %p) // to %s at %d, %d\n", > + _mesa_debug(ctx, "glDrawPixels(%d, %d, %s, %s, %p) // to %s at %ld, %ld\n", > width, height, > _mesa_enum_to_string(format), > _mesa_enum_to_string(type), > pixels, > _mesa_enum_to_string(ctx->DrawBuffer->ColorDrawBuffer[0]), > - IROUND(ctx->Current.RasterPos[0]), > - IROUND(ctx->Current.RasterPos[1])); > + lroundf(ctx->Current.RasterPos[0]), > + lroundf(ctx->Current.RasterPos[1])); > > > if (width < 0 || height < 0) { > @@ -140,8 +141,8 @@ _mesa_DrawPixels( GLsizei width, GLsizei height, > if (ctx->RenderMode == GL_RENDER) { > if (width > 0 && height > 0) { > /* Round, to satisfy conformance tests (matches SGI's OpenGL) */ > - GLint x = IROUND(ctx->Current.RasterPos[0]); > - GLint y = IROUND(ctx->Current.RasterPos[1]); > + GLint x = lroundf(ctx->Current.RasterPos[0]); > + GLint y = lroundf(ctx->Current.RasterPos[1]); > > if (_mesa_is_bufferobj(ctx->Unpack.BufferObj)) { > /* unpack from PBO */ > @@ -196,13 +197,13 @@ _mesa_CopyPixels( GLint srcx, GLint srcy, GLsizei width, GLsizei height, > > if (MESA_VERBOSE & VERBOSE_API) > _mesa_debug(ctx, > - "glCopyPixels(%d, %d, %d, %d, %s) // from %s to %s at %d, %d\n", > + "glCopyPixels(%d, %d, %d, %d, %s) // from %s to %s at %ld, %ld\n", > srcx, srcy, width, height, > _mesa_enum_to_string(type), > _mesa_enum_to_string(ctx->ReadBuffer->ColorReadBuffer), > _mesa_enum_to_string(ctx->DrawBuffer->ColorDrawBuffer[0]), > - IROUND(ctx->Current.RasterPos[0]), > - IROUND(ctx->Current.RasterPos[1])); > + lroundf(ctx->Current.RasterPos[0]), > + lroundf(ctx->Current.RasterPos[1])); > > if (width < 0 || height < 0) { > _mesa_error(ctx, GL_INVALID_VALUE, "glCopyPixels(width or height < 0)"); > @@ -264,8 +265,8 @@ _mesa_CopyPixels( GLint srcx, GLint srcy, GLsizei width, GLsizei height, > if (ctx->RenderMode == GL_RENDER) { > /* Round to satisfy conformance tests (matches SGI's OpenGL) */ > if (width > 0 && height > 0) { > - GLint destx = IROUND(ctx->Current.RasterPos[0]); > - GLint desty = IROUND(ctx->Current.RasterPos[1]); > + GLint destx = lroundf(ctx->Current.RasterPos[0]); > + GLint desty = lroundf(ctx->Current.RasterPos[1]); > ctx->Driver.CopyPixels( ctx, srcx, srcy, width, height, destx, desty, > type ); > } > diff --git a/src/mesa/main/eval.c b/src/mesa/main/eval.c > index 86c8f75..b21a90d 100644 > --- a/src/mesa/main/eval.c > +++ b/src/mesa/main/eval.c > @@ -704,7 +704,7 @@ _mesa_GetnMapivARB( GLenum target, GLenum query, GLsizei bufSize, GLint *v ) > if (bufSize < numBytes) > goto overflow; > for (i=0;i<n;i++) { > - v[i] = IROUND(data[i]); > + v[i] = lroundf(data[i]); > } > } > break; > @@ -728,17 +728,17 @@ _mesa_GetnMapivARB( GLenum target, GLenum query, GLsizei bufSize, GLint *v ) > numBytes = 2 * sizeof *v; > if (bufSize < numBytes) > goto overflow; > - v[0] = IROUND(map1d->u1); > - v[1] = IROUND(map1d->u2); > + v[0] = lroundf(map1d->u1); > + v[1] = lroundf(map1d->u2); > } > else { > numBytes = 4 * sizeof *v; > if (bufSize < numBytes) > goto overflow; > - v[0] = IROUND(map2d->u1); > - v[1] = IROUND(map2d->u2); > - v[2] = IROUND(map2d->v1); > - v[3] = IROUND(map2d->v2); > + v[0] = lroundf(map2d->u1); > + v[1] = lroundf(map2d->u2); > + v[2] = lroundf(map2d->v1); > + v[3] = lroundf(map2d->v2); > } > break; > default: > diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c > index 307a5ff..5e6cd9b 100644 > --- a/src/mesa/main/get.c > +++ b/src/mesa/main/get.c > @@ -1527,13 +1527,13 @@ _mesa_GetIntegerv(GLenum pname, GLint *params) > break; > > case TYPE_FLOAT_4: > - params[3] = IROUND(((GLfloat *) p)[3]); > + params[3] = lroundf(((GLfloat *) p)[3]); > case TYPE_FLOAT_3: > - params[2] = IROUND(((GLfloat *) p)[2]); > + params[2] = lroundf(((GLfloat *) p)[2]); > case TYPE_FLOAT_2: > - params[1] = IROUND(((GLfloat *) p)[1]); > + params[1] = lroundf(((GLfloat *) p)[1]); > case TYPE_FLOAT: > - params[0] = IROUND(((GLfloat *) p)[0]); > + params[0] = lroundf(((GLfloat *) p)[0]); > break; > > case TYPE_FLOATN_4: > @@ -1621,13 +1621,13 @@ _mesa_GetInteger64v(GLenum pname, GLint64 *params) > break; > > case TYPE_FLOAT_4: > - params[3] = IROUND64(((GLfloat *) p)[3]); > + params[3] = llroundf(((GLfloat *) p)[3]); > case TYPE_FLOAT_3: > - params[2] = IROUND64(((GLfloat *) p)[2]); > + params[2] = llroundf(((GLfloat *) p)[2]); > case TYPE_FLOAT_2: > - params[1] = IROUND64(((GLfloat *) p)[1]); > + params[1] = llroundf(((GLfloat *) p)[1]); > case TYPE_FLOAT: > - params[0] = IROUND64(((GLfloat *) p)[0]); > + params[0] = llroundf(((GLfloat *) p)[0]); > break; > > case TYPE_FLOATN_4: > @@ -2110,22 +2110,22 @@ _mesa_GetIntegeri_v( GLenum pname, GLuint index, GLint *params ) > switch (type) { > case TYPE_FLOAT_4: > case TYPE_FLOATN_4: > - params[3] = IROUND(v.value_float_4[3]); > + params[3] = lroundf(v.value_float_4[3]); > case TYPE_FLOAT_3: > case TYPE_FLOATN_3: > - params[2] = IROUND(v.value_float_4[2]); > + params[2] = lroundf(v.value_float_4[2]); > case TYPE_FLOAT_2: > case TYPE_FLOATN_2: > - params[1] = IROUND(v.value_float_4[1]); > + params[1] = lroundf(v.value_float_4[1]); > case TYPE_FLOAT: > case TYPE_FLOATN: > - params[0] = IROUND(v.value_float_4[0]); > + params[0] = lroundf(v.value_float_4[0]); > break; > > case TYPE_DOUBLEN_2: > - params[1] = IROUND(v.value_double_2[1]); > + params[1] = lround(v.value_double_2[1]); > case TYPE_DOUBLEN: > - params[0] = IROUND(v.value_double_2[0]); > + params[0] = lround(v.value_double_2[0]); > break; > > case TYPE_INT: > diff --git a/src/mesa/main/imports.h b/src/mesa/main/imports.h > index d61279a..1d4b9c1 100644 > --- a/src/mesa/main/imports.h > +++ b/src/mesa/main/imports.h > @@ -144,24 +144,6 @@ static inline GLfloat LOG2(GLfloat x) > > > /** > - * Convert float to int by rounding to nearest integer, away from zero. > - */ > -static inline int IROUND(float f) > -{ > - return (int) ((f >= 0.0F) ? (f + 0.5F) : (f - 0.5F)); > -} > - > - > -/** > - * Convert float to int64 by rounding to nearest integer. > - */ > -static inline GLint64 IROUND64(float f) > -{ > - return (GLint64) ((f >= 0.0F) ? (f + 0.5F) : (f - 0.5F)); > -} > - > - > -/** > * Convert positive float to int by rounding to nearest integer. > */ > static inline int IROUND_POS(float f) > diff --git a/src/mesa/main/light.c b/src/mesa/main/light.c > index 14b4b04..2c2e3af 100644 > --- a/src/mesa/main/light.c > +++ b/src/mesa/main/light.c > @@ -850,12 +850,12 @@ _mesa_GetMaterialiv( GLenum face, GLenum pname, GLint *params ) > params[3] = FLOAT_TO_INT( mat[MAT_ATTRIB_EMISSION(f)][3] ); > break; > case GL_SHININESS: > - *params = IROUND( mat[MAT_ATTRIB_SHININESS(f)][0] ); > + *params = lroundf( mat[MAT_ATTRIB_SHININESS(f)][0] ); > break; > case GL_COLOR_INDEXES: > - params[0] = IROUND( mat[MAT_ATTRIB_INDEXES(f)][0] ); > - params[1] = IROUND( mat[MAT_ATTRIB_INDEXES(f)][1] ); > - params[2] = IROUND( mat[MAT_ATTRIB_INDEXES(f)][2] ); > + params[0] = lroundf( mat[MAT_ATTRIB_INDEXES(f)][0] ); > + params[1] = lroundf( mat[MAT_ATTRIB_INDEXES(f)][1] ); > + params[2] = lroundf( mat[MAT_ATTRIB_INDEXES(f)][2] ); > break; > default: > _mesa_error( ctx, GL_INVALID_ENUM, "glGetMaterialfv(pname)" ); > diff --git a/src/mesa/main/pixel.c b/src/mesa/main/pixel.c > index 608a545..47c7f50 100644 > --- a/src/mesa/main/pixel.c > +++ b/src/mesa/main/pixel.c > @@ -114,7 +114,7 @@ store_pixelmap(struct gl_context *ctx, GLenum map, GLsizei mapsize, > /* special case */ > ctx->PixelMaps.StoS.Size = mapsize; > for (i = 0; i < mapsize; i++) { > - ctx->PixelMaps.StoS.Map[i] = (GLfloat)IROUND(values[i]); > + ctx->PixelMaps.StoS.Map[i] = roundf(values[i]); > } > break; > case GL_PIXEL_MAP_I_TO_I: > diff --git a/src/mesa/main/pixelstore.c b/src/mesa/main/pixelstore.c > index fc81533..e691c01 100644 > --- a/src/mesa/main/pixelstore.c > +++ b/src/mesa/main/pixelstore.c > @@ -28,6 +28,7 @@ > */ > > > +#include "c99_math.h" > #include "glheader.h" > #include "bufferobj.h" > #include "context.h" > @@ -223,7 +224,7 @@ invalid_value_error: > void GLAPIENTRY > _mesa_PixelStoref( GLenum pname, GLfloat param ) > { > - _mesa_PixelStorei( pname, IROUND(param) ); > + _mesa_PixelStorei( pname, lroundf(param) ); > } > > > diff --git a/src/mesa/main/samplerobj.c b/src/mesa/main/samplerobj.c > index 32180fb1..9b16e78 100644 > --- a/src/mesa/main/samplerobj.c > +++ b/src/mesa/main/samplerobj.c > @@ -1327,19 +1327,19 @@ _mesa_GetSamplerParameteriv(GLuint sampler, GLenum pname, GLint *params) > /* GL spec 'Data Conversions' section specifies that floating-point > * value in integer Get function is rounded to nearest integer > */ > - *params = IROUND(sampObj->MinLod); > + *params = _mesa_lroundevenf(sampObj->MinLod); > break; > case GL_TEXTURE_MAX_LOD: > /* GL spec 'Data Conversions' section specifies that floating-point > * value in integer Get function is rounded to nearest integer > */ > - *params = IROUND(sampObj->MaxLod); > + *params = _mesa_lroundevenf(sampObj->MaxLod); > break; > case GL_TEXTURE_LOD_BIAS: > /* GL spec 'Data Conversions' section specifies that floating-point > * value in integer Get function is rounded to nearest integer > */ > - *params = IROUND(sampObj->LodBias); > + *params = _mesa_lroundevenf(sampObj->LodBias); > break; > case GL_TEXTURE_COMPARE_MODE: > if (!ctx->Extensions.ARB_shadow) > @@ -1355,7 +1355,7 @@ _mesa_GetSamplerParameteriv(GLuint sampler, GLenum pname, GLint *params) > /* GL spec 'Data Conversions' section specifies that floating-point > * value in integer Get function is rounded to nearest integer > */ > - *params = IROUND(sampObj->MaxAnisotropy); > + *params = _mesa_lroundevenf(sampObj->MaxAnisotropy); > break; > case GL_TEXTURE_BORDER_COLOR: > params[0] = FLOAT_TO_INT(sampObj->BorderColor.f[0]); > diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c > index 88c2c14..518d031 100644 > --- a/src/mesa/main/texparam.c > +++ b/src/mesa/main/texparam.c > @@ -1959,7 +1959,7 @@ get_tex_parameteriv(struct gl_context *ctx, > /* GL spec 'Data Conversions' section specifies that floating-point > * value in integer Get function is rounded to nearest integer > */ > - *params = IROUND(obj->Sampler.MinLod); > + *params = _mesa_lroundevenf(obj->Sampler.MinLod); > break; > case GL_TEXTURE_MAX_LOD: > if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx)) > @@ -1967,7 +1967,7 @@ get_tex_parameteriv(struct gl_context *ctx, > /* GL spec 'Data Conversions' section specifies that floating-point > * value in integer Get function is rounded to nearest integer > */ > - *params = IROUND(obj->Sampler.MaxLod); > + *params = _mesa_lroundevenf(obj->Sampler.MaxLod); > break; > case GL_TEXTURE_BASE_LEVEL: > if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx)) > @@ -1984,7 +1984,7 @@ get_tex_parameteriv(struct gl_context *ctx, > /* GL spec 'Data Conversions' section specifies that floating-point > * value in integer Get function is rounded to nearest integer > */ > - *params = IROUND(obj->Sampler.MaxAnisotropy); > + *params = _mesa_lroundevenf(obj->Sampler.MaxAnisotropy); > break; > case GL_GENERATE_MIPMAP_SGIS: > if (ctx->API != API_OPENGL_COMPAT && ctx->API != API_OPENGLES) > @@ -2022,7 +2022,7 @@ get_tex_parameteriv(struct gl_context *ctx, > /* GL spec 'Data Conversions' section specifies that floating-point > * value in integer Get function is rounded to nearest integer > */ > - *params = IROUND(obj->Sampler.LodBias); > + *params = _mesa_lroundevenf(obj->Sampler.LodBias); > break; > case GL_TEXTURE_CROP_RECT_OES: > if (ctx->API != API_OPENGLES || !ctx->Extensions.OES_draw_texture) > diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp > index 036530e..60d9d81 100644 > --- a/src/mesa/main/uniform_query.cpp > +++ b/src/mesa/main/uniform_query.cpp > @@ -407,7 +407,7 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location, > * a floating-point value is rounded to the > * nearest integer..." > */ > - dst[i].i = IROUND(src[i].f); > + dst[i].i = _mesa_lroundevenf(src[i].f); > break; > case GLSL_TYPE_BOOL: > dst[i].i = src[i].i ? 1 : 0; > diff --git a/src/mesa/swrast/s_blit.c b/src/mesa/swrast/s_blit.c > index 3e838a4..b0339f5 100644 > --- a/src/mesa/swrast/s_blit.c > +++ b/src/mesa/swrast/s_blit.c > @@ -296,7 +296,7 @@ blit_nearest(struct gl_context *ctx, > > for (dstRow = 0; dstRow < dstHeight; dstRow++) { > GLfloat srcRowF = (dstRow + 0.5F) / dstHeight * srcHeight - 0.5F; > - GLint srcRow = IROUND(srcRowF); > + GLint srcRow = lroundf(srcRowF); > GLubyte *dstRowStart = dstMap + dstRowStride * dstRow; > > assert(srcRow >= 0); > diff --git a/src/mesa/swrast/s_context.h b/src/mesa/swrast/s_context.h > index 7cf0e30..15ece26 100644 > --- a/src/mesa/swrast/s_context.h > +++ b/src/mesa/swrast/s_context.h > @@ -434,7 +434,7 @@ _swrast_unmap_renderbuffers(struct gl_context *ctx); > #define FIXED_EPSILON 1 > #define FIXED_SCALE ((float) FIXED_ONE) > #define FIXED_DBL_SCALE ((double) FIXED_ONE) > -#define FloatToFixed(X) (IROUND((X) * FIXED_SCALE)) > +#define FloatToFixed(X) (lroundf((X) * FIXED_SCALE)) > #define FixedToDouble(X) ((X) * (1.0 / FIXED_DBL_SCALE)) > #define IntToFixed(I) ((I) << FIXED_SHIFT) > #define FixedToInt(X) ((X) >> FIXED_SHIFT) > </div></div><div class="HOEnZb"><div class="h5">_______________________________________________ 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></div></div></div>