[Mesa-dev] Mesa (master): draw: fix flat shading and screen-space linear interpolation in clipper

[Marek Olšák]

On Wed, Jul 11, 2012 at 1:09 PM, Jose Fonseca <jfonseca at vmware.com> wrote:
>
>
> ----- Original Message -----
>>
>>
>> ----- Original Message -----
>> > On Tue, Jul 10, 2012 at 3:30 PM, Jose Fonseca <jfonseca at vmware.com>
>> > wrote:
>> > > Yep. The interfaces are busted.
>> > >
>> > > Without native integers we get
>> > >
>> > >    DCL SV[0], INSTANCEID
>> > >    ...
>> > >    ARL ADDR[0].x, SV[0].xxxx
>> > >
>> > > and with integers we get
>> > >
>> > >    DCL SV[0], INSTANCEID
>> > >    ...
>> > >    UARL ADDR[0].x, SV[0].xxxx
>> > >
>> > > Olivier's fix is incorrect. It works on the above cases by
>> > > guessing
>> > > the type, but if we have:
>> > >
>> > >    DCL SV[0], INSTANCEID
>> > >    ...
>> > >    MOV TEMP[0].x, SV[0].xxxx
>> > >    ARL ADDR[0].x, TEMP[0].xxxx
>> > >
>> > > and
>> > >
>> > >    DCL SV[0], INSTANCEID
>> > >    ...
>> > >    MOV TEMP[0].x, SV[0].xxxx
>> > >    UARL ADDR[0].x, TEMP[0].xxxx
>> > >
>> > > it is impossible to guess -- the change is merely replacing a bug
>> > > with another.
>> > >
>> > >
>> > > For the record, the problem also happens without LLVM:
>> > >
>> > >   DRAW_USE_LLVM=0 draw-instanced -auto
>> > >
>> > >
>> > >
>> > > Anyway, AFAICT, all hardware out there that really supports
>> > > INSTANCEID/VERTEXID also supports native integers, so this is a
>> > > problem specific to the draw module.
>> > >
>> > > But given that draw module can support anything, this is actually
>> > > self inflicted! In short, INSTANCEID/VERTEXID without integers is
>> > > an historic artifact, that should not exist going forward.
>> > >
>> > > The right fix is merely making sure that PIPE_SHADER_CAP_INTEGERS
>> > > is accurately advertised as 1 by the draw module (just like
>> > > Stephane secretly did in 45fc069600ddbfe07a0a0cd5280161a8c7c55dd0
>> > > :)
>> >
>> > I just wanted to tell you Stephane's change cannot work and it even
>> > has no effect at the moment. The native integer support is global
>> > in
>> > core Mesa. It's because integer uniforms are converted to floats
>> > based
>> > on the global NativeInteger flag for all shader stages and that
>> > can't
>> > be fixed easily, because uniforms can be shared between shaders.
>> > Basically, all drivers must advertise integer support either for
>> > all
>> > shader stages or none.
>>
>> I see..
>>
>> > Given that, I only see two possible outcomes:
>> >
>> > 1) Disable INSTANCEID support in DX9-level drivers using Draw
>> > (that's
>> > only i915g AFAIK) and only support INSTANCEID with integers.
>>
>> > 2) Let drivers set the type of system values in Draw, so that Draw
>> > doesn't have to guess what it should be.
>>
>> My issue with 2) with this is that TGSI semantics depend on an
>> external state. I think this is ugly and confusing.
>>
>> If Stephane's OK with 1), I'd prefer taking it.
>
> My current plan is to:
> - make it clear that INSTANCEID/VERTEXID always means integer
> - require PIPE_SHADER_CAP_INTEGERS to be advertise in the vertex shader stage in order to advertise INSTANCEID/VERTEXID in Mesa statetracker
> - given that Mesa assumes integer, insert a I2F when loading INSTANCEID/VERTEXID (this meets the new semantics while avoiding a big re-architecture)

The first two points sound good, but why I2F? Note that softpipe fully
supports integers while llvmpipe doesn't, and I2F after loading
INSTANCEID would very likely break softpipe.

Marek

>
> I believe this is  a good compromise:
> - clear semantics
> - not hard to implement
> - no functionality regression for i915
>
> It might get me some time to get it done, but I'll get to it asap.
>
> Jose