[Mesa-dev] [PATCH] mesa: introduce a clear color union to be used for int/unsigned buffers

Dave Airlie airlied at gmail.com
Wed Sep 14 03:24:49 PDT 2011

> Looks pretty good, Dave.
> Reviewed-by: Brian Paul <brianp at vmware.com>
> I'd be OK with committing as-is, but I'd suggest a few minor things:

Pushed with edits: comments below:

> 1. Do we really need to call the field ClearColorUnclamped?  I think we
> could keep it as ClearColor and just add a comment that values are not
> clamped.  But there are a bunch of other dual clamped/unclamped fields in
> Mesa now.  We should probably review all of those and see where we can
> consolidate the clamped/unclamped values.

Yeah we should have a look at those, I've dropped the unclamped in the
version I pushed.

> 2. There seem to be many occurances of code like this:
>  UNCLAMPED_FLOAT_TO_UBYTE(color[0], c.f[0]);
>  UNCLAMPED_FLOAT_TO_UBYTE(color[1], c.f[1]);
>  UNCLAMPED_FLOAT_TO_UBYTE(color[2], c.f[2]);
>  UNCLAMPED_FLOAT_TO_UBYTE(color[3], c.f[3]);
> I think it would be good to have a helper function like this
> static inline void
> _mesa_float_rgba_to_ubyte(GLubyte dst[4], const GLfloat src[4])
> {
> ...
> }
> in colormac.h to simply things.  That could be done in a follow-on commit.

Done as a follow-on. Some cleanup on drivers could be done.

> 3. gl_texture_object::BorderColor is also a union of float/int/uint. Maybe
> we should rename gl_clear_color to something slightly more generic and use
> it for BorderColor too.  It might be used in other places in the future too.

called it gl_color_union for want of a good name, took the border
color union out.

> 4. I don't think the st_translate_clear_color() function is needed. It's
> just a wrapper for st_translate_color().

Done as well.

Thanks for reviewing.

More information about the mesa-dev mailing list