[Mesa-dev] Mesa (master): r300g: fix gl_PointCoord

[Brian Paul]

On 08/25/2010 09:53 AM, Luca Barbieri wrote:
>> There really needs to be a new TGSI semantic label for this.  The draw
>> module needs some way to determine which fragment shader input expects PNTC.
>>   Care to write a patch?
> Actually, I'd propose using a GENERIC slot which is currently
> unassigned, and using sprite_coord_enable to activate coord
> replacement on it.
> This seems simpler, since we need coordinate replacement anyway.

Ah, right.  I was thinking that PNTC worked for all point types, but 
it's only for sprite points, which is where sprite_coord_enable applies.

I think the only complication is in the state tracker where we'll have 
to examine the fragment shader during raster state validation to 
determine if/where we need to set sprite_coord_enable bits.


> The really theoretically nice solution would be adding a new semantic,
> and having a new linkage table that could be set to route writes to
> that new semantics to reads from any GENERIC (for coord replacement).
> However this is a massive change, and not necessarily good (i.e. it
> might not match hardware well).
>
>> I'm having trouble understanding what you mean there.
> Suppose a vertex shader writes GENERIC[1],  GENERIC[2] and
> GENERIC[60], and sprite_coord_enable is set to (1<<  1) | (1<<  2).
>
> Draw will put sprite coordinates in GENERIC[2] and GENERIC[60],
> because they are the 2nd and 3rd input written by the vertex shader.
> r300g and nvfx will put sprite coordinates in GENERIC[1] and
> GENERIC[2], because they are GENERICs number 1 and 2.
> I'm not what mesa/st assumes (but clearly, it must be incompatible
> with either draw or r300g).
>
> Note that draw's interpretation has the flaw that it depends on the
> vertex shader in a weird way, while r300g/nvfx's interpretation
> prevents doing sprite coord replacement on GENERIC[32] and later
> GENERICs.

I was just looking at the gallium docs for sprite_coord_enable and 
it's not clearly defined (no big surprise).

It seems to me that if bit 'k' is set in sprite_coord_enable then 
GENERIC[k] should get the auto-generated texcoord.  That's how it's 
interpreted in the draw module but not in the state tracker (we're 
just getting lucky there because texcoord[i] is usually generic[i]).

BTW, I think it's helpful to think of sprite coord gen as something 
that's done for fragment shader inputs, not vertex shader outputs.  We 
could implement sprite coord gen in a geometry shader (which is kind 
of what the draw module's wide-point stage is emulating).


> There a lot of other subtle issues like this resulting from the
> underspecification of how linkage is supposed to work.
>
> IMHO the only way to fix it is to have someone with the authority to
> decide take time to look at all API and hardware and formalize exactly
> things like what is the range of allowed indices for the various
> semantics, how sprite_coord_enable is to be interpreted and so on.

Does my definition of sprite_coord_enable above sound good?


> However, there is also the related issue that TGSI doesn't match
> modern GPUs and GLSL, and switching to a new IR might heavily
> influence the decision above (e.g. if we switch to LLVM bitcode for
> the shader interface, then it's more natural to say that the driver
> should link varyings by name dynamically, probably with the help of
> common code, than saying that for TGSI).
> .

Yeah, one step at a time.

-Brian