Mesa (master): svga: fix shadow comparison failures

Brian Paul brianp at kemper.freedesktop.org
Wed Dec 27 04:47:37 UTC 2017


Module: Mesa
Branch: master
Commit: 1e0b64ced978a952bc6061cda3db2bdcfa0879d7
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=1e0b64ced978a952bc6061cda3db2bdcfa0879d7

Author: Brian Paul <brianp at vmware.com>
Date:   Sat Dec 23 14:16:52 2017 -0700

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 at vmware.com>
Reviewed-by: Charmaine Lee <charmainel at vmware.com>

---

 src/gallium/drivers/svga/svga_context.h       |  2 +-
 src/gallium/drivers/svga/svga_pipe_sampler.c  | 74 +++++++++++++++++----------
 src/gallium/drivers/svga/svga_shader.h        |  5 ++
 src/gallium/drivers/svga/svga_state_sampler.c | 21 ++++++--
 src/gallium/drivers/svga/svga_tgsi_vgpu10.c   | 12 +++--
 5 files changed, 78 insertions(+), 36 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_context.h b/src/gallium/drivers/svga/svga_context.h
index 80c59c05e8..fd0c31222e 100644
--- 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 2e98eb457e..e9d1d6f452 100644
--- 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 dc462c94af..70d1246982 100644
--- 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 763437d9b8..9bd0d5303b 100644
--- 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 c718eae8f3..deb8e5a1ef 100644
--- 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);




More information about the mesa-commit mailing list