texture-integer: use usampler/uint value

Submitted by Ilia Mirkin on Nov. 21, 2015, 5:24 a.m.

Details

Message ID 1448083457-22657-1-git-send-email-imirkin@alum.mit.edu
State New
Headers show
Series "texture-integer: use usampler/uint value" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Ilia Mirkin Nov. 21, 2015, 5:24 a.m.
All of the random values are positive. Mixing uint texture with isampler
is not allowed, and happens to cause problems for freedreno. Using uint
works.
---
 tests/spec/gl-3.0/texture-integer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/spec/gl-3.0/texture-integer.c b/tests/spec/gl-3.0/texture-integer.c
index 5fe90aa..389164b 100644
--- a/tests/spec/gl-3.0/texture-integer.c
+++ b/tests/spec/gl-3.0/texture-integer.c
@@ -105,10 +105,10 @@  static const struct format_info Formats[] = {
 static const char *FragShaderText =
 	"#version 130 \n"
 	"uniform vec4 bias; \n"
-	"uniform isampler2D tex; \n"
+	"uniform usampler2D tex; \n"
 	"void main() \n"
 	"{ \n"
-	"   ivec4 t = texture(tex, gl_TexCoord[0].xy); \n"
+	"   uvec4 t = texture(tex, gl_TexCoord[0].xy); \n"
 	"   gl_FragColor = vec4(t) + bias; \n"
 	"} \n";
 

Comments

On 11/20/2015 09:24 PM, Ilia Mirkin wrote:
> All of the random values are positive. Mixing uint texture with isampler
> is not allowed, and happens to cause problems for freedreno. Using uint
> works.

I guess I'm confused what the actual problem is.  The test previously
used isampler with ivec4.  Did that work with freedreno?  Is the UINT
vs. INT base type of the texture the problem?

In other news... piglit's coverage of integer samplers is *pathetic*.
I grepped for all uses of isampler or usampler.  The second grep just
filters out the compiler tests.

$ grep -r [iu]sampler tests/spec/ tests/shaders/ | egrep -v '[.](vert|frag|geom):'
tests/spec/arb_texture_buffer_range/ranges.c:	"uniform usamplerBuffer buf;\n"
tests/spec/arb_stencil_texturing/draw.c:/** A program that samples from stencil using usampler2D. */
tests/spec/arb_stencil_texturing/draw.c: * should be read using an unsigned integer usampler2D.  So, we need two
tests/spec/arb_stencil_texturing/draw.c:	"uniform usampler2D tex;\n"
tests/spec/arb_stencil_texturing/draw.c:	piglit_require_GLSL_version(130); /* for usampler2D */
tests/spec/ext_texture_integer/texture-integer-glsl130.c:	"uniform isampler2D tex; \n"
tests/spec/ext_framebuffer_multisample/fast-clear.c:	"uniform isampler2DMS tex;\n"
tests/spec/ext_framebuffer_multisample/fast-clear.c:	"uniform usampler2DMS tex;\n"
tests/spec/gl-3.0/texture-integer.c:	"uniform isampler2D tex; \n"
tests/spec/arb_texture_view/rendering-formats.c:			"uniform usampler2D s;\n"
tests/spec/arb_texture_stencil8/draw.c:/** A program that samples from stencil using usampler2D. */
tests/spec/arb_texture_stencil8/draw.c:	"uniform usampler2D tex;\n"
tests/spec/arb_sample_shading/execution/builtin-gl-sample-id.cpp:			 "isampler2DMS", ", i");
tests/spec/arb_sample_shading/execution/builtin-gl-sample-id.cpp:		asprintf(&frag_1, frag_template, "", "isampler2DRect", "");

Only 2D targets (no array, 3D, or cube targets) are tested.  Almost all
of the tests require other extensions that many drivers do not support.

There are plenty of compiler tests that verify all the sampler types
and sampler functions are available, so I guess that something.

> ---
>  tests/spec/gl-3.0/texture-integer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/spec/gl-3.0/texture-integer.c b/tests/spec/gl-3.0/texture-integer.c
> index 5fe90aa..389164b 100644
> --- a/tests/spec/gl-3.0/texture-integer.c
> +++ b/tests/spec/gl-3.0/texture-integer.c
> @@ -105,10 +105,10 @@ static const struct format_info Formats[] = {
>  static const char *FragShaderText =
>  	"#version 130 \n"
>  	"uniform vec4 bias; \n"
> -	"uniform isampler2D tex; \n"
> +	"uniform usampler2D tex; \n"
>  	"void main() \n"
>  	"{ \n"
> -	"   ivec4 t = texture(tex, gl_TexCoord[0].xy); \n"
> +	"   uvec4 t = texture(tex, gl_TexCoord[0].xy); \n"
>  	"   gl_FragColor = vec4(t) + bias; \n"
>  	"} \n";
>  
>
On Mon, Nov 23, 2015 at 4:17 PM, Ian Romanick <idr@freedesktop.org> wrote:
> On 11/20/2015 09:24 PM, Ilia Mirkin wrote:
>> All of the random values are positive. Mixing uint texture with isampler
>> is not allowed, and happens to cause problems for freedreno. Using uint
>> works.
>
> I guess I'm confused what the actual problem is.  The test previously
> used isampler with ivec4.  Did that work with freedreno?

Nope, that's why I'm changing it :)

>  Is the UINT
> vs. INT base type of the texture the problem?

Yes, it appears so. Adreno a3xx/a4xx has instructions like

sam (u32)result, coords
sam (s32)result, coords

And it *actually* converts. For example if you have an int texture
format, and do

sam (f32)result, coords

then the result will contain floatified integers. All this leads to it
being really sensitive to mismatches between sampler type in the
shader and the declared texture format. My recollection is that such
mismatches result in undefined behavior, so what the hardware is doing
is "OK". Except then the tests fail.

A proper fix would optionally use an int or uint sampler, but... meh.

>
> In other news... piglit's coverage of integer samplers is *pathetic*.
> I grepped for all uses of isampler or usampler.  The second grep just
> filters out the compiler tests.

And almost none that test rendering.

  -ilia
On Mon, Nov 23, 2015 at 4:34 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Mon, Nov 23, 2015 at 4:17 PM, Ian Romanick <idr@freedesktop.org> wrote:
>> On 11/20/2015 09:24 PM, Ilia Mirkin wrote:
>>> All of the random values are positive. Mixing uint texture with isampler
>>> is not allowed, and happens to cause problems for freedreno. Using uint
>>> works.
>>
>> I guess I'm confused what the actual problem is.  The test previously
>> used isampler with ivec4.  Did that work with freedreno?
>
> Nope, that's why I'm changing it :)
>
>>  Is the UINT
>> vs. INT base type of the texture the problem?
>
> Yes, it appears so. Adreno a3xx/a4xx has instructions like
>
> sam (u32)result, coords
> sam (s32)result, coords
>
> And it *actually* converts. For example if you have an int texture
> format, and do
>
> sam (f32)result, coords
>
> then the result will contain floatified integers. All this leads to it
> being really sensitive to mismatches between sampler type in the
> shader and the declared texture format. My recollection is that such
> mismatches result in undefined behavior, so what the hardware is doing
> is "OK". Except then the tests fail.
>
> A proper fix would optionally use an int or uint sampler, but... meh.
>
>>
>> In other news... piglit's coverage of integer samplers is *pathetic*.
>> I grepped for all uses of isampler or usampler.  The second grep just
>> filters out the compiler tests.
>
> And almost none that test rendering.

By the way, an updated version of this patch makes an identical change for

tests/spec/ext_texture_integer/texture-integer-glsl130.c:
"uniform isampler2D tex; \n"

I never sent it since I assumed that nobody would look at it.

  -ilia