Looks good to me AFAICT. Jose
----- Original Message ----- > From: Roland Scheidegger <srol...@vmware.com> > > Instead of skipping x/y clipping completely if there's point_tri_clip points > use guard band clipping. This should be easier (previously we could not > disable > generating the x/y bits in the clip mask for llvm path, hence requiring > custom > clip path), and it also allows us to enable this for tris-as-points more > easily > too (this would require custom tri clip filtering too otherwise). Moreover, > some unexpected things could have happen if there's a NaN or just a huge > number > in some tri-turned-point, as the driver's rasterizer would need to deal with > it > and that might well lead to undefined behavior in typical rasterizers (which > need to convert these numbers to fixed point). Using a guardband should hence > be more robust, while "usually" guaranteeing the same results. (Only > "usually" > because unlike hw guardbands draw guardband is always just twice the vp size, > hence small vp but large points could still lead to different results.) > Unfortunately because the clipmask generated is completely unaffected by > guard > band clipping, we still need a custom clip stage for points (but not for > tris, > as the actual clipping there takes guard band into account). > --- > src/gallium/auxiliary/draw/draw_context.c | 8 ++--- > src/gallium/auxiliary/draw/draw_pipe_clip.c | 34 > ++++++++++++++------ > src/gallium/auxiliary/draw/draw_private.h | 2 +- > .../auxiliary/draw/draw_pt_fetch_shade_pipeline.c | 15 +++++---- > .../draw/draw_pt_fetch_shade_pipeline_llvm.c | 8 +++-- > 5 files changed, 42 insertions(+), 25 deletions(-) > > diff --git a/src/gallium/auxiliary/draw/draw_context.c > b/src/gallium/auxiliary/draw/draw_context.c > index 9b5bcb5..842fdd3 100644 > --- a/src/gallium/auxiliary/draw/draw_context.c > +++ b/src/gallium/auxiliary/draw/draw_context.c > @@ -262,10 +262,10 @@ static void update_clip_flags( struct draw_context > *draw ) > draw->rasterizer && draw->rasterizer->depth_clip); > draw->clip_user = draw->rasterizer && > draw->rasterizer->clip_plane_enable != 0; > - draw->clip_points_xy = draw->clip_xy && > - (!draw->driver.bypass_clip_points || > - (draw->rasterizer && > - !draw->rasterizer->point_tri_clip)); > + draw->guard_band_points_xy = draw->guard_band_xy || > + (draw->driver.bypass_clip_points && > + (draw->rasterizer && > + draw->rasterizer->point_tri_clip)); > } > > /** > diff --git a/src/gallium/auxiliary/draw/draw_pipe_clip.c > b/src/gallium/auxiliary/draw/draw_pipe_clip.c > index adfa4b6..9b339ae 100644 > --- a/src/gallium/auxiliary/draw/draw_pipe_clip.c > +++ b/src/gallium/auxiliary/draw/draw_pipe_clip.c > @@ -615,19 +615,33 @@ clip_point( struct draw_stage *stage, > stage->next->point( stage->next, header ); > } > > + > /* > * Clip points but ignore the first 4 (xy) clip planes. > - * (This is necessary because we don't generate a different shader variant > - * just for points hence xy clip bits are still generated. This is not > really > - * optimal because of the extra calculations both in generating clip masks > - * and executing the clip stage but it gets the job done.) > + * (Because the generated clip mask is completely unaffacted by guard band, > + * we still need to manually evaluate the x/y planes if they are outside > + * the guard band and not just outside the vp.) > */ > static void > -clip_point_no_xy( struct draw_stage *stage, > - struct prim_header *header ) > +clip_point_guard_xy( struct draw_stage *stage, > + struct prim_header *header ) > { > - if ((header->v[0]->clipmask & 0xfffffff0) == 0) > - stage->next->point( stage->next, header ); > + unsigned clipmask = header->v[0]->clipmask; > + if ((clipmask & 0xffffffff) == 0) > + stage->next->point(stage->next, header); > + else if ((clipmask & 0xfffffff0) == 0) { > + while (clipmask) { > + const unsigned plane_idx = ffs(clipmask)-1; > + clipmask &= ~(1 << plane_idx); /* turn off this plane's bit */ > + /* TODO: this should really do proper guardband clipping, > + * currently just throw out infs/nans. > + */ > + if (util_is_inf_or_nan(header->v[0]->clip[0]) || > + util_is_inf_or_nan(header->v[0]->clip[1])) > + return; > + } > + stage->next->point(stage->next, header); > + } > } > > > @@ -636,7 +650,7 @@ static void > clip_first_point( struct draw_stage *stage, > struct prim_header *header ) > { > - stage->point = stage->draw->clip_points_xy ? clip_point : > clip_point_no_xy; > + stage->point = stage->draw->guard_band_points_xy ? clip_point_guard_xy : > clip_point; > stage->point(stage, header); > } > > @@ -662,7 +676,7 @@ clip_line( struct draw_stage *stage, > > static void > clip_tri( struct draw_stage *stage, > - struct prim_header *header ) > + struct prim_header *header ) > { > unsigned clipmask = (header->v[0]->clipmask | > header->v[1]->clipmask | > diff --git a/src/gallium/auxiliary/draw/draw_private.h > b/src/gallium/auxiliary/draw/draw_private.h > index 5bcb8a8..7f9b26c 100644 > --- a/src/gallium/auxiliary/draw/draw_private.h > +++ b/src/gallium/auxiliary/draw/draw_private.h > @@ -232,7 +232,7 @@ struct draw_context > boolean clip_z; > boolean clip_user; > boolean guard_band_xy; > - boolean clip_points_xy; > + boolean guard_band_points_xy; > > boolean force_passthrough; /**< never clip or shade */ > > diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > index ab8a0c6..67c0c16 100644 > --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > @@ -60,7 +60,7 @@ struct fetch_pipeline_middle_end { > */ > static void fetch_pipeline_prepare( struct draw_pt_middle_end *middle, > unsigned prim, > - unsigned opt, > + unsigned opt, > unsigned *max_vertices ) > { > struct fetch_pipeline_middle_end *fpme = (struct > fetch_pipeline_middle_end *)middle; > @@ -72,8 +72,10 @@ static void fetch_pipeline_prepare( struct > draw_pt_middle_end *middle, > > const unsigned gs_out_prim = (gs ? gs->output_primitive : > u_assembled_prim(prim)); > - unsigned nr = MAX2( vs->info.num_inputs, > - draw_total_vs_outputs(draw) ); > + unsigned nr = MAX2(vs->info.num_inputs, > + draw_total_vs_outputs(draw)); > + unsigned point_clip = draw->rasterizer->fill_front == > PIPE_POLYGON_MODE_POINT || > + gs_out_prim == PIPE_PRIM_POINTS; > > if (gs) { > nr = MAX2(nr, gs->info.num_outputs + 1); > @@ -97,18 +99,17 @@ static void fetch_pipeline_prepare( struct > draw_pt_middle_end *middle, > */ > fpme->vertex_size = sizeof(struct vertex_header) + nr * 4 * > sizeof(float); > > - > > draw_pt_fetch_prepare( fpme->fetch, > vs->info.num_inputs, > fpme->vertex_size, > instance_id_index ); > draw_pt_post_vs_prepare( fpme->post_vs, > - gs_out_prim == PIPE_PRIM_POINTS ? > - draw->clip_points_xy : > draw->clip_xy, > + draw->clip_xy, > draw->clip_z, > draw->clip_user, > - draw->guard_band_xy, > + point_clip ? draw->guard_band_points_xy : > + draw->guard_band_xy, > draw->identity_viewport, > draw->rasterizer->clip_halfz, > (draw->vs.edgeflag_output ? TRUE : FALSE) ); > diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c > b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c > index bca658f..a1a99dc 100644 > --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c > +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c > @@ -143,6 +143,8 @@ llvm_middle_end_prepare( struct draw_pt_middle_end > *middle, > u_assembled_prim(in_prim); > const unsigned nr = MAX2(vs->info.num_inputs, > draw_total_vs_outputs(draw)); > + unsigned point_clip = draw->rasterizer->fill_front == > PIPE_POLYGON_MODE_POINT || > + out_prim == PIPE_PRIM_POINTS; > > fpme->input_prim = in_prim; > fpme->opt = opt; > @@ -155,11 +157,11 @@ llvm_middle_end_prepare( struct draw_pt_middle_end > *middle, > > > draw_pt_post_vs_prepare( fpme->post_vs, > - out_prim == PIPE_PRIM_POINTS ? > - draw->clip_points_xy : > draw->clip_xy, > + draw->clip_xy, > draw->clip_z, > draw->clip_user, > - draw->guard_band_xy, > + point_clip ? draw->guard_band_points_xy : > + draw->guard_band_xy, > draw->identity_viewport, > draw->rasterizer->clip_halfz, > (draw->vs.edgeflag_output ? TRUE : FALSE) ); > -- > 1.7.9.5 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev