[Mesa-dev] [PATCH] st/mesa: fix sampler view counting

[Brian Paul]

The bug in question was happening because when all texturing got 
turned off we were calling pipe->set_sampler_views(count=0).  In 
llvmpipe this caused the function to return early, before we set the 
LP_NEW_SAMPLER_VIEW flag.  That caused us to miss some state validation.

I agree that unused sampler views shouldn't have an effect on 
rendering, but in this case it does (and perhaps would effect other 
drivers as well).  As I mentioned, drivers used to null-out the 
sampler views not specified by pipe::set_sampler_views(n) but that 
behavior is going away.  During the transition, this patch gives a bit 
of a safety net.

-Brian


On 08/17/2012 12:03 PM, Marek Olšák wrote:
> This looks good, but I don't see how it could fix anything.
> Bound-but-unused sampler views should have no effect on rendering.
>
> Marek
>
> On Fri, Aug 17, 2012 at 4:28 PM, Brian Paul<brianp at vmware.com>  wrote:
>> In the past, when we called pipe::set_sampler_views(n) the drivers set
>> samplers [n..MAX] to NULL.  We no longer do that.  The state tracker
>> code was already trying to set unused sampler views to NULL to cover
>> that case, but the logic was broken and unnoticed until now.  This patch
>> fixes it.
>>
>> Fixes http://bugs.freedesktop.org/show_bug.cgi?id=53617
>> ---
>>   src/mesa/state_tracker/st_atom_texture.c |   11 +++++++++--
>>   1 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_atom_texture.c b/src/mesa/state_tracker/st_atom_texture.c
>> index 6e2efd9..df05e83 100644
>> --- a/src/mesa/state_tracker/st_atom_texture.c
>> +++ b/src/mesa/state_tracker/st_atom_texture.c
>> @@ -265,7 +265,7 @@ update_textures(struct st_context *st,
>>   {
>>      const GLuint old_max = *num_textures;
>>      GLbitfield samplers_used = prog->SamplersUsed;
>> -   GLuint unit;
>> +   GLuint unit, new_count;
>>
>>      if (samplers_used == 0x0&&  old_max == 0)
>>         return;
>> @@ -294,9 +294,16 @@ update_textures(struct st_context *st,
>>         pipe_sampler_view_reference(&(sampler_views[unit]), sampler_view);
>>      }
>>
>> +   /* Ex: if old_max = 3 and *num_textures = 1, we need to pass an
>> +    * array of views={X, NULL, NULL} to unref the old texture views
>> +    * at positions [1] and [2].
>> +    */
>> +   new_count = MAX2(*num_textures, old_max);
>> +   assert(new_count<= max_units);
>> +
>>      cso_set_sampler_views(st->cso_context,
>>                            shader_stage,
>> -                         MIN2(*num_textures, max_units),
>> +                         new_count,
>>                            sampler_views);
>>   }
>>
>> --
>> 1.7.3.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev