On Mon, Nov 21, 2011 at 6:52 PM, Eric Anholt <e...@anholt.net> wrote:
> On Sun, 20 Nov 2011 15:08:55 +0100, Marek Olšák <mar...@gmail.com> wrote:
>> unpack_float_z_Z24_X8 fails with -O2 in:
>> - fbo-blit-d24s8
>> - fbo-depth-sample-compare
>> - fbo-readpixels-depth-formats
>> - glean/depthStencil
>>
>> With -O0, it works fine.
>>
>> I am removing all the assertions. There's not much point in having them,
>> is there?
>
> Is -ffast-math involved at all?
Yes, -fno-fast-math makes the problem go away.
>
> At a guess, replacing "* scale" with "/ (float)0xffffff" makes the
> failure happen either way?
Yes. Only using GLdouble fixes it.
It makes sense. The mantissa without the sign has 23 bits, but 24 bits
are required to exactly represent 0xffffff if I am not mistaken.
I have found a couple more cases where it could fail in the same way
and fixed them in the attached patch. Please review.
Marek
From 255ac5e56056bd29d36466f47e0dc82e8b145264 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= <mar...@gmail.com>
Date: Sun, 11 Dec 2011 16:18:36 +0100
Subject: [PATCH] mesa: fix possible precision issues in pack/unpack/fetch functions
GLfloat doesn't have enough precision to exactly represent 0xffffff
and 0xffffffff. (and a reciprocal of those, if I am not mistaken)
If -ffast-math is enabled, using GLfloat causes assertion failures in:
- fbo-blit-d24s8
- fbo-depth-sample-compare
- fbo-readpixels-depth-formats
- glean/depthStencil
For example:
fbo-depth-sample-compare: main/format_unpack.c:1769:
unpack_float_z_Z24_X8: Assertion `dst[i] <= 1.0F' failed.
---
src/mesa/main/format_pack.c | 20 ++++++++++----------
src/mesa/main/format_unpack.c | 8 ++++----
src/mesa/swrast/s_texfetch_tmp.h | 4 ++--
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/mesa/main/format_pack.c b/src/mesa/main/format_pack.c
index ba23bab..390b494 100644
--- a/src/mesa/main/format_pack.c
+++ b/src/mesa/main/format_pack.c
@@ -2058,7 +2058,7 @@ pack_float_z_Z24_S8(const GLfloat *src, void *dst)
{
/* don't disturb the stencil values */
GLuint *d = ((GLuint *) dst);
- const GLfloat scale = (GLfloat) 0xffffff;
+ const GLdouble scale = (GLdouble) 0xffffff;
GLuint s = *d & 0xff;
GLuint z = (GLuint) (*src * scale);
assert(z <= 0xffffff);
@@ -2070,7 +2070,7 @@ pack_float_z_S8_Z24(const GLfloat *src, void *dst)
{
/* don't disturb the stencil values */
GLuint *d = ((GLuint *) dst);
- const GLfloat scale = (GLfloat) 0xffffff;
+ const GLdouble scale = (GLdouble) 0xffffff;
GLuint s = *d & 0xff000000;
GLuint z = (GLuint) (*src * scale);
assert(z <= 0xffffff);
@@ -2089,7 +2089,7 @@ static void
pack_float_z_Z32(const GLfloat *src, void *dst)
{
GLuint *d = ((GLuint *) dst);
- const GLfloat scale = (GLfloat) 0xffffffff;
+ const GLdouble scale = (GLdouble) 0xffffffff;
*d = (GLuint) (*src * scale);
}
@@ -2169,7 +2169,7 @@ static void
pack_uint_z_Z32_FLOAT(const GLuint *src, void *dst)
{
GLuint *d = ((GLuint *) dst);
- const GLfloat scale = 1.0f / (GLfloat) 0xffffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffffff;
*d = *src * scale;
assert(*d >= 0.0f);
assert(*d <= 1.0f);
@@ -2179,7 +2179,7 @@ static void
pack_uint_z_Z32_FLOAT_X24S8(const GLuint *src, void *dst)
{
GLfloat *d = ((GLfloat *) dst);
- const GLfloat scale = 1.0f / (GLfloat) 0xffffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffffff;
*d = *src * scale;
assert(*d >= 0.0f);
assert(*d <= 1.0f);
@@ -2280,7 +2280,7 @@ _mesa_pack_float_z_row(gl_format format, GLuint n,
{
/* don't disturb the stencil values */
GLuint *d = ((GLuint *) dst);
- const GLfloat scale = (GLfloat) 0xffffff;
+ const GLdouble scale = (GLdouble) 0xffffff;
GLuint i;
for (i = 0; i < n; i++) {
GLuint s = d[i] & 0xff;
@@ -2295,7 +2295,7 @@ _mesa_pack_float_z_row(gl_format format, GLuint n,
{
/* don't disturb the stencil values */
GLuint *d = ((GLuint *) dst);
- const GLfloat scale = (GLfloat) 0xffffff;
+ const GLdouble scale = (GLdouble) 0xffffff;
GLuint i;
for (i = 0; i < n; i++) {
GLuint s = d[i] & 0xff000000;
@@ -2318,7 +2318,7 @@ _mesa_pack_float_z_row(gl_format format, GLuint n,
case MESA_FORMAT_Z32:
{
GLuint *d = ((GLuint *) dst);
- const GLfloat scale = (GLfloat) 0xffffffff;
+ const GLdouble scale = (GLdouble) 0xffffffff;
GLuint i;
for (i = 0; i < n; i++) {
d[i] = (GLuint) (src[i] * scale);
@@ -2392,7 +2392,7 @@ _mesa_pack_uint_z_row(gl_format format, GLuint n,
case MESA_FORMAT_Z32_FLOAT:
{
GLuint *d = ((GLuint *) dst);
- const GLfloat scale = 1.0f / (GLfloat) 0xffffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffffff;
GLuint i;
for (i = 0; i < n; i++) {
d[i] = src[i] * scale;
@@ -2404,7 +2404,7 @@ _mesa_pack_uint_z_row(gl_format format, GLuint n,
case MESA_FORMAT_Z32_FLOAT_X24S8:
{
GLfloat *d = ((GLfloat *) dst);
- const GLfloat scale = 1.0f / (GLfloat) 0xffffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffffff;
GLuint i;
for (i = 0; i < n; i++) {
d[i * 2] = src[i] * scale;
diff --git a/src/mesa/main/format_unpack.c b/src/mesa/main/format_unpack.c
index f821c2b..4f23f3d 100644
--- a/src/mesa/main/format_unpack.c
+++ b/src/mesa/main/format_unpack.c
@@ -587,7 +587,7 @@ unpack_Z24_S8(const void *src, GLfloat dst[][4], GLuint n)
{
/* only return Z, not stencil data */
const GLuint *s = ((const GLuint *) src);
- const GLfloat scale = 1.0F / (GLfloat) 0xffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffff;
GLuint i;
for (i = 0; i < n; i++) {
dst[i][0] =
@@ -604,7 +604,7 @@ unpack_S8_Z24(const void *src, GLfloat dst[][4], GLuint n)
{
/* only return Z, not stencil data */
const GLuint *s = ((const GLuint *) src);
- const GLfloat scale = 1.0F / (GLfloat) 0xffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffff;
GLuint i;
for (i = 0; i < n; i++) {
dst[i][0] =
@@ -1761,7 +1761,7 @@ unpack_float_z_Z24_X8(GLuint n, const void *src, GLfloat *dst)
{
/* only return Z, not stencil data */
const GLuint *s = ((const GLuint *) src);
- const GLfloat scale = 1.0F / (GLfloat) 0xffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffff;
GLuint i;
for (i = 0; i < n; i++) {
dst[i] = (s[i] >> 8) * scale;
@@ -1775,7 +1775,7 @@ unpack_float_z_X8_Z24(GLuint n, const void *src, GLfloat *dst)
{
/* only return Z, not stencil data */
const GLuint *s = ((const GLuint *) src);
- const GLfloat scale = 1.0F / (GLfloat) 0xffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffff;
GLuint i;
for (i = 0; i < n; i++) {
dst[i] = (s[i] & 0x00ffffff) * scale;
diff --git a/src/mesa/swrast/s_texfetch_tmp.h b/src/mesa/swrast/s_texfetch_tmp.h
index e9512b5..877c29c 100644
--- a/src/mesa/swrast/s_texfetch_tmp.h
+++ b/src/mesa/swrast/s_texfetch_tmp.h
@@ -2270,7 +2270,7 @@ static void FETCH(f_z24_s8)( const struct swrast_texture_image *texImage,
{
/* only return Z, not stencil data */
const GLuint *src = TEXEL_ADDR(GLuint, texImage, i, j, k, 1);
- const GLfloat scale = 1.0F / (GLfloat) 0xffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffff;
texel[0] = ((*src) >> 8) * scale;
ASSERT(texImage->Base.TexFormat == MESA_FORMAT_Z24_S8 ||
texImage->Base.TexFormat == MESA_FORMAT_Z24_X8);
@@ -2298,7 +2298,7 @@ static void FETCH(f_s8_z24)( const struct swrast_texture_image *texImage,
{
/* only return Z, not stencil data */
const GLuint *src = TEXEL_ADDR(GLuint, texImage, i, j, k, 1);
- const GLfloat scale = 1.0F / (GLfloat) 0xffffff;
+ const GLdouble scale = 1.0 / (GLdouble) 0xffffff;
texel[0] = ((*src) & 0x00ffffff) * scale;
ASSERT(texImage->Base.TexFormat == MESA_FORMAT_S8_Z24 ||
texImage->Base.TexFormat == MESA_FORMAT_X8_Z24);
--
1.7.4.1
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
