shaders: Make glsl-fs-lots-of-tex less susceptable to rounding error.

Submitted by Jason Ekstrand on July 11, 2014, 8:01 p.m.

Details

Message ID 1405108889-17110-1-git-send-email-jason.ekstrand@intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Jason Ekstrand July 11, 2014, 8:01 p.m.
The only reason this test passed before is because we were rounding wrong
in texture upload.  Previously, the shader started with 0.01, effectively
multiplied it by 34, and then compared it to 0.4.  The problem is that,
when converting from float to unsigned byte, you get 0.01 * 255 = 2.55.
Previously, we were rounding this up to 3.  Then 3 * 34 = 102 and
102 / 255 = 0.4.  However, the OpenGL spec specifies that you should simply
cast the floating point value to the integer value and this rounds down to
2.  The result is that you get 2 * 34 = 68 and 68 / 255 = 2.667 which is
nowhere close to 0.4.

The new version simply starts with 0.4 and then divides by 4 at the end of
the shader to get back to 0.4.

Signed-off-by: Jason Ekstrand <jason.ekstrand@intel.com>
---
 tests/shaders/glsl-fs-lots-of-tex.shader_test | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/shaders/glsl-fs-lots-of-tex.shader_test b/tests/shaders/glsl-fs-lots-of-tex.shader_test
index a31778f..9056774 100644
--- a/tests/shaders/glsl-fs-lots-of-tex.shader_test
+++ b/tests/shaders/glsl-fs-lots-of-tex.shader_test
@@ -34,12 +34,13 @@  void main()
 	vec4 p = texture2D(tex, cst.xy) + texture2D(tex, cst.xy);
 	vec4 q = texture2D(tex, cst.xy) + texture2D(tex, cst.xy);
 
-	gl_FragColor = a + b + c + d + e + f + g + h + i + j + k + l + m + n + o + p + q;
+	vec4 sum = a + b + c + d + e + f + g + h + i + j + k + l + m + n + o + p + q;
+	gl_FragColor = sum / 34.0f;
 }
 
 [test]
 uniform int tex 0
 uniform vec4 cst 0.0 0.0 0.0 0.0
-texture checkerboard 0 0 (8, 8) (0.01, 0.0, 0.0, 0.0) (0.01, 0.0, 0.0, 0.0)
+texture checkerboard 0 0 (8, 8) (0.4, 0.0, 0.0, 0.0) (0.4, 0.0, 0.0, 0.0)
 draw rect -1 -1 2 2
 relative probe rgb (0.75, 0.75) (0.4, 0.0, 0.0)

Comments

Kinda like http://patchwork.freedesktop.org/patch/17718/ ? :) The
feedback I got (although not apparently in e-mail) was that it was
shady. OTOH, the test doesn't do what it claims to due to the CSE that
now happens, so perhaps it should just get deleted.

On Fri, Jul 11, 2014 at 4:01 PM, Jason Ekstrand <jason@jlekstrand.net> wrote:
> The only reason this test passed before is because we were rounding wrong
> in texture upload.  Previously, the shader started with 0.01, effectively
> multiplied it by 34, and then compared it to 0.4.  The problem is that,
> when converting from float to unsigned byte, you get 0.01 * 255 = 2.55.
> Previously, we were rounding this up to 3.  Then 3 * 34 = 102 and
> 102 / 255 = 0.4.  However, the OpenGL spec specifies that you should simply
> cast the floating point value to the integer value and this rounds down to
> 2.  The result is that you get 2 * 34 = 68 and 68 / 255 = 2.667 which is
> nowhere close to 0.4.
>
> The new version simply starts with 0.4 and then divides by 4 at the end of
> the shader to get back to 0.4.
>
> Signed-off-by: Jason Ekstrand <jason.ekstrand@intel.com>
> ---
>  tests/shaders/glsl-fs-lots-of-tex.shader_test | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tests/shaders/glsl-fs-lots-of-tex.shader_test b/tests/shaders/glsl-fs-lots-of-tex.shader_test
> index a31778f..9056774 100644
> --- a/tests/shaders/glsl-fs-lots-of-tex.shader_test
> +++ b/tests/shaders/glsl-fs-lots-of-tex.shader_test
> @@ -34,12 +34,13 @@ void main()
>         vec4 p = texture2D(tex, cst.xy) + texture2D(tex, cst.xy);
>         vec4 q = texture2D(tex, cst.xy) + texture2D(tex, cst.xy);
>
> -       gl_FragColor = a + b + c + d + e + f + g + h + i + j + k + l + m + n + o + p + q;
> +       vec4 sum = a + b + c + d + e + f + g + h + i + j + k + l + m + n + o + p + q;
> +       gl_FragColor = sum / 34.0f;
>  }
>
>  [test]
>  uniform int tex 0
>  uniform vec4 cst 0.0 0.0 0.0 0.0
> -texture checkerboard 0 0 (8, 8) (0.01, 0.0, 0.0, 0.0) (0.01, 0.0, 0.0, 0.0)
> +texture checkerboard 0 0 (8, 8) (0.4, 0.0, 0.0, 0.0) (0.4, 0.0, 0.0, 0.0)
>  draw rect -1 -1 2 2
>  relative probe rgb (0.75, 0.75) (0.4, 0.0, 0.0)
> --
> 2.0.0
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
n Fri, Jul 11, 2014 at 1:06 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:

> Kinda like http://patchwork.freedesktop.org/patch/17718/ ? :)


Yeah, kinda like that :-).  Only my patch solves it a little more for-sure


> The
> feedback I got (although not apparently in e-mail) was that it was
> shady. OTOH, the test doesn't do what it claims to due to the CSE that
> now happens, so perhaps it should just get deleted.
>

If it's not doing anything, then let's get rid of it.  In any case, I won't
care about it failing.
--Jason


>
> On Fri, Jul 11, 2014 at 4:01 PM, Jason Ekstrand <jason@jlekstrand.net>
> wrote:
> > The only reason this test passed before is because we were rounding wrong
> > in texture upload.  Previously, the shader started with 0.01, effectively
> > multiplied it by 34, and then compared it to 0.4.  The problem is that,
> > when converting from float to unsigned byte, you get 0.01 * 255 = 2.55.
> > Previously, we were rounding this up to 3.  Then 3 * 34 = 102 and
> > 102 / 255 = 0.4.  However, the OpenGL spec specifies that you should
> simply
> > cast the floating point value to the integer value and this rounds down
> to
> > 2.  The result is that you get 2 * 34 = 68 and 68 / 255 = 2.667 which is
> > nowhere close to 0.4.
> >
> > The new version simply starts with 0.4 and then divides by 4 at the end
> of
> > the shader to get back to 0.4.
> >
> > Signed-off-by: Jason Ekstrand <jason.ekstrand@intel.com>
> > ---
> >  tests/shaders/glsl-fs-lots-of-tex.shader_test | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/shaders/glsl-fs-lots-of-tex.shader_test
> b/tests/shaders/glsl-fs-lots-of-tex.shader_test
> > index a31778f..9056774 100644
> > --- a/tests/shaders/glsl-fs-lots-of-tex.shader_test
> > +++ b/tests/shaders/glsl-fs-lots-of-tex.shader_test
> > @@ -34,12 +34,13 @@ void main()
> >         vec4 p = texture2D(tex, cst.xy) + texture2D(tex, cst.xy);
> >         vec4 q = texture2D(tex, cst.xy) + texture2D(tex, cst.xy);
> >
> > -       gl_FragColor = a + b + c + d + e + f + g + h + i + j + k + l + m
> + n + o + p + q;
> > +       vec4 sum = a + b + c + d + e + f + g + h + i + j + k + l + m + n
> + o + p + q;
> > +       gl_FragColor = sum / 34.0f;
> >  }
> >
> >  [test]
> >  uniform int tex 0
> >  uniform vec4 cst 0.0 0.0 0.0 0.0
> > -texture checkerboard 0 0 (8, 8) (0.01, 0.0, 0.0, 0.0) (0.01, 0.0, 0.0,
> 0.0)
> > +texture checkerboard 0 0 (8, 8) (0.4, 0.0, 0.0, 0.0) (0.4, 0.0, 0.0,
> 0.0)
> >  draw rect -1 -1 2 2
> >  relative probe rgb (0.75, 0.75) (0.4, 0.0, 0.0)
> > --
> > 2.0.0
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
>