[Mesa-dev] [RFC PATCH 08/10] i956/clip: push dp4 into load_clip_distance

[Paul Berry]

On 3 August 2013 19:59, Chris Forbes <chrisf at ijw.co.nz> wrote:

> Soon the dp4 is only going to be used for fixed clip planes.
>
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> ---
>  src/mesa/drivers/dri/i965/brw_clip_tri.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_tri.c
> b/src/mesa/drivers/dri/i965/brw_clip_tri.c
> index daa7a5f..bd3ffb3 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_tri.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_tri.c
> @@ -224,17 +224,19 @@ void brw_clip_tri_flat_shade( struct
> brw_clip_compile *c )
>
>
>  static inline void
> -load_vertex_pos(struct brw_clip_compile *c, struct brw_indirect vtx,
> -                struct brw_reg dst,
> -                GLuint hpos_offset, GLuint clip_offset)
> +load_clip_distance(struct brw_clip_compile *c, struct brw_indirect vtx,
> +                struct brw_reg dst, GLuint hpos_offset, GLuint
> clip_offset,
> +                int cond)
>

Can we add a comment above this function explaining what it does?  In
particular, it would be nice to have a comment explaining that this
function ends with an implicit comparison between the clip distance and
zero, using "cond" as a conditional modifier.

With that fixed, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>


>  {
>     struct brw_compile *p = &c->func;
>
>     /*
>      * Roughly:
>      * dst = (vertex_src_mask & 1) ? src.hpos : src.clipvertex;
> +    * dst = dp4(dst, plane)         + set f0 based on `cond`
>      */
>
> +   dst = vec4(dst);
>     brw_set_conditionalmod(p, BRW_CONDITIONAL_NZ);
>     brw_AND(p, vec1(brw_null_reg()), c->reg.vertex_src_mask,
> brw_imm_ud(1));
>     brw_IF(p, BRW_EXECUTE_1);
> @@ -246,6 +248,9 @@ load_vertex_pos(struct brw_clip_compile *c, struct
> brw_indirect vtx,
>        brw_MOV(p, dst, deref_4f(vtx, hpos_offset));
>     }
>     brw_ENDIF(p);
> +
> +   brw_set_conditionalmod(p, cond);
> +   brw_DP4(p, dst, dst, c->reg.plane_equation);
>  }
>
>
> @@ -313,17 +318,15 @@ void brw_clip_tri( struct brw_clip_compile *c )
>              */
>             brw_MOV(p, get_addr_reg(vtx), deref_1uw(inlist_ptr, 0));
>
> -            load_vertex_pos(c, vtxPrev, vec4(c->reg.dpPrev), hpos_offset,
> clipvert_offset);
> +            load_clip_distance(c, vtxPrev, c->reg.dpPrev, hpos_offset,
> clipvert_offset,
> +                  BRW_CONDITIONAL_L);
>             /* IS_NEGATIVE(prev) */
> -           brw_set_conditionalmod(p, BRW_CONDITIONAL_L);
> -           brw_DP4(p, vec4(c->reg.dpPrev), vec4(c->reg.dpPrev),
> c->reg.plane_equation);
>             brw_IF(p, BRW_EXECUTE_1);
>             {
> -               load_vertex_pos(c, vtx, vec4(c->reg.dp), hpos_offset,
> clipvert_offset);
> +               load_clip_distance(c, vtx, c->reg.dp, hpos_offset,
> clipvert_offset,
> +                     BRW_CONDITIONAL_GE);
>                /* IS_POSITIVE(next)
>                 */
> -              brw_set_conditionalmod(p, BRW_CONDITIONAL_GE);
> -              brw_DP4(p, vec4(c->reg.dp), vec4(c->reg.dp),
> c->reg.plane_equation);
>                brw_IF(p, BRW_EXECUTE_1);
>                {
>
> @@ -362,11 +365,10 @@ void brw_clip_tri( struct brw_clip_compile *c )
>                brw_ADD(p, get_addr_reg(outlist_ptr),
> get_addr_reg(outlist_ptr), brw_imm_uw(sizeof(short)));
>                brw_ADD(p, c->reg.nr_verts, c->reg.nr_verts, brw_imm_ud(1));
>
> -               load_vertex_pos(c, vtx, vec4(c->reg.dp), hpos_offset,
> clipvert_offset);
> +               load_clip_distance(c, vtx, c->reg.dp, hpos_offset,
> clipvert_offset,
> +                     BRW_CONDITIONAL_L);
>                /* IS_NEGATIVE(next)
>                 */
> -              brw_set_conditionalmod(p, BRW_CONDITIONAL_L);
> -              brw_DP4(p, vec4(c->reg.dp), vec4(c->reg.dp),
> c->reg.plane_equation);
>                brw_IF(p, BRW_EXECUTE_1);
>                {
>                   /* Going out of bounds.  Avoid division by zero as we
> --
> 1.8.3.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130805/48b110f0/attachment-0001.html>