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

[Roland Scheidegger]

Am 03.04.2014 16:56, schrieb Brian Paul:
> 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.  Rather than assert and die, issue
> a warning.
> 
> Cc: "10.0" "10.1" <mesa-stable at lists.freedesktop.org>
> ---
>  src/gallium/drivers/svga/svga_state_fs.c |   12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gallium/drivers/svga/svga_state_fs.c b/src/gallium/drivers/svga/svga_state_fs.c
> index dde739c..1d7e43e 100644
> --- a/src/gallium/drivers/svga/svga_state_fs.c
> +++ b/src/gallium/drivers/svga/svga_state_fs.c
> @@ -235,15 +235,19 @@ make_fs_key(const struct svga_context *svga,
>     if (svga->curr.blend->need_white_fragments) {
>        key->white_fragments = 1;
>     }
> -   
> +
> +   if (svga->curr.num_sampler_views != svga->curr.num_samplers) {
> +      debug_warn_once("svga: mismatched number of sampler views "
> +                      "vs. samplers\n");
> +   }
> +
Note it is a pretty weak check anyway - you could have the same number
of views and samplers and they could still be mismatched (e.g. views
0/1/3 and and samplers 1/2/3).

>     /* 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) {
> @@ -262,7 +266,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 to me.

(I am not sure if it would make sense to require views and samplers
being consistent here. As you said, for d3d10 it is obviously a no-go,
and requiring it for shaders only if they use the gl-style tex opcodes
therefore would be slightly odd. OTOH almost noone (except llvmpipe)
really understands the d3d10 sample opcodes and I don't know if drivers
handle that properly otherwise or assume these being consistent.)

Roland