[Mesa-dev] [PATCH] i965: Clamp "Maximum VP Index" to 1 when gl_ViewportIndex isn't written.

[Kenneth Graunke]

fs_visitor::emit_urb_writes skips writing the VUE header for shaders
that don't write gl_PointSize, gl_Layer, or gl_ViewportIndex.  This
leaves their values uninitialized.  Kristian's nearby comment says:

"But often none of the special varyings that live there are written
 and in that case we can skip writing to the vue header, provided the
 corresponding state properly clamps the values further down the
 pipeline."

However, we were clamping gl_ViewportIndex to [0, 15], so we would end
up using a random viewport.  To fix this, detect when the shader doesn't
write gl_ViewportIndex, and clamp it to [0, 0].

The vec4 backend always writes zeros to the VUE header, so it doesn't
suffer from this problem.  With vec4-style HWord writes, we can write
the header and position together in a single message.  In the FS world,
we would need 4 extra MOVs of 0 and a longer message, or a separate
OWord write.  It's likely cheaper to just clamp the value.

Fixes DiRT Showdown and Bioshock Infinite, which only rendered half of
the screen - the lower left of two triangles.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93054
Cc: mesa-stable at lists.freedesktop.org
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/mesa/drivers/dri/i965/gen6_clip_state.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

I've tried several times to write a Piglit test for this bug, but haven't
managed to make a test that reproduces it.  Something about these games
triggers it every time, but none of my minimal tests do the trick.

diff --git a/src/mesa/drivers/dri/i965/gen6_clip_state.c b/src/mesa/drivers/dri/i965/gen6_clip_state.c
index 5ba3a80..47324b7 100644
--- a/src/mesa/drivers/dri/i965/gen6_clip_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_clip_state.c
@@ -177,6 +177,10 @@ upload_clip_state(struct brw_context *brw)
    if (!is_drawing_points(brw) && !is_drawing_lines(brw))
       dw2 |= GEN6_CLIP_XY_TEST;
 
+   const int max_vp_index =
+      (brw->vue_map_geom_out.slots_valid & VARYING_BIT_VIEWPORT) != 0 ?
+      ctx->Const.MaxViewports : 1;
+
    BEGIN_BATCH(4);
    OUT_BATCH(_3DSTATE_CLIP << 16 | (4 - 2));
    OUT_BATCH(dw1);
@@ -186,7 +190,7 @@ upload_clip_state(struct brw_context *brw)
    OUT_BATCH(U_FIXED(0.125, 3) << GEN6_CLIP_MIN_POINT_WIDTH_SHIFT |
              U_FIXED(255.875, 3) << GEN6_CLIP_MAX_POINT_WIDTH_SHIFT |
              (_mesa_geometric_layers(fb) > 0 ? 0 : GEN6_CLIP_FORCE_ZERO_RTAINDEX) |
-             ((ctx->Const.MaxViewports - 1) & GEN6_CLIP_MAX_VP_INDEX_MASK));
+             ((max_vp_index - 1) & GEN6_CLIP_MAX_VP_INDEX_MASK));
    ADVANCE_BATCH();
 }
 
@@ -201,7 +205,8 @@ const struct brw_tracked_state gen6_clip_state = {
                BRW_NEW_GEOMETRY_PROGRAM |
                BRW_NEW_META_IN_PROGRESS |
                BRW_NEW_PRIMITIVE |
-               BRW_NEW_RASTERIZER_DISCARD,
+               BRW_NEW_RASTERIZER_DISCARD |
+               BRW_NEW_VUE_MAP_GEOM_OUT,
    },
    .emit = upload_clip_state,
 };
@@ -218,7 +223,8 @@ const struct brw_tracked_state gen7_clip_state = {
                BRW_NEW_GEOMETRY_PROGRAM |
                BRW_NEW_META_IN_PROGRESS |
                BRW_NEW_PRIMITIVE |
-               BRW_NEW_RASTERIZER_DISCARD,
+               BRW_NEW_RASTERIZER_DISCARD |
+               BRW_NEW_VUE_MAP_GEOM_OUT,
    },
    .emit = upload_clip_state,
 };
-- 
2.8.2