[Mesa-dev] [PATCH 01/36] Refactor code that converts between gl_vert_result and gl_frag_attrib.

[Paul Berry]

On 2 September 2011 11:59, Eric Anholt <eric at anholt.net> wrote:

> On Fri,  2 Sep 2011 09:06:40 -0700, Paul Berry <stereotype441 at gmail.com>
> wrote:
> > Previously, this conversion was duplicated in several places in the
> > i965 driver.  This patch moves it to a common location in mtypes.h,
> > near the declaration of gl_vert_result and gl_frag_attrib.
> >
> > I've also added comments to remind us that we may need to revisit the
> > conversion code when adding elements to gl_vert_result and
> > gl_frag_attrib.
> > ---
> >  src/gallium/drivers/i965/brw_wm_glsl.c      |    9 +-----
> >  src/mesa/drivers/dri/i965/brw_fs.cpp        |   16 +--------
> >  src/mesa/drivers/dri/i965/brw_vs_constval.c |    8 +---
> >  src/mesa/drivers/dri/i965/brw_wm_pass2.c    |    9 +-----
> >  src/mesa/drivers/dri/i965/gen6_sf_state.c   |   15 ++------
> >  src/mesa/main/mtypes.h                      |   45
> +++++++++++++++++++++++++-
> >  6 files changed, 53 insertions(+), 49 deletions(-)
> >
> > diff --git a/src/gallium/drivers/i965/brw_wm_glsl.c
> b/src/gallium/drivers/i965/brw_wm_glsl.c
> > index fb8e40d..9efb003 100644
> > --- a/src/gallium/drivers/i965/brw_wm_glsl.c
> > +++ b/src/gallium/drivers/i965/brw_wm_glsl.c
> > @@ -388,14 +388,7 @@ static void prealloc_reg(struct brw_wm_compile *c)
> >
> >      /* fragment shader inputs */
> >      for (i = 0; i < VERT_RESULT_MAX; i++) {
> > -       int fp_input;
> > -
> > -       if (i >= VERT_RESULT_VAR0)
> > -       fp_input = i - VERT_RESULT_VAR0 + FRAG_ATTRIB_VAR0;
> > -       else if (i <= VERT_RESULT_TEX7)
> > -       fp_input = i;
> > -       else
> > -       fp_input = -1;
> > +       int fp_input = vert_result_to_frag_attrib(i);
> >
> >         if (fp_input >= 0 && inputs & (1 << fp_input)) {
> >         urb_read_length = reg_index;
>
> Probably don't want to be touching this code.
>
> > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> > index 44ebf0a..776872c 100644
> > --- a/src/mesa/main/mtypes.h
> > +++ b/src/mesa/main/mtypes.h
> > @@ -215,7 +215,9 @@ typedef enum
> >
> >
> >  /**
> > - * Indexes for vertex program result attributes
> > + * Indexes for vertex program result attributes.  Note that
> > + * vert_result_to_frag_attrib() and frag_attrib_to_vert_result() make
> > + * assumptions about the layout of this enum.
> >   */
> >  typedef enum
> >  {
> > @@ -313,7 +315,9 @@ typedef enum
> >
> >
> >  /**
> > - * Indexes for fragment program input attributes.
> > + * Indexes for fragment program input attributes.  Note that
> > + * vert_result_to_frag_attrib() and frag_attrib_to_vert_result() make
> > + * assumptions about the layout of this enum.
> >   */
> >  typedef enum
> >  {
> > @@ -336,6 +340,43 @@ typedef enum
> >  } gl_frag_attrib;
> >
> >  /**
> > + * Convert from a gl_vert_result value to the corresponding
> gl_frag_attrib.
> > + *
> > + * VERT_RESULT_HPOS is converted to FRAG_ATTRIB_WPOS.
> > + *
> > + * gl_vert_result values which have no corresponding gl_frag_attrib
> > + * (VERT_RESULT_PSIZ, VERT_RESULT_BFC0, VERT_RESULT_BFC1, and
> > + * VERT_RESULT_EDGE) are converted to a value of -1.
> > + */
> > +static inline int vert_result_to_frag_attrib(int vert_result)
> > +{
> > +   if (vert_result >= VERT_RESULT_VAR0)
> > +      return vert_result - VERT_RESULT_VAR0 + FRAG_ATTRIB_VAR0;
> > +   else if (vert_result <= VERT_RESULT_TEX7)
> > +      return vert_result;
> > +   else
> > +      return -1;
> > +}
> > +
> > +/**
> > + * Convert from a gl_frag_attrib value to the corresponding
> gl_vert_result.
> > + *
> > + * FRAG_ATTRIB_WPOS is converted to VERT_RESULT_HPOS.
> > + *
> > + * gl_frag_attrib values which have no corresponding gl_vert_result
> > + * (FRAG_ATTRIB_FACE and FRAG_ATTRIB_PNTC) are converted to a value of
> -1.
> > + */
> > +static inline int frag_attrib_to_vert_result(int frag_attrib)
> > +{
> > +   if (frag_attrib <= FRAG_ATTRIB_TEX7)
> > +      return frag_attrib;
> > +   else if (frag_attrib >= FRAG_ATTRIB_VAR0)
> > +      return frag_attrib - FRAG_ATTRIB_VAR0 + VERT_RESULT_VAR0;
> > +   else
> > +      return -1;
> > +}
> > +
> > +/**
> >   * Bitflags for fragment program input attributes.
> >   */
> >  /*@{*/
>
> This looks to be the first inline functions in mtypes.h.  I suspect the
> code should go somewhere else, but I don't know where.
>

My reasoning for putting these functions in mtypes.h was that I wanted to
put them as near as possible to the enums they were related to, especially
since they will likely need updating when we modify those enums.  I'm open
to other suggestions about where to put them if anyone cares strongly.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110902/ba0450f0/attachment.htm>