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

[Jose Fonseca]

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'm working on a patch that does this, and tries to cleanup draw_get_shader_param() too.

Jose


----- Original Message -----
> FWIW, I think Draw needs to know if a driver does or does not support
> integers, so that it can return system values in the correct type
> (not
> sure if that includes gallivm too). The reason for that is that
> glsl_to_tgsi generates *different* shaders depending on the native
> integer cap. I think this is good; the drivers which don't have
> integer system values and integer instructions just get shaders
> prepared to be executed on floating-point units only.
> 
> However the aforementioned modification of Draw would only be
> required
> if there were a driver using Draw for fallbacks, i.e. if a driver
> doesn't know in advance whether a shader will be executed in software
> or hardware (that's only svga, I guess?).
> 
> The software drivers and the drivers using Draw on hardware without
> vertex shaders can always use Draw with full integer support (and
> glsl_to_tgsi should generate vertex shaders with integers*) and
> nothing would need to be changed. Such drivers should return
> draw_get_shader_param(..) in get_shader_param, so that the state
> tracker knows exactly what the Draw module expects.
> 
> * I just realized there is a bug in glsl_to_tgsi which doesn't follow
> the integer shader cap correctly. I'll send patches.
> 
> Marek
> 
> On Tue, Jul 3, 2012 at 10:43 PM, Olivier Galibert
> <galibert at pobox.com> wrote:
> > On Tue, Jul 03, 2012 at 12:39:47PM -0700, Jose Fonseca wrote:
> >> Note that all registers are stored as floats (for convenience, and
> >> because LLVM has no unions), so integers are bitcasted into floats
> >> while storing/loading.  And I'm not sure if your patch would break
> >> that.
> >
> > I did test the patch with a llvmpipe in a glsl 120/no native
> > integer
> > setup.  draw_instanced worked.  I didn't try a full piglit though.
> >
> >
> >> I still think that having draw/gallivm guessing whether native
> >> integer support is intended or not is bad. Either:
> >> 1) TGSI is extended (e.g., more type annotations) so that
> >> native-integer support can inferred from it
> >> 2) draw/gallivm need to now if the driver has native-integer or
> >> not
> >>
> >> I'm inclined towards 1), as TGSI should be self-documented. That
> >> is,
> >> it should not be necessary to know if the driver has or not native
> >> integer support to know whether system values should be assumed to
> >> be integers or floats...
> >
> > It could be argued that dtype being TGSI_TYPE_FLOAT is the
> > documentation on what is expected.  But I'm quickly reaching the
> > point
> > where I don't really care, just tell me what you want.  As long as
> > textureFetch stays the only issue between llvmpipe and 1.30 I'm ok.
> >
> > Of course doing textureFetch right is going to require an
> > interesting
> > overhaul of the texture allocations... need to finish fixing the
> > gm45
> > interpolation/clipping first.
> >
> > Best,
> >
> >   OG.
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>