Re: [Mesa-dev] [PATCH v2] mesa: Fix swizzling for luminance/intensity in _mesa_readpixels
On Mon, 2017-07-31 at 23:15 +0100, Chris Wilson wrote: > Quoting Chris Wilson (2017-07-31 22:51:25) > > Luminance/Intensity when converted to RGB should be replicated to > > fill > > the RGB channels, but they differ on how the alpha channel is > > filled, as > > luminance is set to 1 (unless alpha is supplied) and intensity is > > replicated into alpha as well. > > > > https://www.khronos.org/opengl/wiki/Image_Format: > > > > Legacy Image Formats > > > > Warning: This section describes legacy OpenGL APIs that have > > been > > removed from core OpenGL 3.1 and above (they are only > > deprecated in > > OpenGL 3.0). It is recommended that you not use this > > functionality in > > your programs. > > > > As with other deprecated functionality, it is advised that you > > not rely > > on these features. > > > > Luminance and intensity formats are color formats. They are one > > or two > > channel formats like RED or RG, but they specify particular > > behavior. > > > > When a GL_RED format is sampled in a shader, the resulting vec4 > > is (Red, > > 0, 0, 1). When a GL_INTENSITY format is sampled, the resulting > > vec4 is > > (I, I, I, I). The single intensity value is read into all four > > components. For GL_LUMINANCE, the result is (L, L, L, 1). There > > is also > > a two-channel GL_LUMINANCE_ALPHA format, which gives (L, L, L, > > A). > > > > v2: luminance -> xxx1, intensity -> , luminance_alpha -> xxxw According to the OpenGL 3.0 spec, Table 3.23 Correspondence of filtered texture components to texture source components: Luminance: LLL1 Luminance_Alpha: LLLA Intensisty: so I think the OpenGL wiki is correct and the patch is valid. I presume that with this patch there are no regressions in CTS, right? Assuming that, this patch is: Reviewed-by: Iago Toral Quiroga Also, doesn't this require changes in Piglit? I remember that we had a bunch of tests for luminance/intensity formats that I guess would need to be updated accordingly, right? Will you send a patch to update piglit? > If that quote is the expected behaviour for glReadPixels and > glGetTexSubImage, there's a similar stanza to fix in texgetimage.c > (As well as the piglit tests to update.) Yes, that would need the same fix. Will you send a patch for that too? I'll be happy to review it (both for Mesa and Piglit). > With a quick grep through VK-GL-CTS I didn't anything to refer to. > -Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa: Fix swizzling for luminance/intensity in _mesa_readpixels
Quoting Kenneth Graunke (2017-08-02 21:43:33) > I'm not so sure. Table 8.25 of the GL 4.4 compatibility spec (page 257), > below glGetTexImage, shows this: > >++ >| Base Internal Format | R | G | B | A | >++ >| ALPHA | 0 | 0 | 0 | Ai | >| LUMINANCE (or 1) | Li | 0 | 0 | 1 | >| LUMINANCE_ALPHA (or 2) | Li | 0 | 0 | Ai | >| INTENSITY | Ii | 0 | 0 | 1 | >| RED| Ri | 0 | 0 | 1 | >| RG | Ri | Gi | 0 | 1 | >| RGB (or 3) | Ri | Gi | Bi | 1 | >| RGBA (or 4)| Ri | Gi | Bi | Ai | >++ > > which indicates that the current bizarre behavior > might actually be correct for glGetTexImage (?) That would make a very nice comment above the swizzling in read_rgba_pixels(). formats.csv still has the old swizzling behaviour listed, but I don't know if that's relevant. -Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa: Fix swizzling for luminance/intensity in _mesa_readpixels
On Wednesday, August 2, 2017 3:28:23 AM PDT Nicolai Hähnle wrote: > On 01.08.2017 00:15, Chris Wilson wrote: > > Quoting Chris Wilson (2017-07-31 22:51:25) > >> Luminance/Intensity when converted to RGB should be replicated to fill > >> the RGB channels, but they differ on how the alpha channel is filled, as > >> luminance is set to 1 (unless alpha is supplied) and intensity is > >> replicated into alpha as well. > >> > >> https://www.khronos.org/opengl/wiki/Image_Format: > >> > >> Legacy Image Formats > >> > >> Warning: This section describes legacy OpenGL APIs that have been > >> removed from core OpenGL 3.1 and above (they are only deprecated in > >> OpenGL 3.0). It is recommended that you not use this functionality in > >> your programs. > >> > >> As with other deprecated functionality, it is advised that you not > >> rely > >> on these features. > >> > >> Luminance and intensity formats are color formats. They are one or two > >> channel formats like RED or RG, but they specify particular behavior. > >> > >> When a GL_RED format is sampled in a shader, the resulting vec4 is > >> (Red, > >> 0, 0, 1). When a GL_INTENSITY format is sampled, the resulting vec4 is > >> (I, I, I, I). The single intensity value is read into all four > >> components. For GL_LUMINANCE, the result is (L, L, L, 1). There is > >> also > >> a two-channel GL_LUMINANCE_ALPHA format, which gives (L, L, L, A). > >> > >> v2: luminance -> xxx1, intensity -> , luminance_alpha -> xxxw > > > > If that quote is the expected behaviour for glReadPixels and > > glGetTexSubImage, there's a similar stanza to fix in texgetimage.c > > (As well as the piglit tests to update.) > > > > With a quick grep through VK-GL-CTS I didn't anything to refer to. > > Is it even possible to have an L/I texture bound as a framebuffer? I > would expect an attempt to do that to fail the color-renderability check. I'm pretty sure you can, and people do. I'm not seeing it listed as a required color renderable format, though... > Do you have a real application that tries to do this? If not, I'd rather > we reject such attempts and maybe add a piglit test to that effect if it > doesn't exist -- and then get rid of this code in readpix.c entirely. > > I think you're right though that texgetimage.c needs to support this. I'm not so sure. Table 8.25 of the GL 4.4 compatibility spec (page 257), below glGetTexImage, shows this: ++ | Base Internal Format | R | G | B | A | ++ | ALPHA | 0 | 0 | 0 | Ai | | LUMINANCE (or 1) | Li | 0 | 0 | 1 | | LUMINANCE_ALPHA (or 2) | Li | 0 | 0 | Ai | | INTENSITY | Ii | 0 | 0 | 1 | | RED| Ri | 0 | 0 | 1 | | RG | Ri | Gi | 0 | 1 | | RGB (or 3) | Ri | Gi | Bi | 1 | | RGBA (or 4)| Ri | Gi | Bi | Ai | ++ which indicates that the current bizarre behavior might actually be correct for glGetTexImage (?) Once again, luminance and intensity formats make absolutely no sense... signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa: Fix swizzling for luminance/intensity in _mesa_readpixels
On 01.08.2017 00:15, Chris Wilson wrote: Quoting Chris Wilson (2017-07-31 22:51:25) Luminance/Intensity when converted to RGB should be replicated to fill the RGB channels, but they differ on how the alpha channel is filled, as luminance is set to 1 (unless alpha is supplied) and intensity is replicated into alpha as well. https://www.khronos.org/opengl/wiki/Image_Format: Legacy Image Formats Warning: This section describes legacy OpenGL APIs that have been removed from core OpenGL 3.1 and above (they are only deprecated in OpenGL 3.0). It is recommended that you not use this functionality in your programs. As with other deprecated functionality, it is advised that you not rely on these features. Luminance and intensity formats are color formats. They are one or two channel formats like RED or RG, but they specify particular behavior. When a GL_RED format is sampled in a shader, the resulting vec4 is (Red, 0, 0, 1). When a GL_INTENSITY format is sampled, the resulting vec4 is (I, I, I, I). The single intensity value is read into all four components. For GL_LUMINANCE, the result is (L, L, L, 1). There is also a two-channel GL_LUMINANCE_ALPHA format, which gives (L, L, L, A). v2: luminance -> xxx1, intensity -> , luminance_alpha -> xxxw If that quote is the expected behaviour for glReadPixels and glGetTexSubImage, there's a similar stanza to fix in texgetimage.c (As well as the piglit tests to update.) With a quick grep through VK-GL-CTS I didn't anything to refer to. Is it even possible to have an L/I texture bound as a framebuffer? I would expect an attempt to do that to fail the color-renderability check. Do you have a real application that tries to do this? If not, I'd rather we reject such attempts and maybe add a piglit test to that effect if it doesn't exist -- and then get rid of this code in readpix.c entirely. I think you're right though that texgetimage.c needs to support this. Cheers, Nicolai -Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev -- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] mesa: Fix swizzling for luminance/intensity in _mesa_readpixels
Quoting Chris Wilson (2017-07-31 22:51:25) > Luminance/Intensity when converted to RGB should be replicated to fill > the RGB channels, but they differ on how the alpha channel is filled, as > luminance is set to 1 (unless alpha is supplied) and intensity is > replicated into alpha as well. > > https://www.khronos.org/opengl/wiki/Image_Format: > > Legacy Image Formats > > Warning: This section describes legacy OpenGL APIs that have been > removed from core OpenGL 3.1 and above (they are only deprecated in > OpenGL 3.0). It is recommended that you not use this functionality in > your programs. > > As with other deprecated functionality, it is advised that you not rely > on these features. > > Luminance and intensity formats are color formats. They are one or two > channel formats like RED or RG, but they specify particular behavior. > > When a GL_RED format is sampled in a shader, the resulting vec4 is (Red, > 0, 0, 1). When a GL_INTENSITY format is sampled, the resulting vec4 is > (I, I, I, I). The single intensity value is read into all four > components. For GL_LUMINANCE, the result is (L, L, L, 1). There is also > a two-channel GL_LUMINANCE_ALPHA format, which gives (L, L, L, A). > > v2: luminance -> xxx1, intensity -> , luminance_alpha -> xxxw If that quote is the expected behaviour for glReadPixels and glGetTexSubImage, there's a similar stanza to fix in texgetimage.c (As well as the piglit tests to update.) With a quick grep through VK-GL-CTS I didn't anything to refer to. -Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
