i965/guardband: Enable for all viewport dimensions (GEN8+)

The goal of guardband clipping is to try to avoid 3d clipping because it
is an expensive operation. When guardband clipping is disabled, all
geometry that intersects the viewport is sent to the FF 3d clipper.
Objects which are entirely enclosed within the viewport are said to be
"trivially accepted" while those entirely outside of the viewport are,
"trivially rejected".

When guardband clipping is turned on the above behavior is changed such
that if the geometry is within the guardband, and intersects the
viewport, it skips the 3d clipper. Prior to GEN8, this was problematic
if the viewport was smaller than the screen as it could allow for
rendering to occur outside of the viewport. That could be mitigated if
the programmer specified a scissor region which was less than or equal
to the viewport - but this is not required for correctness in OpenGL. In
theory you could be clever with the guardband so as not to invoke this
problem. We do not do this, and have no data that suggests we should
bother (nor the converse data).

With viewport extents in place on GEN8, it should be safe to turn on
guardband clipping for all cases

While here, add a comment to the code which confused me thoroughly.

v2: Update grammar in commit message. Reword comments based on Ken's
suggestion.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

diff --git a/src/mesa/drivers/dri/i965/gen6_clip_state.c b/src/mesa/drivers/dri/i965/gen6_clip_state.c
index 52027e0e18a88282b1314e9b21c6f085c215ad7f..a8ed1212bf5e32cc018e96d2d6dce5b149912427 100644 (file)
--- a/src/mesa/drivers/dri/i965/gen6_clip_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_clip_state.c
@@ -97,17 +97,27 @@ upload_clip_state(struct brw_context *brw)
            GEN6_USER_CLIP_CLIP_DISTANCES_SHIFT);
 
    dw2 |= GEN6_CLIP_GB_TEST;
-   for (unsigned i = 0; i < ctx->Const.MaxViewports; i++) {
-      if (ctx->ViewportArray[i].X != 0 ||
-          ctx->ViewportArray[i].Y != 0 ||
-          ctx->ViewportArray[i].Width != (float) fb->Width ||
-          ctx->ViewportArray[i].Height != (float) fb->Height) {
-         dw2 &= ~GEN6_CLIP_GB_TEST;
-         if (brw->gen >= 8) {
-            perf_debug("Disabling GB clipping due to lack of Gen8 viewport "
-                       "clipping setup code.  This should be fixed.\n");
+
+   /* 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) {
+      for (unsigned i = 0; i < ctx->Const.MaxViewports; i++) {
+         if (ctx->ViewportArray[i].X != 0 ||
+             ctx->ViewportArray[i].Y != 0 ||
+             ctx->ViewportArray[i].Width != (float) fb->Width ||
+             ctx->ViewportArray[i].Height != (float) fb->Height) {
+            dw2 &= ~GEN6_CLIP_GB_TEST;
+            break;
          }
-         break;
       }
    }