[Mesa-dev] [PATCH 1/2] i965: Refactor brw_draw_prims.

[Kenneth Graunke]

On Tuesday, August 29, 2017 9:24:00 AM PDT Plamena Manolova wrote:
> brw_draw_prims needs to be refactored prior to
> ARB_indirect_parameters implementation.
> 
> Signed-off-by: Plamena Manolova <plamena.manolova at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_draw.c | 343 +++++++++++++++++++----------------
>  1 file changed, 189 insertions(+), 154 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
> index a8ad2ac..7597bae 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -531,7 +531,7 @@ brw_postdraw_set_buffers_need_resolve(struct brw_context *brw)
>  
>        if (!irb)
>           continue;
> -     
> +
>        brw_render_cache_set_add_bo(brw, irb->mt->bo);
>        intel_miptree_finish_render(brw, irb->mt, irb->mt_level,
>                                    irb->mt_layer, irb->layer_count,
> @@ -594,21 +594,163 @@ brw_postdraw_reconcile_align_wa_slices(struct brw_context *brw)
>   * fallback conditions.
>   */
>  static void
> -brw_try_draw_prims(struct gl_context *ctx,
> -                   const struct gl_vertex_array *arrays[],
> -                   const struct _mesa_prim *prims,
> -                   GLuint nr_prims,
> -                   const struct _mesa_index_buffer *ib,
> -                   bool index_bounds_valid,
> -                   GLuint min_index,
> -                   GLuint max_index,
> -                   struct brw_transform_feedback_object *xfb_obj,
> -                   unsigned stream,
> -                   struct gl_buffer_object *indirect)
> +brw_try_draw_prim(struct gl_context *ctx,
> +                  const struct gl_vertex_array *arrays[],
> +                  const struct _mesa_prim *prim,
> +                  const struct _mesa_index_buffer *ib,
> +                  bool index_bounds_valid,
> +                  GLuint min_index,
> +                  GLuint max_index,
> +                  struct brw_transform_feedback_object *xfb_obj,
> +                  unsigned stream,
> +                  struct gl_buffer_object *indirect)
>  {

Hey Pam,

I'm sorry it took me so long to look at these patches...especially since
this refactoring patch isn't going to apply anymore. :(

A couple of bits of feedback:

- You changed 'i' to 'prim->draw_id' in a couple of places.  That looks
  fine, but could we do that in a separate patch, before the other
  refactors?  That way, if the vbo module fails to set prim->draw_id in
  some case, and this breaks something, 'git bisect' would find a tiny
  simple patch instead of one with a lot of code motion.

- What do you think of calling these functions...

  - brw_prepare_drawing()  (instead of brw_prepare_draw_prims)
  - brw_finish_drawing()   (instead of brw_end_draw_prims)
  - brw_draw_single_prim() (instead of brw_try_draw_prim)

- Would it be possible to introduce those functions in separate patches?

Otherwise, this looks good to me.

I'll try and land your new refactoring patches as soon as you respin
them so you don't end up having to rewrite them a third time.  Again,
sorry about that...!

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170925/96583f43/attachment-0001.sig>