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.
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.
Fixed with Mesa commit 3486f6f31b8cdb01e480cfbd8814c1e4222d26b0 Leaving bug open until piglit test is incorporated.
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?
(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
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.
(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.
> > - 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)
(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.
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
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.
Piglit test has now landed. Closing. commit 318e0b8c5cdf7aac628f85d0c4a00db8f2d6ab0d Author: Bruce Merry <bmerry@gmail.com> Date: Sat Dec 28 11:35:09 2013 +0200 ext_packed_float: Add a test that queries GL_RGBA_SIGNED_COMPONENTS_EXT V2 (Timothy Arceri): - use piglit_get_gl_enum_name() - use ARRAY_SIZE() - set config.khr_no_error_support Reviewed-by: Brian Paul <brianp@vmware.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73096
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.