[Mesa-dev] [PATCH v3 09/28] mesa: Add RGBA to Luminance conversion helpers
Iago Toral
itoral at igalia.com
Mon Dec 15 23:29:14 PST 2014
On Mon, 2014-12-15 at 10:19 -0800, Jason Ekstrand wrote:
>
>
> 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.
My plan was:
1) Add a "GLenum type" parameter so we can detect if we are in the int
case (GL_BYTE, GL_SHORT, etc) or the uint case (GL_UNSIGNED_BYTE,
GL_UNSIGNED_SHORT, etc).
2) If we are clamping uints we use _mesa_unsigned_to_unsigned to clamp.
If we are clamping ints, we use _mesa_signed_to_signed. This is because
glReadPixels converts from the render buffer format to RGBA UINT when
the dst type is uint and to RGBA INT otherwise before calling this
function.
It would look something like this (pseudocode):
void
_mesa_pack_luminance_from_rgba_integer(GLuint n, GLuint rgba[][4],
GLvoid *dstAddr, GLenum dst_format, GLenum dst_type)
{
// Compute L = R + G + B as we do now, without clamping
// Handle clamping if necessary (only if the dst_type has
// a different size
int bits = bits = _mesa_sizeof_type(dst_type) * 8;
if (bits < 32) {
int components = _mesa_components_in_format(dst_format);
int is_signed = !_mesa_is_type_unsigned(dst_type);
if (is_signed)
for (i = 0; i < n; i++) {
dst[i*components] =
_mesa_signed_to_signed(dst[i*components], bits);
} else {
for (i = 0; i < n; i++) {
dst[i*components] =
_mesa_unsigned_to_unsigned(dst[i*components], bits);
}
}
}
We could add the unsigned_to_signed and signed_to_unsigned paths too if
you prefer it that way, but they won't be used as far as I can see. In
that case we would also need to add a src_type parameter.
>
>
> 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
>
>
>
More information about the mesa-dev
mailing list