[Mesa-dev] [PATCH v3 09/28] mesa: Add RGBA to Luminance conversion helpers
Jason Ekstrand
jason at jlekstrand.net
Mon Dec 15 10:19:20 PST 2014
On Mon, Dec 15, 2014 at 3:12 AM, Iago Toral <itoral at igalia.com> wrote:
>
> On Fri, 2014-12-12 at 11:36 -0800, Jason Ekstrand wrote:
> >
> >
> > On Tue, Dec 9, 2014 at 4:07 AM, Iago Toral Quiroga <itoral at igalia.com>
> > wrote:
> > For glReadPixels with a Luminance destination format we
> > compute luminance
> > values from RGBA as L=R+G+B. This, however, requires ad-hoc
> > implementation,
> > since pack/unpack functions or _mesa_swizzle_and_convert won't
> > do this
> > (and thus, neither will _mesa_format_convert). This patch adds
> > helpers
> > to do this computation so they can be used to support
> > conversion to luminance
> > formats.
> >
> > The current implementation of glReadPixels does this
> > computation as part
> > of the span functions in pack.c (see
> > _mesa_pack_rgba_span_float), that do
> > this together with other things like type conversion, etc. We
> > do not want
> > to use these functions but use _mesa_format_convert instead
> > (later patches
> > will remove the color span functions), so we need to extract
> > this functionality
> > as helpers.
> > ---
> > src/mesa/main/pack.c | 63
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > src/mesa/main/pack.h | 9 ++++++++
> > 2 files changed, 72 insertions(+)
> >
> > diff --git a/src/mesa/main/pack.c b/src/mesa/main/pack.c
> > index de6ab27..fa4046c 100644
> > --- a/src/mesa/main/pack.c
> > +++ b/src/mesa/main/pack.c
> > @@ -4334,4 +4334,67 @@ _mesa_rebase_rgba_uint(GLuint n, GLuint
> > rgba[][4], GLenum baseFormat)
> > }
> > }
> >
> > +void
> > +_mesa_pack_luminance_from_rgba_float(GLuint n, GLfloat
> > rgba[][4],
> > + GLvoid *dstAddr, GLenum
> > dst_format,
> > + GLbitfield transferOps)
> > +{
> > + int i;
> > + GLfloat *dst = (GLfloat *) dstAddr;
> > +
> > + switch (dst_format) {
> > + case GL_LUMINANCE:
> > + if (transferOps & IMAGE_CLAMP_BIT) {
> > + for (i = 0; i < n; i++) {
> > + GLfloat sum = rgba[i][RCOMP] + rgba[i][GCOMP] +
> > rgba[i][BCOMP];
> > + dst[i] = CLAMP(sum, 0.0F, 1.0F);
> > + }
> > + } else {
> > + for (i = 0; i < n; i++) {
> > + dst[i] = rgba[i][RCOMP] + rgba[i][GCOMP] +
> > rgba[i][BCOMP];
> > + }
> > + }
> > + return;
> > + case GL_LUMINANCE_ALPHA:
> > + if (transferOps & IMAGE_CLAMP_BIT) {
> > + for (i = 0; i < n; i++) {
> > + GLfloat sum = rgba[i][RCOMP] + rgba[i][GCOMP] +
> > rgba[i][BCOMP];
> > + dst[2*i] = CLAMP(sum, 0.0F, 1.0F);
> > + dst[2*i+1] = rgba[i][ACOMP];
> > + }
> > + } else {
> > + for (i = 0; i < n; i++) {
> > + dst[2*i] = rgba[i][RCOMP] + rgba[i][GCOMP] +
> > rgba[i][BCOMP];
> > + dst[2*i+1] = rgba[i][ACOMP];
> > + }
> > + }
> > + return;
> > + default:
> > + assert(!"Unsupported format");
> > + }
> > +}
> > +
> > +void
> > +_mesa_pack_luminance_from_rgba_integer(GLuint n, GLuint
> > rgba[][4],
> > + GLvoid *dstAddr,
> > GLenum dst_format)
> > +{
> > + int i;
> > + GLuint *dst = (GLuint *) dstAddr;
> > +
> > + switch (dst_format) {
> > + case GL_LUMINANCE:
> > + for (i = 0; i < n; i++) {
> > + dst[i] = rgba[i][RCOMP] + rgba[i][GCOMP] +
> > rgba[i][BCOMP];
> >
> >
> > I know this is getting old, but don't we need to do some clamping
> > here? If we do, then we have to handle signed vs. unsigned
> > seperately. Yeah, that's annoying.
>
> You are right, reviewing the original code it looks like it ends up
> clamping the result of the triple addition to the type of the dst when
> the type of the dst has a different size.
>
> In our case, we call this function from the implementation of
> glreadpixels when an integer dst luminance format is involved. In that
> case we convert to RGBA_INT RGBA_UINT first depending on the type of the
> luminance format and then we call this function, so I think we only need
> to handle clamping when we pack from RGBA_INT to short and when we pack
> from RGBA_UINT to unsigned short (meaning that we do not need to handle
> scenarios where we convert from int to ushort for example). I'll fix
> this.
>
Yeah, RGBA_[U]INT in is the only case that matters there. What were you
thinking here? Personally, I'd probably just make this function clamp and
then not special case it. That said, I'm up for other solutions too.
>
> Iago
>
> >
> > + }
> > + return;
> > + case GL_LUMINANCE_ALPHA:
> > + for (i = 0; i < n; i++) {
> > + dst[2*i] = rgba[i][RCOMP] + rgba[i][GCOMP] +
> > rgba[i][BCOMP];
> > + dst[2*i+1] = rgba[i][ACOMP];
> > + }
> > + return;
> > + default:
> > + assert(!"Unsupported format");
> > + }
> > +}
> >
> > diff --git a/src/mesa/main/pack.h b/src/mesa/main/pack.h
> > index 2173b65..2783f23 100644
> > --- a/src/mesa/main/pack.h
> > +++ b/src/mesa/main/pack.h
> > @@ -155,4 +155,13 @@ _mesa_rebase_rgba_float(GLuint n, GLfloat
> > rgba[][4], GLenum baseFormat);
> > extern void
> > _mesa_rebase_rgba_uint(GLuint n, GLuint rgba[][4], GLenum
> > baseFormat);
> >
> > +extern void
> > +_mesa_pack_luminance_from_rgba_float(GLuint n, GLfloat
> > rgba[][4],
> > + GLvoid *dstAddr, GLenum
> > dst_format,
> > + GLbitfield transferOps);
> > +
> > +extern void
> > +_mesa_pack_luminance_from_rgba_integer(GLuint n, GLuint
> > rgba[][4],
> > + GLvoid *dstAddr,
> > GLenum dst_format);
> > +
> > #endif
> > --
> > 1.9.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141215/75e4b2df/attachment.html>
More information about the mesa-dev
mailing list