[Mesa-dev] [PATCH v2] mesa: Fix swizzling for luminance/intensity in _mesa_readpixels

[Iago Toral]

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 -> xxxx, 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:      IIII

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 <itoral at igalia.com>

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