[Mesa-dev] [PATCH 00/12] RFC: Combine shader input/output enums.

Brian Paul brianp at vmware.com
Tue Mar 12 16:44:44 PDT 2013


On 03/11/2013 04:51 PM, Paul Berry wrote:
> This patch series combines the enums gl_vert_result, gl_geom_attrib,
> gl_geom_result, and gl_frag_attrib into a single enum.
>
> These four enums serve very similar purposes: they all identify data
> that flows through the pipeline from vertex shader to fragment shader,
> but their definitions don't match.  For example, the enum value
> representing CLIP_DIST0 is:
>
> - 17 in gl_vert_result,
> - 9 in gl_geom_attrib,
> - 16 in gl_geom_result, and
> - 14 in gl_frag_attrib.
>
> This isn't so much of a problem for Gallium drivers, since
> src/mesa/state_tracker/st_program.c translates these enums into
> TGSI_SEMANTIC_* values and indices, which are common to all pipeline
> stages.  But it would be a major obstacle to implementing geometry
> shaders in the i965 driver.  And IMHO it's a source of unnecessary
> confusion in Mesa core which is only going to get worse as we add more
> pipeline stages.
>
> This patch series replaces all four enums with a single enum called
> gl_varying_slot, and updates all the code that refers to those enums.
> The new enum is slightly larger than any of the enums that it
> replaces, since some of the varying slots aren't used by all pipeline
> stages.
>
> Patch 01 changes some of the bitfields that are used to refer to
> gl_frag_attrib from 32-bit to 64-bit, so that they won't run out of
> bits once the change is made.
>
> Patch 02 adds the new enum (and bitfield macros that refer to it).
>
> Patches 03-12 replace the old enums with the new enum.  To make code
> review easier, and to ease future bisections, I've tried to break this
> down into (a) patches that renumber the old enums to match the new
> enum and (b) search-and-replace patches that replace uses of the old
> enum with the new enum.  The former should be easy to code review
> (since they just affect the definition of the enums), and the latter
> should be less likely to introduce regressions (since they are simple
> search-and-replace patches).
>
> This series has been tested on i965 Gen5-7, swrast, and Gallium
> softpipe.
>
> Note: patches 04 and 12 are very large, so for purposes of mailing
> list discussion I'm going to trim them down to representative example
> hunks.  The complete series can be found in branch
> "combine_varying_slot_enums" of
> git://github.com/stereotype441/mesa.git.

Looks OK to me.  I'm cloning your repo now to do some testing...
Will get back to you in a day or so.

-Brian



More information about the mesa-dev mailing list