From f6725d627cb3945f2577b6ba637ee16830d0b13c Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Wed, 23 Jul 2014 17:55:40 -0700 Subject: 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 --- src/mesa/drivers/dri/i965/gen6_clip_state.c | 30 +++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/mesa/drivers/dri/i965/gen6_clip_state.c b/src/mesa/drivers/dri/i965/gen6_clip_state.c index 52027e0e18a..a8ed1212bf5 100644 --- 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; } } -- cgit v1.2.3