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

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/13/2011 08:47 AM, Brian Paul wrote:
> On 09/13/2011 04:54 AM, Dave Airlie wrote:
>> From: Dave Airlie<airlied at redhat.com>
>> 
>> This introduces a new gl_clear_color union and moves the current 
>> ClearColorUnclamped to use it, it removes ClearColor completely
>> and all drivers are modified to expected unclamped floats
>> instead.
>> 
>> also fixes st to use translated color in one place it wasn't.
>> 
>> Signed-off-by: Dave Airlie<airlied at redhat.com> --- 
>> src/mesa/drivers/common/meta.c                |   10 ++-- 
>> src/mesa/drivers/dri/intel/intel_blit.c       |   10 ++-- 
>> src/mesa/drivers/dri/nouveau/nouveau_driver.c |    2 +- 
>> src/mesa/drivers/dri/nouveau/nouveau_util.h   |   10 +++ 
>> src/mesa/drivers/dri/nouveau/nv20_context.c   |    2 +- 
>> src/mesa/drivers/dri/r200/r200_state.c        |   11 ++-- 
>> src/mesa/drivers/dri/radeon/radeon_state.c    |   11 ++-- 
>> src/mesa/drivers/windows/gdi/wmesa.c          |    9 ++- 
>> src/mesa/drivers/x11/xm_dd.c                  |   37 +++++----- 
>> src/mesa/main/attrib.c                        |    8 +- 
>> src/mesa/main/blend.c                         |    3 +- 
>> src/mesa/main/clear.c                         |   94 
>> ++++++++++-------------- src/mesa/main/dd.h
>> |    3 +- src/mesa/main/get.c                           |   11
>> ++- src/mesa/main/mtypes.h                        |    9 ++- 
>> src/mesa/state_tracker/st_cb_clear.c          |   14 ++-- 
>> src/mesa/state_tracker/st_format.c            |    9 +++ 
>> src/mesa/state_tracker/st_format.h            |    3 + 
>> src/mesa/swrast/s_clear.c                     |   47
>> +++++++------ 19 files changed, 163 insertions(+), 140
>> deletions(-)
> 
> 
> 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:
> 
> 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.
> 
> 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.
> 
> 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.

I was going to suggest the same thing.

> 4. I don't think the st_translate_clear_color() function is needed.
> It's just a wrapper for st_translate_color().
> 
> -Brian
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5w2M0ACgkQX1gOwKyEAw9OwwCdEQ0tZTZ6bC5/czdOp7Dz4xP+
ZWgAoJ3xWoED6nPBoD+j1lSOZSG0WcrP
=/k4X
-----END PGP SIGNATURE-----