[Mesa-dev] [PATCH 7/7] mesa: Replace uses of IROUND{, 64} with libm functions.
Roland Scheidegger
sroland at vmware.com
Fri Jul 31 18:52:01 PDT 2015
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
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)
>
More information about the mesa-dev
mailing list