[Mesa-dev] [PATCH] i965: Always scissor on Gen7/7.5 instead of disabling guardband.

[Kenneth Graunke]

Previously we disabled the guardband when the viewport was smaller than
the framebuffer on Gen7/7.5, to prevent portions of primitives from
being draw outside of the viewport.  On Gen8+, we relied on the viewport
extents test to effectively scissor this away for us.

We can simply always enable scissoring instead.  We already include the
viewport in the scissor rectangle, so this will effectively do the
viewport extents test for us.  (The only difference is that the scissor
rectangle doesn't support sub-pixel values.  I think that's okay.)

Scissoring ought to be cheap (more or less free?), and enabling the
guardband reduces the cost of clipping.  It also allows us to drop
_NEW_SCISSOR from this atom (which isn't a huge deal - we already
listen to a ton of other bits).

This also fixes misrendering in Blender, where the "floor plane" grid
lines started rendering at wrong angles after I disabled XY clipping
of line primitives.  Enabling the guardband seems to solve the issue.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99339
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/mesa/drivers/dri/i965/gen6_clip_state.c | 26 --------------------------
 src/mesa/drivers/dri/i965/gen6_sf_state.c   | 15 +++------------
 src/mesa/drivers/dri/i965/gen7_sf_state.c   | 12 ++----------
 3 files changed, 5 insertions(+), 48 deletions(-)

I have not benchmarked this yet.  I should.

diff --git a/src/mesa/drivers/dri/i965/gen6_clip_state.c b/src/mesa/drivers/dri/i965/gen6_clip_state.c
index 0b3c7f16f18..8e893f5668f 100644
--- a/src/mesa/drivers/dri/i965/gen6_clip_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_clip_state.c
@@ -203,32 +203,6 @@ upload_clip_state(struct brw_context *brw)
       }
    }
 
-   /* If the viewport dimensions are smaller than the drawable dimensions,
-    * we have to disable guardband clipping prior to Gen8.  We always program
-    * the guardband to a fixed size, which is almost always larger than the
-    * viewport.  Any geometry which intersects the viewport but lies within
-    * the guardband would bypass the 3D clipping stage, so it wouldn't be
-    * clipped to the viewport.  Rendering would happen beyond the viewport,
-    * but still inside the drawable.
-    *
-    * Gen8+ introduces a viewport extents test which restricts rendering to
-    * the viewport, so we can ignore this restriction.
-    */
-   if (brw->gen < 8) {
-      const float fb_width = (float)_mesa_geometric_width(fb);
-      const float fb_height = (float)_mesa_geometric_height(fb);
-
-      for (unsigned i = 0; i < viewport_count; i++) {
-         if (ctx->ViewportArray[i].X != 0 ||
-             ctx->ViewportArray[i].Y != 0 ||
-             ctx->ViewportArray[i].Width != fb_width ||
-             ctx->ViewportArray[i].Height != fb_height) {
-            dw2 &= ~GEN6_CLIP_GB_TEST;
-            break;
-         }
-      }
-   }
-
    /* BRW_NEW_RASTERIZER_DISCARD */
    if (ctx->RasterDiscard) {
       dw2 |= GEN6_CLIP_MODE_REJECT_ALL;
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 738e4f03cda..dd547790c9a 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -286,13 +286,12 @@ upload_sf_state(struct brw_context *brw)
 
    dw1 = GEN6_SF_SWIZZLE_ENABLE | num_outputs << GEN6_SF_NUM_OUTPUTS_SHIFT;
    dw2 = GEN6_SF_STATISTICS_ENABLE;
+   dw3 = GEN6_SF_SCISSOR_ENABLE;
+   dw4 = 0;
 
    if (brw->sf.viewport_transform_enable)
        dw2 |= GEN6_SF_VIEWPORT_TRANSFORM_ENABLE;
 
-   dw3 = 0;
-   dw4 = 0;
-
    /* _NEW_POLYGON */
    if (ctx->Polygon._FrontBit == render_to_fbo)
       dw2 |= GEN6_SF_WINDING_CCW;
@@ -340,13 +339,6 @@ upload_sf_state(struct brw_context *brw)
        unreachable("not reached");
    }
 
-   /* _NEW_SCISSOR | _NEW_POLYGON,
-    * BRW_NEW_GS_PROG_DATA | BRW_NEW_TES_PROG_DATA | BRW_NEW_PRIMITIVE
-    */
-   if (ctx->Scissor.EnableFlags ||
-       brw_is_drawing_points(brw) || brw_is_drawing_lines(brw))
-      dw3 |= GEN6_SF_SCISSOR_ENABLE;
-
    /* _NEW_POLYGON */
    if (ctx->Polygon.CullFlag) {
       switch (ctx->Polygon.CullFaceMode) {
@@ -449,8 +441,7 @@ const struct brw_tracked_state gen6_sf_state = {
                _NEW_MULTISAMPLE |
                _NEW_POINT |
                _NEW_POLYGON |
-               _NEW_PROGRAM |
-               _NEW_SCISSOR,
+               _NEW_PROGRAM,
       .brw   = BRW_NEW_BLORP |
                BRW_NEW_CONTEXT |
                BRW_NEW_FRAGMENT_PROGRAM |
diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c b/src/mesa/drivers/dri/i965/gen7_sf_state.c
index f1b3169cdcc..d577a360153 100644
--- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
@@ -173,7 +173,7 @@ upload_sf_state(struct brw_context *brw)
        unreachable("not reached");
    }
 
-   dw2 = 0;
+   dw2 = GEN6_SF_SCISSOR_ENABLE;
 
    if (ctx->Polygon.CullFlag) {
       switch (ctx->Polygon.CullFaceMode) {
@@ -193,13 +193,6 @@ upload_sf_state(struct brw_context *brw)
       dw2 |= GEN6_SF_CULL_NONE;
    }
 
-   /* _NEW_SCISSOR | _NEW_POLYGON,
-    * BRW_NEW_GS_PROG_DATA | BRW_NEW_PRIMITIVE | BRW_NEW_TES_PROG_DATA
-    */
-   if (ctx->Scissor.EnableFlags ||
-       brw_is_drawing_points(brw) || brw_is_drawing_lines(brw))
-      dw2 |= GEN6_SF_SCISSOR_ENABLE;
-
    /* _NEW_LINE */
    {
       uint32_t line_width_u3_7 = brw_get_line_width(brw);
@@ -260,8 +253,7 @@ const struct brw_tracked_state gen7_sf_state = {
                _NEW_MULTISAMPLE |
                _NEW_POINT |
                _NEW_POLYGON |
-               _NEW_PROGRAM |
-               _NEW_SCISSOR,
+               _NEW_PROGRAM,
       .brw   = BRW_NEW_BLORP |
                BRW_NEW_CONTEXT |
                BRW_NEW_GS_PROG_DATA |
-- 
2.11.0