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

[Chris Wilson]

On Thu, Jan 12, 2017 at 12:19:41AM -0800, Kenneth Graunke wrote:
> 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) {

Would not

	if (brw->gen < 8 &&
	    !(brw_is_drawing_points(brw) || brw_is_drawing_lines(brw)))

by itself fix the misrendering as a first step without any concern for
wider impact?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre