[Mesa-dev] [PATCH 2/2] i965: Fix flat integral varyings.

[Paul Berry]

On 25 October 2011 17:08, Kenneth Graunke <kenneth at whitecape.org> wrote:

> On 10/25/2011 04:47 PM, Paul Berry wrote:
> > Previously, the vertex and fragment shader back-ends assumed that all
> > varyings were floats.  In GLSL 1.30 this is no longer true--they can
> > also be of integral types provided that they have an interpolation
> > qualifier of "flat".
> >
> > This required two changes in each back-end: assigning the correct type
> > to the register that holds the varying value during shader execution,
> > and assigning the correct type to the register that ties the varying
> > value to the rest of the graphics pipeline (the message register in
> > the case of VS, and the payload register in the case of FS).
> >
> > Fixes piglit tests fs-int-interpolation and fs-uint-interpolation.
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs.cpp           |    4 ++--
> >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |    4 +++-
> >  2 files changed, 5 insertions(+), 3 deletions(-)
>
> Now that I see the code, I'd honestly just update brw_type_for_base_type
> to do:
>
> if (type->is_array())
>   type = type->element_type();
>
> and then change the GLSL_TYPE_ARRAY case to be an assert.
>
> brw_type_for_base_type was returning a bogus value before, and I'm
> pretty sure nothing depends on the prior behavior.  This patch
> demonstrates a use where you really want it to return something
> sensible.  I don't think the old bogus value (UD) is ever preferable to
> this new behavior.
>
> Then we don't need another odd "get related type" function, either.
>

That's what I was going to do at first too, but before I tried it I ran an
experiment to convince myself that nothing depended on the prior behavior: I
changed brw_type_for_base_type() to assert whenever it is called on an array
type, and then did a piglit run.  Result: lots of assertion failures.  Does
that experimental result change your opinion?  It makes me worried that your
proposal will break some part of the shader back-ends that I don't
understand.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111025/566953a1/attachment.htm>