Bug 73096 - Query GL_RGBA_SIGNED_COMPONENTS_EXT missing
Query GL_RGBA_SIGNED_COMPONENTS_EXT missing
Status: NEW
Product: Mesa
Classification: Unclassified
Component: Mesa core
10.0
All All
: medium normal
Assigned To: Brian Paul
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-12-28 09:43 UTC by Bruce Merry
Modified: 2014-01-12 13:07 UTC (History)
0 users

See Also:


Attachments
Patch to add a regression test to piglit (3.90 KB, patch)
2013-12-28 09:43 UTC, Bruce Merry
Details | Splinter Review
patch to implement GL_RGBA_SIGNED_COMPONENTS_EXT query (3.67 KB, patch)
2014-01-06 19:55 UTC, Brian Paul
Details | Splinter Review
Patch to add a regression test to piglit (7.13 KB, text/plain)
2014-01-08 14:14 UTC, Bruce Merry
Details
glxinfo output (19.14 KB, text/plain)
2014-01-08 17:07 UTC, Bruce Merry
Details
Patch to add a regression test to piglit (7.12 KB, text/plain)
2014-01-12 13:07 UTC, Bruce Merry
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Bruce Merry 2013-12-28 09:43:34 UTC
Created attachment 91251 [details] [review]
Patch to add a regression test to piglit

The EXT_packed_float extension defines a glGet* query GL_RGBA_SIGNED_COMPONENTS_EXT. However, querying it in Mesa generates GL_INVALID_ENUM, and a search of the codebase seems to indicate that it is not implemented.

I'm using Mesa 10.0.1, which is not yet a valid Version in the bug-tracker.

I'll attach a patch to piglit to add a test for this. Let me know if you'd like me to do anything about getting it pushed into the piglit code base.
Comment 1 Brian Paul 2014-01-06 19:55:41 UTC
Created attachment 91557 [details] [review]
patch to implement GL_RGBA_SIGNED_COMPONENTS_EXT query

Hi Bruce,

Thanks for catching that.  I'm attaching a patch to add the missing query code.  I'll post to mesa-dev for review too.

Regarding your piglit test: it should probably be moved to the tests/spec/ext_packed_float/ directory.  We're trying to move away from tests/bugs/  It looks good otherwise.
Comment 2 Brian Paul 2014-01-06 21:00:22 UTC
Fixed with Mesa commit 3486f6f31b8cdb01e480cfbd8814c1e4222d26b0
Leaving bug open until piglit test is incorporated.
Comment 3 Bruce Merry 2014-01-07 08:33:54 UTC
This piece of code looks wrong:

+         /* At this time, all color channels have same signedness */
+         v->value_int_4[0] =
+         v->value_int_4[1] =
+         v->value_int_4[2] =
+         v->value_int_4[3] = is_signed;

According to issue 17 of EXT_packed_float:

        If a component (such as alpha) has
        zero bits, the component should not be considered signed and so
        the bit for the respective component should be zeroed.

So for example, RG32F should return 1, 1, 0, 0.

I should probably extend the piglit test to verify the values for different formats (which would catch this error). I have roughly zero experience with piglit: any tips on how to structure such a test? Should I just iterate through some formats within a test, or have a separate test per format? Does piglit have helpers for setting up FBOs that I should be using?
Comment 4 Brian Paul 2014-01-07 16:32:38 UTC
(In reply to comment #3)
> This piece of code looks wrong:
> 
> +         /* At this time, all color channels have same signedness */
> +         v->value_int_4[0] =
> +         v->value_int_4[1] =
> +         v->value_int_4[2] =
> +         v->value_int_4[3] = is_signed;
> 
> According to issue 17 of EXT_packed_float:
> 
>         If a component (such as alpha) has
>         zero bits, the component should not be considered signed and so
>         the bit for the respective component should be zeroed.
> 
> So for example, RG32F should return 1, 1, 0, 0.

Ah, thanks, Bruce.  I missed that bit.  I'll post a follow-on patch and cc you.


> I should probably extend the piglit test to verify the values for different
> formats (which would catch this error). I have roughly zero experience with
> piglit: any tips on how to structure such a test? Should I just iterate
> through some formats within a test, or have a separate test per format? Does
> piglit have helpers for setting up FBOs that I should be using?

I'd suggest one test that creates renderbuffers (or textures) with a number of different formats and checks the GL_RGBA_SIGNED_COMPONENTS_EXT query.  I don't think piglit has any general FBO utility code.  But there's plenty to copy from in tests/fbo/*.c
Comment 5 Bruce Merry 2014-01-08 14:14:09 UTC
Created attachment 91675 [details]
Patch to add a regression test to piglit

Here is an extended version of the piglit patch. Some things I'm not quite sure about:
- Can I just assume compatibility profile, or should I have extra code in to skip things like LUMINANCE in core profile?
- When I run the test, I get an error 0x500 (INVALID_ENUM) on the RGBA8UI case. I'm not sure if that means there is another bug where RenderbufferStorage isn't accepting this format.
- Should this be a plain test or a concurrent test in tests/all.tests? I don't really understand what the requirements on a concurrent test are.
Comment 6 Brian Paul 2014-01-08 16:38:11 UTC
(In reply to comment #5)
> Created attachment 91675 [details]
> Patch to add a regression test to piglit
> 
> Here is an extended version of the piglit patch. Some things I'm not quite
> sure about:

The test looks pretty good overall.  Thanks.


> - Can I just assume compatibility profile, or should I have extra code in to
> skip things like LUMINANCE in core profile?

As-is, you should always get a compatibility profile.


> - When I run the test, I get an error 0x500 (INVALID_ENUM) on the RGBA8UI
> case. I'm not sure if that means there is another bug where
> RenderbufferStorage isn't accepting this format.

Which driver?  I'm seeing an error for the GL_LUMINANCE8_SNORM case with our llvmpipe driver.  I think we're missing some snorm cases in Mesa.  NVIDIA's driver doesn't advertise GL_EXT_texture_snorm and reports incomplete framebuffer when I try to force the lum, lum_alpha and intensity snorm formats.


> - Should this be a plain test or a concurrent test in tests/all.tests? I
> don't really understand what the requirements on a concurrent test are.

It can be concurrent.  Few tests need to be non-concurrent.
Comment 7 Bruce Merry 2014-01-08 16:56:06 UTC
> > - When I run the test, I get an error 0x500 (INVALID_ENUM) on the RGBA8UI
> > case. I'm not sure if that means there is another bug where
> > RenderbufferStorage isn't accepting this format.
> 
> Which driver?  I'm seeing an error for the GL_LUMINANCE8_SNORM case with our llvmpipe driver.  I think we're missing some snorm cases in Mesa.  NVIDIA's driver doesn't advertise GL_EXT_texture_snorm and reports incomplete framebuffer when I try to force the lum, lum_alpha and intensity snorm formats.

I built Mesa with
./configure --prefix=/home/bruce/opt/mesa --enable-xlib-glx --disable-dri --with-gallium-drivers=swrast --with-egl-platforms=x11
then set my LD_LIBRARY_PATH to lib/gallium (I have no idea whether that is the same thing as llvmpipe or not - I just did the first thing that got me a working driver).

I get this in my output:
Skipping GL_LUMINANCE8_SNORM: framebuffer not complete
Testing GL_RGBA8UI_EXT
Unexpected GL error: GL_INVALID_ENUM 0x500
(Error at /home/bruce/src/piglit/tests/spec/ext_packed_float/query-rgba-signed-components.c:76)
Comment 8 Brian Paul 2014-01-08 17:04:45 UTC
(In reply to comment #7)
> > > - When I run the test, I get an error 0x500 (INVALID_ENUM) on the RGBA8UI
> > > case. I'm not sure if that means there is another bug where
> > > RenderbufferStorage isn't accepting this format.
> > 
> > Which driver?  I'm seeing an error for the GL_LUMINANCE8_SNORM case with our llvmpipe driver.  I think we're missing some snorm cases in Mesa.  NVIDIA's driver doesn't advertise GL_EXT_texture_snorm and reports incomplete framebuffer when I try to force the lum, lum_alpha and intensity snorm formats.
> 
> I built Mesa with
> ./configure --prefix=/home/bruce/opt/mesa --enable-xlib-glx --disable-dri
> --with-gallium-drivers=swrast --with-egl-platforms=x11
> then set my LD_LIBRARY_PATH to lib/gallium (I have no idea whether that is
> the same thing as llvmpipe or not - I just did the first thing that got me a
> working driver).
> 
> I get this in my output:
> Skipping GL_LUMINANCE8_SNORM: framebuffer not complete
> Testing GL_RGBA8UI_EXT
> Unexpected GL error: GL_INVALID_ENUM 0x500
> (Error at
> /home/bruce/src/piglit/tests/spec/ext_packed_float/query-rgba-signed-
> components.c:76)

running glxinfo will tell you the driver, and I'd be interested to see the version info too.
Comment 9 Bruce Merry 2014-01-08 17:07:27 UTC
Created attachment 91694 [details]
glxinfo output

Attached full glxinfo output in case you want to see extensions. Version info is

OpenGL vendor string: VMware, Inc.
OpenGL renderer string: Gallium 0.4 on softpipe
OpenGL version string: 2.1 Mesa 10.1.0-devel (git-8d1400f)
OpenGL shading language version string: 1.30
Comment 10 Bruce Merry 2014-01-12 13:07:40 UTC
Created attachment 91898 [details]
Patch to add a regression test to piglit

Updated the test patch. The only change is to change the test from plain to concurrent. It's also rebased to apply at top-of-tree.