[Mesa-dev] [PATCH 1/2] i965: Fix fragment shader struct inputs.

[Matt Turner]

On Tue, Nov 24, 2015 at 10:35 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> Apparently we have literally no support for FS varying struct inputs.
> This is somewhat surprising, given that we've had tests for that very
> feature that have been passing for a long time.
>
> Normally, varying packing splits up structures for us, so we don't see
> them in the backend.  However, with SSO, varying packing isn't around
> to save us, and we get actual structs that we have to handle.
>
> This patch changes fs_visitor::emit_general_interpolation() to work
> recursively, properly handling nested structs/arrays/and so on.
> (It's easier to read with diff -b, as indentation changes.)
>
> When using the vec4 VS backend, this fixes rendering in an upcoming
> game from Feral Interactive.  (The scalar VS backend requires additional
> bug fixes in the next patch.)
>
> Cc: "11.1 11.0" <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp     | 155 ++++++++++++++++---------------
>  src/mesa/drivers/dri/i965/brw_fs.h       |   4 +-
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp |   3 +-
>  3 files changed, 82 insertions(+), 80 deletions(-)
>
> I seem to recall that passing by non-const reference is generally frowned
> upon by most folks here (including Paul), so I'm happy to convert to pointers
> if people would prefer that.  It's just a lot more asterisks.

Yes, please. It *is* a lot more asterisks, but that's kind of the
point -- it makes it explicit when you're writing something that isn't
local.

> I've also thought about renaming this to emit_general_interpolation_helper
> and making emit_general_interpolation just take a nir_variable, so that it
> hides the pass-by-reference while also simplifying the interface.

That could be better.

With the reference -> pointer change, this and 2/2 are

Reviewed-by: Matt Turner <mattst88 at gmail.com>

Nice work.