> Looks pretty good, Dave.
>
> Reviewed-by: Brian Paul <bri...@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.
Dave.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
