On 09/13/2011 04:54 AM, Dave Airlie wrote:
From: Dave Airlie<airl...@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<airl...@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 <bri...@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.

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

-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to