[Mesa-dev] [PATCH] draw: handle nan clipdistance
Roland Scheidegger
sroland at vmware.com
Thu Aug 15 11:01:10 PDT 2013
Am 15.08.2013 19:12, schrieb Zack Rusin:
> If clipdistance for one of the vertices is nan (or inf) then the
> entire primitive should be discarded.
>
> Signed-off-by: Zack Rusin <zackr at vmware.com>
> ---
> src/gallium/auxiliary/draw/draw_cliptest_tmp.h | 2 +-
> src/gallium/auxiliary/draw/draw_llvm.c | 3 ++
> src/gallium/auxiliary/draw/draw_pipe_clip.c | 13 +++++-
> src/gallium/auxiliary/gallivm/lp_bld_arit.c | 53 ++++++++++++++++++++++++
> src/gallium/auxiliary/gallivm/lp_bld_arit.h | 11 +++++
> 5 files changed, 79 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> index e4500db..fc54810 100644
> --- a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> +++ b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> @@ -140,7 +140,7 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
> clipdist = out->data[cd[0]][i];
> else
> clipdist = out->data[cd[1]][i-4];
> - if (clipdist < 0)
> + if (clipdist < 0 || util_is_inf_or_nan(clipdist))
> mask |= 1 << plane_idx;
> } else {
> if (dot4(clipvertex, plane[plane_idx]) < 0)
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c
> index 84e3392..1e9eadb 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> @@ -1261,6 +1261,7 @@ generate_clipmask(struct draw_llvm *llvm,
> if (clip_user) {
> LLVMValueRef planes_ptr = draw_jit_context_planes(gallivm, context_ptr);
> LLVMValueRef indices[3];
> + LLVMValueRef is_nan;
>
> /* userclip planes */
> while (ucp_enable) {
> @@ -1280,6 +1281,8 @@ generate_clipmask(struct draw_llvm *llvm,
> clipdist = LLVMBuildLoad(builder, outputs[cd[1]][i-4], "");
> }
> test = lp_build_compare(gallivm, f32_type, PIPE_FUNC_GREATER, zero, clipdist);
> + is_nan = lp_build_is_inf_or_nan(gallivm, vs_type, clipdist);
I think this should either use is_inf_or_nan (if it only cares about
nan, though I guess it really does care about inf too) or else the var
should be named is_inf_or_nan.
> + test = LLVMBuildOr(builder, test, is_nan, "");
> temp = lp_build_const_int_vec(gallivm, i32_type, 1 << plane_idx);
> test = LLVMBuildAnd(builder, test, temp, "");
> mask = LLVMBuildOr(builder, mask, test, "");
> diff --git a/src/gallium/auxiliary/draw/draw_pipe_clip.c b/src/gallium/auxiliary/draw/draw_pipe_clip.c
> index b76e9a5..2f2aadb 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe_clip.c
> +++ b/src/gallium/auxiliary/draw/draw_pipe_clip.c
> @@ -104,7 +104,7 @@ static void interp_attr( float dst[4],
> float t,
> const float in[4],
> const float out[4] )
> -{
> +{
> dst[0] = LINTERP( t, out[0], in[0] );
> dst[1] = LINTERP( t, out[1], in[1] );
> dst[2] = LINTERP( t, out[2], in[2] );
> @@ -380,6 +380,9 @@ do_clip_tri( struct draw_stage *stage,
> dp_prev = getclipdist(clipper, vert_prev, plane_idx);
> clipmask &= ~(1<<plane_idx);
>
> + if (util_is_inf_or_nan(dp_prev))
> + return; //discard nan
> +
> assert(n < MAX_CLIPPED_VERTICES);
> if (n >= MAX_CLIPPED_VERTICES)
> return;
> @@ -392,6 +395,9 @@ do_clip_tri( struct draw_stage *stage,
>
> float dp = getclipdist(clipper, vert, plane_idx);
>
> + if (util_is_inf_or_nan(dp))
> + return; //discard nan
> +
> if (!IS_NEGATIVE(dp_prev)) {
> assert(outcount < MAX_CLIPPED_VERTICES);
> if (outcount >= MAX_CLIPPED_VERTICES)
> @@ -522,6 +528,9 @@ do_clip_line( struct draw_stage *stage,
> const float dp0 = getclipdist(clipper, v0, plane_idx);
> const float dp1 = getclipdist(clipper, v1, plane_idx);
>
> + if (util_is_inf_or_nan(dp0) || util_is_inf_or_nan(dp1))
> + return; //discard nan
> +
> if (dp1 < 0.0F) {
> float t = dp1 / (dp1 - dp0);
> t1 = MAX2(t1, t);
> @@ -594,7 +603,7 @@ clip_tri( struct draw_stage *stage,
> unsigned clipmask = (header->v[0]->clipmask |
> header->v[1]->clipmask |
> header->v[2]->clipmask);
> -
> +
> if (clipmask == 0) {
> /* no clipping needed */
> stage->next->tri( stage->next, header );
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> index 98409c3..72b563e 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> @@ -3671,3 +3671,56 @@ lp_build_isfinite(struct lp_build_context *bld,
> return lp_build_compare(bld->gallivm, int_type, PIPE_FUNC_NOTEQUAL,
> intx, infornan32);
> }
> +
> +/*
> + * Returns true if the number is nan or inf or false otherwise.
> + * The input has to be a floating point vector.
> + */
> +LLVMValueRef
> +lp_build_is_inf_or_nan(struct gallivm_state *gallivm,
> + const struct lp_type type,
> + LLVMValueRef x)
> +{
> + LLVMBuilderRef builder = gallivm->builder;
> + struct lp_type int_type = lp_int_type(type);
> + LLVMValueRef const0 = lp_build_const_int_vec(gallivm, int_type,
> + 0x7f800000);
> + LLVMValueRef ret;
> +
> + assert(type.floating);
> +
> + ret = LLVMBuildBitCast(builder, x, lp_build_vec_type(gallivm, int_type), "");
> + ret = LLVMBuildAnd(builder, ret, const0, "");
> + ret = lp_build_compare(gallivm, int_type, PIPE_FUNC_EQUAL,
> + ret, const0);
> +
> + return ret;
> +}
> +
> +
> +/*
> + * Returns true if the number is nan and false otherwise.
> + * The input has to be a floating point vector.
> + */
> +LLVMValueRef
> +lp_build_is_nan(struct gallivm_state *gallivm,
> + const struct lp_type type,
> + LLVMValueRef x)
> +{
> + LLVMBuilderRef builder = gallivm->builder;
> + struct lp_type int_type = lp_int_type(type);
> + LLVMValueRef const0 = lp_build_const_int_vec(gallivm, int_type,
> + 0x7fffffff);
> + LLVMValueRef const1 = lp_build_const_int_vec(gallivm, int_type,
> + 0x7f800000);
> + LLVMValueRef ret;
> +
> + assert(type.floating);
> +
> + ret = LLVMBuildBitCast(builder, x, lp_build_vec_type(gallivm, int_type), "");
> + ret = LLVMBuildAnd(builder, ret, const0, "");
> + ret = lp_build_compare(gallivm, int_type, PIPE_FUNC_GREATER,
> + ret, const1);
> +
> + return ret;
> +}
I realize this function isn't used but it looks unnecessarily
complicated - two constants one AND plus one comparison when you could
simply do a single comparison (compare x with x with unordered not
equal). This is actually doubly bad with AVX because the int comparison
is going to use 4 instructions instead of 1 (extract/2 cmp/1 insert),
well if this runs 8-wide at least.
(The lp_build_is_inf_or_nan() OTOH can't really do this because it needs
to care about both negative and positive infinities, though actually the
only caller so far only seems to care about positive infinities, so a
single comparison (unordered greater equal infinity) would do too - but
it's difficult to make those functions both generic and optimized so is
tolerable.)
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.h b/src/gallium/auxiliary/gallivm/lp_bld_arit.h
> index 35119d1..22f940b 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_arit.h
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.h
> @@ -42,6 +42,7 @@
>
> struct lp_type;
> struct lp_build_context;
> +struct gallivm_state;
>
>
> /**
> @@ -353,4 +354,14 @@ lp_build_isfinite(struct lp_build_context *bld,
> LLVMValueRef x);
>
>
> +LLVMValueRef
> +lp_build_is_inf_or_nan(struct gallivm_state *gallivm,
> + const struct lp_type type,
> + LLVMValueRef x);
> +
> +LLVMValueRef
> +lp_build_is_nan(struct gallivm_state *gallivm,
> + const struct lp_type type,
> + LLVMValueRef x);
> +
> #endif /* !LP_BLD_ARIT_H */
>
Otherwise looks good. Though I'm not sure you really need to kill the
prims if the clip distances are infinite? I thought this would be ok (as
opposed to the NaNs which clearly are not). Though maybe we can't handle
the Inf values later on somewhere...
Roland
More information about the mesa-dev
mailing list