On Wed, 26 Oct 2011 10:11:15 -0700, Paul Berry <stereotype...@gmail.com> wrote: Non-text part: multipart/alternative > On 24 October 2011 14:17, Eric Anholt <e...@anholt.net> wrote: > > > These produce BRW_NEW_SURFACES (used by binding table emit()) and > > BRW_NEW_NR_WM_SURFACES (used by WM unit emit()). Fixes a bug where > > with no texturing and no color buffer, we wouldn't consider the null > > renderbuffer in nr_surfaces. This was harmless because nr_surfaces is > > only used for the prefetch info in the unit state.
> > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > index bb7fd2e..782efd5 100644
> > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > @@ -541,47 +541,16 @@ brw_update_renderbuffer_surface(struct brw_context
> > *brw,
> > I915_GEM_DOMAIN_RENDER);
> > }
> >
> > -static void
> > -prepare_wm_surfaces(struct brw_context *brw)
> > -{
> > - struct gl_context *ctx = &brw->intel.ctx;
> > - int i;
> > - int nr_surfaces = 0;
> > -
> > - for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) {
> > - nr_surfaces = SURF_INDEX_DRAW(i) + 1;
> > - }
> > -
> > - if (brw->wm.const_bo) {
> > - nr_surfaces = SURF_INDEX_FRAG_CONST_BUFFER + 1;
> > - }
> > -
> > - for (i = 0; i < BRW_MAX_TEX_UNIT; i++) {
> > - const struct gl_texture_unit *texUnit = &ctx->Texture.Unit[i];
> > -
> > - if (texUnit->_ReallyEnabled) {
> > - nr_surfaces = SURF_INDEX_TEXTURE(i) + 1;
> > - }
> > - }
> > -
> > - /* Have to update this in our prepare, since the unit's prepare
> > - * relies on it.
> > - */
> > - if (brw->wm.nr_surfaces != nr_surfaces) {
> > - brw->wm.nr_surfaces = nr_surfaces;
> > - brw->state.dirty.brw |= BRW_NEW_NR_WM_SURFACES;
> > - }
> > -}
> > -
> > /**
> > * Constructs the set of surface state objects pointed to by the
> > * binding table.
> > */
> > static void
> > -upload_wm_surfaces(struct brw_context *brw)
> > +brw_upload_wm_surfaces(struct brw_context *brw)
> > {
> > struct gl_context *ctx = &brw->intel.ctx;
> > GLuint i;
> > + int nr_surfaces = 0;
> >
> > /* _NEW_BUFFERS | _NEW_COLOR */
> > /* Update surfaces for drawing buffers */
> > @@ -595,8 +564,15 @@ upload_wm_surfaces(struct brw_context *brw)
> > brw_update_null_renderbuffer_surface(brw, i);
> > }
> > }
> > + nr_surfaces =
> > SURF_INDEX_DRAW(ctx->DrawBuffer->_NumColorDrawBuffers);
> > } else {
> > brw_update_null_renderbuffer_surface(brw, 0);
> > + nr_surfaces = SURF_INDEX_DRAW(0) + 1;
> >
>
> This looks like a behavioral change to me. The old prepare_wm_surfaces()
> code would have left nr_surfaces = 0 in this case. Was this an intentional
> change? Was the old code buggy?
>
> I have similar questions about patch 15/33.
I mentioned it in the commit message :)
pgpu5ACXipj2a.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
