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

[Kenneth Graunke]

Previously we disabled the guardband when the viewport was smaller than
the framebuffer on Gen6-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.)

Given that the viewport extents test is essentially a second scissor,
and is enabled for basically all 3D drawing on Gen8+, it stands to
reason that scissoring is cheap.  Enabling the guardband reduces the
cost of clipping, which is expensive.

The Windows driver appears to never disable guardband clipping, and
appears to use scissoring in this case.  I don't know if they leave
it on universally though.

This 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(-)

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