[Mesa-dev] [PATCH] i965: fixed clamping in set_scissor_bits when the y is flipped

[Nanley Chery]

On Mon, Dec 10, 2018 at 12:42:40PM +0200, Eleni Maria Stea wrote:
> Calculating the scissor rectangle fields with the y flipped (0 on top)
> can generate negative values that will cause assertion failure later on
> as the scissor fields are all unsigned. We must clamp the bbox values
> again to make sure they don't exceed the fb_height. Also fixed a
> calculation error.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108999

Good find. Could you send the test to the piglit list?

> ---
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index 8e3fcbf12e..5d8fc8214e 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -2424,8 +2424,21 @@ set_scissor_bits(const struct gl_context *ctx, int i,
>        /* memory: Y=0=top */
>        sc->ScissorRectangleXMin = bbox[0];
>        sc->ScissorRectangleXMax = bbox[1] - 1;
> +
> +      /* Clamping to fb_height is necessary because otherwise the
> +       * subtractions below would produce a negative result, which would
> +       * then be assigned to the unsigned YMin/YMax scissor fields,
> +       * resulting in an assertion failure in GENX(SCISSOR_RECT_pack)
> +       */
> +
> +      if (bbox[3] > fb_height)
> +         bbox[3] = fb_height;
> +
> +      if (bbox[2] > fb_height)
> +         bbox[2] = fb_height;
> +

We should be able to fix this bug in a simpler manner by changing the
MAX2 calls at the top of this function to CLAMP calls.

>        sc->ScissorRectangleYMin = fb_height - bbox[3];
> -      sc->ScissorRectangleYMax = fb_height - bbox[2] - 1;
> +      sc->ScissorRectangleYMax = fb_height - (bbox[2] - 1);

I don't think we want to start adding 1 instead of subtracting 1. The
subtraction is there to satisfy the requirement for the HW packet.

-Nanley

>     }
>  }
>  
> -- 
> 2.20.0.rc2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev