On 01/09/2016 12:48 PM, Jason Ekstrand wrote: > > On Jan 8, 2016 6:59 PM, "Ian Romanick" <i...@freedesktop.org > <mailto:i...@freedesktop.org>> wrote: >> >> From: Ian Romanick <ian.d.roman...@intel.com > <mailto:ian.d.roman...@intel.com>> >> >> Some meta operations can be called recursively. Future changes (the >> "Don't pollute the ... namespace" changes) will cause objects with >> invalid names to be used. If a nested meta operation tries to restore >> an object named 0xDEADBEEF, it will fail. >> >> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com > <mailto:ian.d.roman...@intel.com>> >> --- >> src/mesa/drivers/common/meta.h | 3 ++- >> src/mesa/drivers/common/meta_blit.c | 9 +++++---- >> 2 files changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/src/mesa/drivers/common/meta.h > b/src/mesa/drivers/common/meta.h >> index 268824c..856dcdb 100644 >> --- a/src/mesa/drivers/common/meta.h >> +++ b/src/mesa/drivers/common/meta.h >> @@ -310,7 +310,8 @@ struct fb_tex_blit_state >> { >> GLint baseLevelSave, maxLevelSave; >> struct gl_sampler_object *samp_obj; >> - GLuint samplerSave, stencilSamplingSave; >> + struct gl_sampler_object *samp_obj_save; >> + GLuint stencilSamplingSave; >> GLuint tempTex; >> }; >> >> diff --git a/src/mesa/drivers/common/meta_blit.c > b/src/mesa/drivers/common/meta_blit.c >> index 6c03f5d..fc4c87a 100644 >> --- a/src/mesa/drivers/common/meta_blit.c >> +++ b/src/mesa/drivers/common/meta_blit.c >> @@ -810,9 +810,9 @@ void >> _mesa_meta_fb_tex_blit_begin(const struct gl_context *ctx, >> struct fb_tex_blit_state *blit) >> { >> - blit->samplerSave = >> - ctx->Texture.Unit[ctx->Texture.CurrentUnit].Sampler ? >> - ctx->Texture.Unit[ctx->Texture.CurrentUnit].Sampler->Name : 0; >> + blit->samp_obj_save = NULL; > > I'm a bit confused here. Why are we stomping this to null? If it's > ever not null then it either hasn't been initialized or we failed at > reference counting. Why not just assert == null instead?
Neither of callers (blitframebuffer_texture and brw_meta_stencil_blit) preinitialize fb_tex_blit_state to zeros, and both use stack variables. The assumption in the callers is that _mesa_meta_fb_tex_blit_begin will do that for them. If samp_obj_save is not NULL, _mesa_reference_sampler_object will try to dereference it. Since the state isn't persistent across calls, we won't catch ref counting problems. I think I'll add a comment to this effect in _mesa_meta_fb_tex_blit_begin. There are other unsent patches in this series that do similar things. I will audit those before sending them. >> + _mesa_reference_sampler_object(ctx, &blit->samp_obj_save, >> + > ctx->Texture.Unit[ctx->Texture.CurrentUnit].Sampler); >> blit->tempTex = 0; >> } >> >> @@ -838,7 +838,8 @@ _mesa_meta_fb_tex_blit_end(struct gl_context *ctx, > GLenum target, >> } >> } >> >> - _mesa_BindSampler(ctx->Texture.CurrentUnit, blit->samplerSave); >> + _mesa_bind_sampler(ctx, ctx->Texture.CurrentUnit, > blit->samp_obj_save); >> + _mesa_reference_sampler_object(ctx, &blit->samp_obj_save, NULL); >> _mesa_DeleteSamplers(1, &blit->samp_obj->Name); >> blit->samp_obj = NULL; >> >> -- >> 2.5.0 >> >> _______________________________________________ >> mesa-dev mailing list >>mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org> >>http://lists.freedesktop.org/mailman/listinfo/mesa-dev > <http://lists.freedesktop.org/mailman/listinfo/mesa-dev> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
