[Mesa-dev] [PATCH] svga: replace sampler assertion with conditional

[Thomas Hellstrom]

On 04/04/2014 01:19 AM, Brian Paul wrote:
> For TEX instructions, the set of samplers and sampler views should
> be consistent.  The XA state tracker sometimes passes an inconsistent
> set of samplers and sampler views.

I might be wrong, but I don't think the XA tracker sends an inconsistent
set here.

>   Rather than assert and die, issue
> a warning.
>
> v2: add debugging code to detect inconsistent state.
> ---
>  src/gallium/drivers/svga/svga_state_fs.c |   31 ++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/svga/svga_state_fs.c b/src/gallium/drivers/svga/svga_state_fs.c
> index 1cdd44f..8837951 100644
> --- a/src/gallium/drivers/svga/svga_state_fs.c
> +++ b/src/gallium/drivers/svga/svga_state_fs.c
> @@ -237,15 +237,38 @@ make_fs_key(const struct svga_context *svga,
>     if (svga->curr.blend->need_white_fragments) {
>        key->white_fragments = 1;
>     }
> -   
> +
> +#ifdef DEBUG
> +   /*
> +    * We expect a consistent set of samplers and sampler views.
> +    * Do some debug checks/warnings here.
> +    */
> +   {
> +      unsigned i, n = MAX2(svga->curr.num_sampler_views,
> +                           svga->curr.num_samplers);
> +      if (svga->curr.num_sampler_views != svga->curr.num_samplers) {
> +         debug_printf("svga: mismatched number of sampler views (%u) "
> +                      "vs. samplers (%u)\n",
> +                      svga->curr.num_sampler_views,
> +                      svga->curr.num_samplers);
> +      }
> +      for (i = 0; i < n; i++) {
> +         if ((svga->curr.sampler_views[i] == NULL) !=
> +             (svga->curr.sampler[i] == NULL))
> +            debug_printf("sampler_view[%u] = %p but sampler[%u] = %p\n",
> +                         i, svga->curr.sampler_views[i],
> +                         i, svga->curr.sampler[i]);
> +      }
> +   }
> +#endif
> +
>     /* XXX: want to limit this to the textures that the shader actually
>      * refers to.
>      *
>      * SVGA_NEW_TEXTURE_BINDING | SVGA_NEW_SAMPLER
>      */
>     for (i = 0; i < svga->curr.num_sampler_views; i++) {
> -      if (svga->curr.sampler_views[i]) {
> -         assert(svga->curr.sampler[i]);
> +      if (svga->curr.sampler_views[i] && svga->curr.sampler[i]) {
>           assert(svga->curr.sampler_views[i]->texture);
>           key->tex[i].texture_target = svga->curr.sampler_views[i]->texture->target;
>           if (!svga->curr.sampler[i]->normalized_coords) {
> @@ -264,7 +287,7 @@ make_fs_key(const struct svga_context *svga,
>  
>     idx = 0;
>     for (i = 0; i < svga->curr.num_samplers; ++i) {
> -      if (svga->curr.sampler_views[i]) {
> +      if (svga->curr.sampler_views[i] && svga->curr.sampler[i]) {
>           struct pipe_resource *tex = svga->curr.sampler_views[i]->texture;
>           struct svga_texture *stex = svga_texture(tex);
>           SVGA3dSurfaceFormat format = stex->key.format;

Looks good otherwise
Reviewed-by: Thomas Hellstrom <thellstrom at vmware.com>

/Thomas