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

[Brian Paul]

On 03/12/2013 05:44 PM, Brian Paul wrote:
> 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.

OK, I did a full piglit run with llvmpipe and there's no regressions.

Tested-by: Brian Paul <brianp at vmware.com>