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

[Stéphane Marchesin]

On Tue, Jul 10, 2012 at 9:04 AM, Jose Fonseca <jfonseca at vmware.com> wrote:
>
>
> ----- 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.
>

There is also option 3): revert the two patches causing the regression.

Stéphane