[Mesa-dev] [PATCH 07/18] meta/blit: Save and restore the sampler using gl_sampler_object instead of GL API object handle

[Ian Romanick]

On 01/09/2016 12:48 PM, Jason Ekstrand wrote:
> 
> On Jan 8, 2016 6:59 PM, "Ian Romanick" <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>>
>> From: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at 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.romanick at intel.com
> <mailto:ian.d.romanick at 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 at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>>http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> <http://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>