[Mesa-dev] [PATCH] llvmpipe: handle offset_clamp

[Jose Fonseca]

----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> This was just ignored (unless for some reason like unfilled polys draw was
> handling this).

Patch looks good.

> I'm not convinced of that code, putting the float for the clamp in the key
> isn't really a good idea. 

Indeed.

> Then again the other floats for depth bias are
> already in there too anyway (should probably have a jit_context for the
> setup function), so this is just a quick fix.
> Also, the "minimum resolvable depth difference" used isn't really right as it
> should be calculated according to the z values of the current primitive
> and not be a constant (of course, this only makes a difference for float
> depth buffers), at least for d3d10, so depth biasing is still not quite
> right.
> ---
>  src/gallium/drivers/llvmpipe/lp_state_setup.c   |   20 +++++++++++++++++++-
>  src/gallium/drivers/llvmpipe/lp_state_setup.h   |    3 ++-
>  src/gallium/drivers/llvmpipe/lp_state_surface.c |    2 ++
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_setup.c
> b/src/gallium/drivers/llvmpipe/lp_state_setup.c
> index ed68b98..2988bed 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_setup.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_setup.c
> @@ -244,6 +244,7 @@ lp_do_offset_tri(struct gallivm_state *gallivm,
>  {
>     LLVMBuilderRef b = gallivm->builder;
>     struct lp_build_context bld;
> +   struct lp_build_context flt_scalar_bld;
>     LLVMValueRef zoffset, mult;
>     LLVMValueRef z0_new, z1_new, z2_new;
>     LLVMValueRef dzdxdzdy, dzdx, dzdy, dzxyz20, dyzzx01, dyzzx01_dzxyz20,
>     dzx01_dyz20;
> @@ -298,6 +299,18 @@ lp_do_offset_tri(struct gallivm_state *gallivm,
>                             lp_build_const_float(gallivm,
>                             key->pgon_offset_units),
>                             mult, "zoffset");
>  
> +   lp_build_context_init(&flt_scalar_bld, gallivm, lp_type_float_vec(32,
> 32));
> +   if (key->pgon_offset_clamp > 0) {
> +      zoffset = lp_build_min(&flt_scalar_bld,
> +                             lp_build_const_float(gallivm,
> key->pgon_offset_clamp),
> +                             zoffset);
> +   }
> +   else if (key->pgon_offset_clamp < 0) {
> +      zoffset = lp_build_max(&flt_scalar_bld,
> +                             lp_build_const_float(gallivm,
> key->pgon_offset_clamp),
> +                             zoffset);
> +   }
> +
>     /* yuck */
>     shuffles[0] = twoi;
>     shuffles[1] = lp_build_const_int32(gallivm, 6);
> @@ -312,6 +325,10 @@ lp_do_offset_tri(struct gallivm_state *gallivm,
>     zoffset = vec4f_from_scalar(gallivm, zoffset, "");
>  
>     /* clamp and do offset */
> +   /*
> +    * XXX I suspect the clamp (is that even right to always clamp to fixed
> 0.0/1.0?)
> +    * should really be per fragment?
> +    */

yes. this doesn't look right. Should probably be a FIXME too.

>     z0z1z2 = lp_build_clamp(&bld, LLVMBuildFAdd(b, z0z1z2, zoffset, ""),
>     bld.zero, bld.one);
>  
>     /* insert into args->a0.z, a1.z, a2.z:
> @@ -810,7 +827,7 @@ lp_make_setup_variant_key(struct llvmpipe_context *lp,
>     key->pixel_center_half = lp->rasterizer->half_pixel_center;
>     key->twoside = lp->rasterizer->light_twoside;
>     key->size = Offset(struct lp_setup_variant_key,
> -		      inputs[key->num_inputs]);
> +                      inputs[key->num_inputs]);
>  
>     key->color_slot  = lp->color_slot [0];
>     key->bcolor_slot = lp->bcolor_slot[0];
> @@ -823,6 +840,7 @@ lp_make_setup_variant_key(struct llvmpipe_context *lp,
>  
>     key->pgon_offset_units = (float) (lp->rasterizer->offset_units *
>     lp->mrd);
>     key->pgon_offset_scale = lp->rasterizer->offset_scale;
> +   key->pgon_offset_clamp = lp->rasterizer->offset_clamp;
>     key->pad = 0;
>     memcpy(key->inputs, fs->inputs, key->num_inputs * sizeof key->inputs[0]);
>     for (i = 0; i < key->num_inputs; i++) {
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_setup.h
> b/src/gallium/drivers/llvmpipe/lp_state_setup.h
> index 73d40a5..c2a2c7f 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_setup.h
> +++ b/src/gallium/drivers/llvmpipe/lp_state_setup.h
> @@ -14,7 +14,7 @@ struct lp_setup_variant_list_item
>  };
>  
>  
> -struct lp_setup_variant_key {
> +struct lp_setup_variant_key {
>     unsigned size:16;
>     unsigned num_inputs:8;
>     int color_slot:8;
> @@ -29,6 +29,7 @@ struct lp_setup_variant_key {
>  
>     float pgon_offset_units;
>     float pgon_offset_scale;
> +   float pgon_offset_clamp;
>     struct lp_shader_input inputs[PIPE_MAX_SHADER_INPUTS];
>  };
>  
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_surface.c
> b/src/gallium/drivers/llvmpipe/lp_state_surface.c
> index 375ceb2..e6aac31 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_surface.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_surface.c
> @@ -65,6 +65,8 @@ llvmpipe_set_framebuffer_state(struct pipe_context *pipe,
>        }
>  
>        /* Tell draw module how deep the Z/depth buffer is */
> +      /* FIXME: mrd constant isn't right should use a value derived from
> +       * current primitive not a constant (for float depth buffers) */
>        if (lp->framebuffer.zsbuf) {
>           int depth_bits;
>           double mrd;
> --
> 1.7.9.5
> 

Jose