On Thu, 27 Oct 2011 10:27:40 -0700, Paul Berry <stereotype...@gmail.com> wrote:
Non-text part: multipart/alternative
> On 24 October 2011 14:17, Eric Anholt <e...@anholt.net> wrote:
>
> > This rearranges the code a bit, and makes the upload of the binding
> > table take only as many surfaces as there are in use.
> > ---
> > src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 62
> > +++++++++-------------
> > 1 files changed, 25 insertions(+), 37 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > index bfc4f5d..0237b58 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> > @@ -138,22 +138,6 @@ brw_update_vs_constant_surface( struct gl_context
> > *ctx,
> > }
> > }
> >
> > -
> > -static void
> > -prepare_vs_surfaces(struct brw_context *brw)
> > -{
> > - int nr_surfaces = 0;
> > -
> > - if (brw->vs.const_bo) {
> > - nr_surfaces = 1;
> > - }
> > -
> > - if (brw->vs.nr_surfaces != nr_surfaces) {
> > - brw->state.dirty.brw |= BRW_NEW_NR_VS_SURFACES;
> > - brw->vs.nr_surfaces = nr_surfaces;
> > - }
> > -}
> > -
> > /**
> > * Vertex shader surfaces (constant buffer).
> > *
> > @@ -161,36 +145,41 @@ prepare_vs_surfaces(struct brw_context *brw)
> > * to be updated, and produces BRW_NEW_NR_VS_SURFACES for the VS unit and
> > * CACHE_NEW_SURF_BIND for the binding table upload.
> > */
> > -static void upload_vs_surfaces(struct brw_context *brw)
> > +static void
> > +brw_upload_vs_surfaces(struct brw_context *brw)
> > {
> > struct gl_context *ctx = &brw->intel.ctx;
> > uint32_t *bind;
> > int i;
> > + int nr_surfaces = 0;
> > +
> > + /* BRW_NEW_VS_CONSTBUF */
> > + if (brw->vs.const_bo) {
> > + nr_surfaces = 1;
> > + brw_update_vs_constant_surface(ctx, SURF_INDEX_VERT_CONST_BUFFER);
> > + }
> > +
> > + if (nr_surfaces != 0) {
> >
>
> This reads a little strange to me. The only way nr_surfaces can be nonzero
> is if the previous if test succeeded, so why not just merge the two if
> statements?
>
>
> > + bind = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> > + sizeof(uint32_t) * nr_surfaces,
> > + 32, &brw->vs.bind_bo_offset);
> >
> > - /* BRW_NEW_NR_VS_SURFACES */
> > - if (brw->vs.nr_surfaces == 0) {
> > + for (i = 0; i < nr_surfaces; i++) {
> > + /* BRW_NEW_VS_CONSTBUF */
> > + bind[i] = brw->vs.surf_offset[i];
> > + }
> >
>
> This for loop also seems weird, since at this point in the code nr_surfaces
> is known to be equal to 1. Why not just do "bind[0] =
> brw->vs.surf_offset[0];"?Basically, because this is only 1/2 of what this function should be doing, and it's about to be get a new block increasing nr_surfaces for vertex texturing.
pgp6vR9dVzv3y.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
