svga: fix shadow comparison failures

In some cases, We do shadow comparison cases in the fragment shader
instead of with texture sampler state.  But when we do so, we must
disable the shadow comparison test in the sampler state.  As it
was, we were doing the comparison twice, which resulted in nonsense.
Also, we had the texcoord and texel value swapped in the comparison
instruction.

Fixes about 38 Piglit tex-miplevel-selection tests.

Reviewed-by: Neha Bhende <bhenden@vmware.com>
Reviewed-by: Charmaine Lee <charmainel@vmware.com>

diff --git a/src/gallium/drivers/svga/svga_context.h b/src/gallium/drivers/svga/svga_context.h
index 80c59c05e86d956d0d66a52bbe637975cd50e7e5..fd0c31222e634bd93a0d57f6d2798b71d86f2645 100644 (file)
--- a/src/gallium/drivers/svga/svga_context.h
+++ b/src/gallium/drivers/svga/svga_context.h
@@ -211,7 +211,7 @@ struct svga_sampler_state {
    unsigned view_min_lod;
    unsigned view_max_lod;
 
-   SVGA3dSamplerId id;
+   SVGA3dSamplerId id[2];
 };
 
 

diff --git a/src/gallium/drivers/svga/svga_pipe_sampler.c b/src/gallium/drivers/svga/svga_pipe_sampler.c
index 2e98eb457e056d684a81fe01f89c8d00131e9497..e9d1d6f4527a5bf0646d62cd5b3795703f5bca2b 100644 (file)
--- a/src/gallium/drivers/svga/svga_pipe_sampler.c
+++ b/src/gallium/drivers/svga/svga_pipe_sampler.c
@@ -183,8 +183,6 @@ define_sampler_state_object(struct svga_context *svga,
 
    COPY_4V(bcolor.value, ps->border_color.f);
 
-   ss->id = util_bitmask_add(svga->sampler_object_id_bm);
-
    assert(ps->min_lod <= ps->max_lod);
 
    if (ps->min_mip_filter == PIPE_TEX_MIPFILTER_NONE) {
@@ -196,24 +194,41 @@ define_sampler_state_object(struct svga_context *svga,
       max_lod = ps->max_lod;
    }
 
-   /* Loop in case command buffer is full and we need to flush and retry */
-   for (try = 0; try < 2; try++) {
-      enum pipe_error ret =
-         SVGA3D_vgpu10_DefineSamplerState(svga->swc,
-                                          ss->id,
-                                          filter,
-                                          ss->addressu,
-                                          ss->addressv,
-                                          ss->addressw,
-                                          ss->lod_bias, /* float */
-                                          max_aniso,
-                                          compare_func,
-                                          bcolor,
-                                          min_lod,       /* float */
-                                          max_lod);      /* float */
-      if (ret == PIPE_OK)
-         return;
-      svga_context_flush(svga, NULL);
+   /* If shadow comparisons are enabled, create two sampler states: one
+    * with the given shadow compare mode, another with shadow comparison off.
+    * We need the later because in some cases, we have to do the shadow
+    * compare in the shader.  So, we don't want to do it twice.
+    */
+   STATIC_ASSERT(PIPE_TEX_COMPARE_NONE == 0);
+   STATIC_ASSERT(PIPE_TEX_COMPARE_R_TO_TEXTURE == 1);
+   ss->id[1] = SVGA3D_INVALID_ID;
+
+   unsigned i;
+   for (i = 0; i <= ss->compare_mode; i++) {
+      ss->id[i] = util_bitmask_add(svga->sampler_object_id_bm);
+
+      /* Loop in case command buffer is full and we need to flush and retry */
+      for (try = 0; try < 2; try++) {
+         enum pipe_error ret =
+            SVGA3D_vgpu10_DefineSamplerState(svga->swc,
+                                             ss->id[i],
+                                             filter,
+                                             ss->addressu,
+                                             ss->addressv,
+                                             ss->addressw,
+                                             ss->lod_bias, /* float */
+                                             max_aniso,
+                                             compare_func,
+                                             bcolor,
+                                             min_lod,       /* float */
+                                             max_lod);      /* float */
+         if (ret == PIPE_OK)
+            break;
+         svga_context_flush(svga, NULL);
+      }
+
+      /* turn off the shadow compare option for second iteration */
+      filter &= ~SVGA3D_FILTER_COMPARE;
    }
 }
 
@@ -332,16 +347,21 @@ svga_delete_sampler_state(struct pipe_context *pipe, void *sampler)
    struct svga_context *svga = svga_context(pipe);
 
    if (svga_have_vgpu10(svga)) {
-      enum pipe_error ret;
+      unsigned i;
+      for (i = 0; i < 2; i++) {
+         enum pipe_error ret;
 
-      svga_hwtnl_flush_retry(svga);
+         if (ss->id[i] != SVGA3D_INVALID_ID) {
+            svga_hwtnl_flush_retry(svga);
 
-      ret = SVGA3D_vgpu10_DestroySamplerState(svga->swc, ss->id);
-      if (ret != PIPE_OK) {
-         svga_context_flush(svga, NULL);
-         ret = SVGA3D_vgpu10_DestroySamplerState(svga->swc, ss->id);
+            ret = SVGA3D_vgpu10_DestroySamplerState(svga->swc, ss->id[i]);
+            if (ret != PIPE_OK) {
+               svga_context_flush(svga, NULL);
+               ret = SVGA3D_vgpu10_DestroySamplerState(svga->swc, ss->id[i]);
+            }
+            util_bitmask_clear(svga->sampler_object_id_bm, ss->id[i]);
+         }
       }
-      util_bitmask_clear(svga->sampler_object_id_bm, ss->id);
    }
 
    FREE(sampler);

diff --git a/src/gallium/drivers/svga/svga_shader.h b/src/gallium/drivers/svga/svga_shader.h
index dc462c94af6a58ee92468e5428b949b25de964fb..70d1246982715b1abb9414f397ae2420f0864b8f 100644 (file)
--- a/src/gallium/drivers/svga/svga_shader.h
+++ b/src/gallium/drivers/svga/svga_shader.h
@@ -158,6 +158,11 @@ struct svga_shader_variant
    /** Is the color output just a constant value? (fragment shader only) */
    boolean constant_color_output;
 
+   /** Bitmask indicating which texture units are doing the shadow
+    * comparison test in the shader rather than the sampler state.
+    */
+   unsigned fs_shadow_compare_units;
+
    /** For FS-based polygon stipple */
    unsigned pstipple_sampler_unit;
 

diff --git a/src/gallium/drivers/svga/svga_state_sampler.c b/src/gallium/drivers/svga/svga_state_sampler.c
index 763437d9b87a579ea0eb6024fed64994c8d56719..9bd0d5303bdd31fd0d8273d8698021ae79fd4527 100644 (file)
--- a/src/gallium/drivers/svga/svga_state_sampler.c
+++ b/src/gallium/drivers/svga/svga_state_sampler.c
@@ -391,8 +391,21 @@ update_samplers(struct svga_context *svga, unsigned dirty )
       unsigned nsamplers;
 
       for (i = 0; i < count; i++) {
+         bool fs_shadow = false;
+
+         if (shader == PIPE_SHADER_FRAGMENT) {
+            struct svga_shader_variant *fs = svga->state.hw_draw.fs;
+            /* If the fragment shader is doing the shadow comparison
+             * for this texture unit, don't enable shadow compare in
+             * the texture sampler state.
+             */
+            if (fs->fs_shadow_compare_units & (1 << i)) {
+               fs_shadow = true;
+            }
+         }
+
          if (svga->curr.sampler[shader][i]) {
-            ids[i] = svga->curr.sampler[shader][i]->id;
+            ids[i] = svga->curr.sampler[shader][i]->id[fs_shadow];
             assert(ids[i] != SVGA3D_INVALID_ID);
          }
          else {
@@ -435,18 +448,18 @@ update_samplers(struct svga_context *svga, unsigned dirty )
       }
 
       if (svga->state.hw_draw.samplers[PIPE_SHADER_FRAGMENT][unit]
-          != sampler->id) {
+          != sampler->id[0]) {
          ret = SVGA3D_vgpu10_SetSamplers(svga->swc,
                                          1, /* count */
                                          unit, /* start */
                                          SVGA3D_SHADERTYPE_PS,
-                                         &sampler->id);
+                                         &sampler->id[0]);
          if (ret != PIPE_OK)
             return ret;
 
          /* save the polygon stipple sampler in the hw draw state */
          svga->state.hw_draw.samplers[PIPE_SHADER_FRAGMENT][unit] =
-            sampler->id;
+            sampler->id[0];
       }
    }
 

diff --git a/src/gallium/drivers/svga/svga_tgsi_vgpu10.c b/src/gallium/drivers/svga/svga_tgsi_vgpu10.c
index c718eae8f3a605b97f504e4fb36974fe99a0d2d3..deb8e5a1efff7799f54d2c056013876d51e5f89d 100644 (file)
--- a/src/gallium/drivers/svga/svga_tgsi_vgpu10.c
+++ b/src/gallium/drivers/svga/svga_tgsi_vgpu10.c
@@ -181,6 +181,9 @@ struct svga_shader_emitter_v10
 
       unsigned fragcoord_input_index;  /**< real fragment position input reg */
       unsigned fragcoord_tmp_index;    /**< 1/w modified position temp reg */
+
+      /** Which texture units are doing shadow comparison in the FS code */
+      unsigned shadow_compare_units;
    } fs;
 
    /* For geometry shaders only */
@@ -4851,6 +4854,8 @@ begin_tex_swizzle(struct svga_shader_emitter_v10 *emit,
    }
    swz->inst_dst = &inst->Dst[0];
    swz->coord_src = &inst->Src[0];
+
+   emit->fs.shadow_compare_units |= shadow_compare << unit;
 }
 
 
@@ -4909,11 +4914,8 @@ end_tex_swizzle(struct svga_shader_emitter_v10 *emit,
       }
 
       /* COMPARE tmp, coord, texel */
-      /* XXX it would seem that the texel and coord arguments should
-       * be transposed here, but piglit tests indicate otherwise.
-       */
       emit_comparison(emit, compare_func,
-                      &swz->tmp_dst, &texel_src, &coord_src);
+                      &swz->tmp_dst, &coord_src, &texel_src);
 
       /* AND dest, tmp, {1.0} */
       begin_emit_instruction(emit);
@@ -6565,6 +6567,8 @@ svga_tgsi_vgpu10_translate(struct svga_context *svga,
       }
    }
 
+   variant->fs_shadow_compare_units = emit->fs.shadow_compare_units;
+
    if (SVGA_DEBUG & DEBUG_TGSI) {
       debug_printf("#####################################\n");
       debug_printf("### TGSI Shader %u\n", shader->id);