[Mesa-dev] [PATCH 2/4] mesa: Replace uses of IROUND{, D, 64} with libm functions.
Roland Scheidegger
sroland at vmware.com
Thu May 26 20:13:03 UTC 2016
Am 26.05.2016 um 20:28 schrieb Matt Turner:
> IROUND is replaces with lroundf.
> IROUNDD is replaces with lround.
> IROUND64 is replaced with llroundf.
> ---
> This is a resend of a patch from last year.
>
> Roland suggested using rint instead of round since it's cheaper, but it
> actually causes test failures in some dEQP tests. Additionally, while
> rint is cheaper than round, it's not as bad as he thought since we're
> using -fno-math-errno -fno-trapping-math.
I think I mostly thought it was expensive because there's never any
native implementation. But maybe libm in the end does something similar
than we did (which indeed shouldn't be that complex).
That said, I'm wondering why these tests fail. GL as far my search
skills go usually doesn't mention what exact rounding must be used,
albeit there's a couple places saying either "round to nearest
preferred" or "nearest integer value must be chosen" - the latter, of
course, means round-to-nearest, but still you could do just about
anything from nearest-even to nearest-away-from-zero or anything else
(albeit I guess true nearest-random wouldn't qualify due to some
invariant rule...).
There's exceptions, such as some layer calculuation in texture coord
calcs, which explicitly requires round-to-nearest-even - but then goes
ahead and says well floor(x+0.5) is also ok...).
Thus I think it would be nice if we could identify _why_ these tests
fail if we use rintf (and unless that's due to a test bug, could use
appropriate rounding function where necessary.)
(But in any case, patch is still ok by me.)
Roland
>
> Brian said it would be interesting to run the old conformance suite
> with swrast. I have not done that... I don't think I know where that
> conformance suite is.
>
> src/mesa/main/drawpix.c | 21 +++++++++++----------
> src/mesa/main/eval.c | 15 ++++++++-------
> src/mesa/main/get.c | 29 +++++++++++++++--------------
> src/mesa/main/imports.h | 25 -------------------------
> src/mesa/main/light.c | 9 +++++----
> 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 | 4 ++--
> src/mesa/swrast/s_blit.c | 3 ++-
> src/mesa/swrast/s_context.h | 3 ++-
> 12 files changed, 56 insertions(+), 74 deletions(-)
>
> diff --git a/src/mesa/main/drawpix.c b/src/mesa/main/drawpix.c
> index 720a082..1b0cb38 100644
> --- a/src/mesa/main/drawpix.c
> +++ b/src/mesa/main/drawpix.c
> @@ -36,6 +36,7 @@
> #include "dispatch.h"
> #include "glformats.h"
> #include "fbobject.h"
> +#include "util/rounding.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..d87d92e 100644
> --- a/src/mesa/main/eval.c
> +++ b/src/mesa/main/eval.c
> @@ -44,6 +44,7 @@
> #include "macros.h"
> #include "mtypes.h"
> #include "main/dispatch.h"
> +#include "util/rounding.h"
>
>
> /*
> @@ -704,7 +705,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 +729,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 e3a0a11..e8e443a 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -40,6 +40,7 @@
> #include "framebuffer.h"
> #include "samplerobj.h"
> #include "stencil.h"
> +#include "util/rounding.h"
>
> /* This is a table driven implemetation of the glGet*v() functions.
> * The basic idea is that most getters just look up an int somewhere
> @@ -1626,13 +1627,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:
> @@ -1720,13 +1721,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:
> @@ -2332,22 +2333,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 d96d666..5ed83e9 100644
> --- a/src/mesa/main/imports.h
> +++ b/src/mesa/main/imports.h
> @@ -141,31 +141,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 double to int by rounding to nearest integer, away from zero.
> - */
> -static inline int IROUNDD(double d)
> -{
> - return (int) ((d >= 0.0) ? (d + 0.5) : (d - 0.5));
> -}
> -
> -/**
> - * 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 4a8dee3..f17b67f 100644
> --- a/src/mesa/main/light.c
> +++ b/src/mesa/main/light.c
> @@ -34,6 +34,7 @@
> #include "util/simple_list.h"
> #include "mtypes.h"
> #include "math/m_matrix.h"
> +#include "util/rounding.h"
>
>
> void GLAPIENTRY
> @@ -840,12 +841,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..f74d0ee 100644
> --- a/src/mesa/main/pixelstore.c
> +++ b/src/mesa/main/pixelstore.c
> @@ -33,6 +33,7 @@
> #include "context.h"
> #include "pixelstore.h"
> #include "mtypes.h"
> +#include "util/rounding.h"
>
>
> void GLAPIENTRY
> @@ -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 14f2b45..bc6d5b7 100644
> --- a/src/mesa/main/samplerobj.c
> +++ b/src/mesa/main/samplerobj.c
> @@ -1390,19 +1390,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 = lroundf(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 = lroundf(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 = lroundf(sampObj->LodBias);
> break;
> case GL_TEXTURE_COMPARE_MODE:
> *params = sampObj->CompareMode;
> @@ -1414,7 +1414,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 = lroundf(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 ba83f8f..91bc5ab 100644
> --- a/src/mesa/main/texparam.c
> +++ b/src/mesa/main/texparam.c
> @@ -2055,7 +2055,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 = lroundf(obj->Sampler.MinLod);
> break;
> case GL_TEXTURE_MAX_LOD:
> if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx))
> @@ -2063,7 +2063,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 = lroundf(obj->Sampler.MaxLod);
> break;
> case GL_TEXTURE_BASE_LEVEL:
> if (!_mesa_is_desktop_gl(ctx) && !_mesa_is_gles3(ctx))
> @@ -2080,7 +2080,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 = lroundf(obj->Sampler.MaxAnisotropy);
> break;
> case GL_GENERATE_MIPMAP_SGIS:
> if (ctx->API != API_OPENGL_COMPAT && ctx->API != API_OPENGLES)
> @@ -2118,7 +2118,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 = lroundf(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 997b0cb..cdde3a5 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -431,13 +431,13 @@ _mesa_get_uniform(struct gl_context *ctx, GLuint program, GLint location,
> * a floating-point value is rounded to the
> * nearest integer..."
> */
> - dst[didx].i = IROUND(src[sidx].f);
> + dst[didx].i = lroundf(src[sidx].f);
> break;
> case GLSL_TYPE_BOOL:
> dst[didx].i = src[sidx].i ? 1 : 0;
> break;
> case GLSL_TYPE_DOUBLE:
> - dst[didx].i = IROUNDD(*(double *)&src[sidx].f);
> + dst[didx].i = lround(*(double *)&src[sidx].f);
> break;
> default:
> assert(!"Should not get here.");
> diff --git a/src/mesa/swrast/s_blit.c b/src/mesa/swrast/s_blit.c
> index 3e838a4..e960f4a 100644
> --- a/src/mesa/swrast/s_blit.c
> +++ b/src/mesa/swrast/s_blit.c
> @@ -31,6 +31,7 @@
> #include "main/format_pack.h"
> #include "main/condrender.h"
> #include "s_context.h"
> +#include "util/rounding.h"
>
>
> #define ABS(X) ((X) < 0 ? -(X) : (X))
> @@ -296,7 +297,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..003bb29 100644
> --- a/src/mesa/swrast/s_context.h
> +++ b/src/mesa/swrast/s_context.h
> @@ -50,6 +50,7 @@
> #include "swrast.h"
> #include "s_fragprog.h"
> #include "s_span.h"
> +#include "util/rounding.h"
>
>
> typedef void (*texture_sample_func)(struct gl_context *ctx,
> @@ -434,7 +435,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