[Mesa-dev] [PATCH 08/10] softpipe: add ARB_texture_cube_map_array support

[Brian Paul]

On 11/06/2012 03:16 PM, Dave Airlie wrote:
> This adds support to the softpipe texture sampler and tgsi exec.
>
> In order to handle the extra input to the texture sampling,
> I've had to expand the interfaces to take a c1 value for storing
> the texture compare value for the TEX2 case.
>
> Signed-off-by: Dave Airlie<airlied at redhat.com>

Your changes look good, but I think it would be a good time to add 
some comments too...


> ---
>   src/gallium/auxiliary/tgsi/tgsi_exec.c       | 112 ++++++++++++-----
>   src/gallium/auxiliary/tgsi/tgsi_exec.h       |   1 +
>   src/gallium/drivers/softpipe/sp_screen.c     |   4 +-
>   src/gallium/drivers/softpipe/sp_tex_sample.c | 172 +++++++++++++++++++++++++--
>   src/gallium/drivers/softpipe/sp_tex_sample.h |   5 +-
>   src/gallium/drivers/softpipe/sp_texture.c    |   4 +
>   6 files changed, 255 insertions(+), 43 deletions(-)
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> index 0b58f09..3ccf94e 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> @@ -1709,6 +1709,7 @@ fetch_texel( struct tgsi_sampler *sampler,
>                const union tgsi_exec_channel *t,
>                const union tgsi_exec_channel *p,
>                const union tgsi_exec_channel *c0,
> +             const union tgsi_exec_channel *c1,
>                enum tgsi_sampler_control control,
>                union tgsi_exec_channel *r,
>                union tgsi_exec_channel *g,

We should probably add some comments to this function to explain what 
c0 and c1 are.  Looking at it now, I don't know.


> @@ -1718,7 +1719,7 @@ fetch_texel( struct tgsi_sampler *sampler,
>      uint j;
>      float rgba[TGSI_NUM_CHANNELS][TGSI_QUAD_SIZE];
>
> -   sampler->get_samples(sampler, s->f, t->f, p->f, c0->f, control, rgba);
> +   sampler->get_samples(sampler, s->f, t->f, p->f, c0->f, c1->f, control, rgba);
>
>      for (j = 0; j<  4; j++) {
>         r->f[j] = rgba[0][j];
> @@ -1738,15 +1739,15 @@ fetch_texel( struct tgsi_sampler *sampler,
>   static void
>   exec_tex(struct tgsi_exec_machine *mach,
>            const struct tgsi_full_instruction *inst,
> -         uint modifier)
> +         uint modifier, uint sampler)

Similarly, a comment for exec_tex() explaining the modifier and 
sampler parameters would be good.

The rest looks good.

For patches 7,8,9:
Reviewed-by: Brian Paul <brianp at vmware.com>