[Mesa-dev] [PATCH 1/2] svga: fix shadow comparison failures

Brian Paul brianp at vmware.com
Sun Dec 24 05:13:46 UTC 2017


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.
---
 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 80c59c0..fd0c312 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 2e98eb4..e9d1d6f 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 dc462c9..70d1246 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 763437d..9bd0d53 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 c718eae..deb8e5a 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);
-- 
1.9.1



More information about the mesa-dev mailing list