[Mesa-dev] [PATCH 1/4] i965/guardband: Improve comments for guardband clipping

[Kenneth Graunke]

On Monday, August 04, 2014 12:24:01 PM Ben Widawsky wrote:
> While working in this part of the code I had a great deal of trouble
> understanding what it was trying to do, and matching it with the spec.
> (mostly due bad wording in the PRM). To help future people, I've cleaned
> up the wording and provided some ascii art.
> ---
>  src/mesa/drivers/dri/i965/gen8_viewport_state.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen8_viewport_state.c b/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> index b366246..b5171e0 100644
> --- a/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> @@ -71,10 +71,24 @@ gen8_upload_sf_clip_viewport(struct brw_context *brw)
>         * maximum screen space coordinates of a small object may larger, but we
>         * have no way to enforce the object size other than through clipping.
>         *
> -       * If you're surprised that we set clip to -gbx to +gbx and it seems like
> -       * we'll end up with 16384 wide, note that for a 8192-wide render target,
> -       * we'll end up with a normal (-1, 1) clip volume that just covers the
> -       * drawable.
> +       * The goal is to create the maximum sized guardband (8K x 8K) with the
> +       * viewport rectangle in the center of the guardband. This looks weird
> +       * because the hardware wants coordinates that are scaled to the viewport
> +       * in NDC. In other words, an 8K x 8K viewport would have [-1,1] for X and Y.
> +       * A 4K viewport would be [-2,2], 2K := [-4,4] etc.
> +       *
> +       * --------------------------------
> +       * |Guardband                     |
> +       * |                              |
> +       * |         ------------         |
> +       * |         |viewport  |         |
> +       * |         |          |         |
> +       * |         |          |         |
> +       * |         |__________|         |
> +       * |                              |
> +       * |                              |
> +       * |______________________________|
> +       *
>         */
>        const float maximum_guardband_extent = 8192;
>        float gbx = maximum_guardband_extent / ctx->ViewportArray[i].Width;
> 

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

and you can probably add a:
Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>
or at least Acked-by

based on #intel-gfx chatter:

[Friday, August 01, 2014] [07:24:53 PM] <bwidawsk>      Kayden, chrisf: useful p
atch? http://sprunge.us/SPeA
[Friday, August 01, 2014] [07:25:48 PM] <Kayden>        looks reasonable to me
[Friday, August 01, 2014] [07:26:48 PM] <chrisf>        same here
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140809/7ac88d20/attachment.sig>