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

[Jose Fonseca]

----- Original Message -----
> On Fri, Jun 29, 2012 at 03:09:23PM -0700, Stéphane Marchesin wrote:
> > I do, but it fixes a regression, so unless you have a fix, it's the
> > way to
> > go. If you have a fix I'll happily test it :)
> 
> Just between us, revert on small regressions may not be optimal long
> term on a project like mesa where the review/commit pipeline is
> clogged.  The risk of losing developers is non-negligible.  The linux
> kernel can afford it because even if you miss a cycle you know that
> you will have another one in two months, and there are a lot of
> intermediate collation trees in which your patch can be tried out and
> shaken for bugs (subsystem trees, -next, akp patch tree, etc).  I'm
> not sure Mesa can afford it.
> 
> That said, try this.
> 
> commit 56555c58d7f16c8d619c21feb23096155e2fb505
> Author: Olivier Galibert <galibert at pobox.com>
> Date:   Sat Jun 30 00:41:20 2012 +0200
> 
>     lp_bld_tgsi_soa: Fix conversion of system values to float.
>     
>     Signed-off-by: Olivier Galibert <galibert at pobox.com>
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> index 55db561..f8df2bc 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> @@ -811,9 +811,10 @@ emit_fetch_system_value(
>        break;
>     }
>  
> +   /* Extend that when atype can happen to be float */
>     if (atype != stype) {
>        if (stype == TGSI_TYPE_FLOAT) {
> -         res = LLVMBuildBitCast(builder, res,
> bld_base->base.vec_type, "");
> +         res = lp_build_int_to_float(&bld_base->base, res);
>        } else if (stype == TGSI_TYPE_UNSIGNED) {
>           res = LLVMBuildBitCast(builder, res,
>           bld_base->uint_bld.vec_type, "");
>        } else if (stype == TGSI_TYPE_SIGNED) {

Olivier,

Thanks for the updated patch.

But I think that this fix is too ad-hoc, and I suspect it may introduce other regressions.

If I understood the problem correctly, the issue here is that some drivers want system values in floats, others want in integers. Right?

If so, then let's make this explicit -- this information must be passed to the draw module somehow (e.g., a new draw function which sets a flag), which should then ensure.

Although it would be much better if TGSI semantics didn't vary with driver capabilities. Could we use different system values names, depending on FLOAT/INTEGER is desired?


Stéphane, I think your help/feedback with i915 will be necessary, as I believe very few have this hardware available now. And I suspect that as Mesa/Gallium progresses towards supporting more OpenGL 3 features, this sort of issue will become more common.


Jose