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.
-Brian
On Fri, Jul 31, 2015 at 7:52 PM, Roland Scheidegger <srol...@vmware.com>
wrote:
> 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)
> >
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
