[Mesa-dev] [PATCH 2/2] llvmpipe: fix txq for 1d/2d arrays.

[Roland Scheidegger]

Am 08.12.2012 07:02, schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
> 
> Noticed would fail, we were doing two things wrong
> 
> a) 1d arrays require the layers in height
> b) minifying the layers field.
> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c | 22 +++++++++++++++++-----
>  src/gallium/drivers/llvmpipe/lp_setup.c           |  6 +++++-
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> index ba265b2..c0389a8 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> @@ -1680,23 +1680,30 @@ lp_build_size_query_soa(struct gallivm_state *gallivm,
>  {
>     LLVMValueRef lod;
>     LLVMValueRef size;
> -   int dims, i;
> +   int dims, i, num_min;
>     struct lp_build_context bld_int_vec;
>  
>     switch (static_state->target) {
>     case PIPE_TEXTURE_1D:
>     case PIPE_BUFFER:
> -      dims = 1;
> +      num_min = dims = 1;
> +      break;
> +   case PIPE_TEXTURE_1D_ARRAY:
> +      num_min = 1;
> +      dims = 2;
>        break;
>     case PIPE_TEXTURE_2D:
>     case PIPE_TEXTURE_CUBE:
>     case PIPE_TEXTURE_RECT:
> -      dims = 2;
> +      num_min = dims = 2;
>        break;
>     case PIPE_TEXTURE_3D:
> +      num_min = dims = 3;
> +      break;
> +   case PIPE_TEXTURE_2D_ARRAY:
> +      num_min = 2;
>        dims = 3;
>        break;
> -
>     default:
>        assert(0);
>        return;
> @@ -1723,19 +1730,24 @@ lp_build_size_query_soa(struct gallivm_state *gallivm,
>                                   dynamic_state->width(dynamic_state, gallivm, unit),
>                                   lp_build_const_int32(gallivm, 0), "");
>  
> +   if (num_min == 1)
> +      size = lp_build_minify(&bld_int_vec, size, lod);      
>     if (dims >= 2) {
>        size = LLVMBuildInsertElement(gallivm->builder, size,
>                                      dynamic_state->height(dynamic_state, gallivm, unit),
>                                      lp_build_const_int32(gallivm, 1), "");
>     }
>  
> +   if (num_min == 2)
> +      size = lp_build_minify(&bld_int_vec, size, lod);      
>     if (dims >= 3) {
>        size = LLVMBuildInsertElement(gallivm->builder, size,
>                                      dynamic_state->depth(dynamic_state, gallivm, unit),
>                                      lp_build_const_int32(gallivm, 2), "");
>     }
>  
> -   size = lp_build_minify(&bld_int_vec, size, lod);
> +   if (num_min == 3)
> +      size = lp_build_minify(&bld_int_vec, size, lod);
>  
>     for (i=0; i < dims; i++) {
>        sizes_out[i] = lp_build_extract_broadcast(gallivm, bld_int_vec.type, int_type,
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
> index 7d40d8c..3dfe335 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
> @@ -678,7 +678,11 @@ lp_setup_set_fragment_sampler_views(struct lp_setup_context *setup,
>           struct lp_jit_texture *jit_tex;
>           jit_tex = &setup->fs.current.jit_context.textures[i];
>           jit_tex->width = tex->width0;
> -         jit_tex->height = tex->height0;
> +         if (tex->target == PIPE_TEXTURE_1D_ARRAY) {
> +            jit_tex->height = tex->array_size;
> +         } else {
> +            jit_tex->height = tex->height0;
> +         }
>           jit_tex->first_level = view->u.tex.first_level;
>           jit_tex->last_level = tex->last_level;
>  
> 
I'm not convinced this is the right thing to do. Nothing requires us to
put the array_size into jit_tex->height, I'd prefer it if we'd always
put it into depth (which is what the rest of the sampling code assumes
where it is). There's actually a very good reason for not putting
array_size into either width or depth depending on 1d or 2d array, since
eventually we'll need some min/max layer instead anyway, having
array_size always put into the same variable makes things simpler.
Also, I'm not entirely happy with the different semantic meaning of
"dims" here. While this was always used somewhat confusingly and didn't
really mean "dims" it was at least used consistently in the sampling
code to mean "the number of "ordinary" coords subject to minification,
wrapping etc" (and I've just recently added a comment somewhere to
indicate this finally).
But otherwise this looks good, certainly that query code couldn't handle
texture arrays (I was unable to get the piglit test to run when I tried
even with version overrides).

Roland