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

Jason Ekstrand jason at jlekstrand.net
Tue Jan 31 18:45:02 UTC 2017


On Sun, Jan 22, 2017 at 10:42 PM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> 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;
>

We could, in theory, only scissor if one of our viewports is actually
smaller than the framebuffer.  Then again, I agree that scissoring should
be cheap so why not.

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


>
>     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
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170131/3e9a0571/attachment.html>


More information about the mesa-dev mailing list