[Mesa-dev] [PATCH v3 09/28] mesa: Add RGBA to Luminance conversion helpers

[Iago Toral]

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.

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