[Mesa-dev] [PATCH 1/6] i965: Make Gen4-5 SF_STATE use the point size calculations from Gen6+.

[Andres Gomez]

It looks like we could want this whole series into -stable (?)

On Wed, 2017-05-10 at 11:47 -0700, Kenneth Graunke wrote:
> Apparently, Nanhai made the Gen4-5 point size calculations round to the
> nearest integer in commit 8d5231a3582e4f2769ac0685cf0174e09750700e,
> "according to spec".  When Eric first ported the driver to Sandybridge,
> he did not implement this rounding.
> 
> In the GL 2.1 and 3.0 specs "Basic Point Rasterization" section, it does
> say "If antialiasing and point sprites are disabled, the actual width is
> determined by rounding the supplied width to the nearest integer, then
> clamping it to the implementation-dependent maximum non-antialised point
> width."
> 
> In contrast, GL 3.1 and later do not appear to contain this rounding.
> 
> It might be reasonable to round, given that we only implement GL 2.1.
> Of course, if we were to do that, we should actually implement the AA
> vs. non-AA distinction.  Brian added an XXX comment reminding us to fix
> this 10 years ago, but it never happened.
> 
> I think a better plan is to follow the newer, unrounded behavior.  This
> is what we do on Gen6+ and it passes all the relevant conformance tests.
> ---
>  src/mesa/drivers/dri/i965/brw_sf_state.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_sf_state.c b/src/mesa/drivers/dri/i965/brw_sf_state.c
> index d50ceb12133..ff6b5ebf79b 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf_state.c
> @@ -254,14 +254,15 @@ static void upload_sf_unit( struct brw_context *brw )
>         */
>        sf->sf6.point_rast_rule = BRW_RASTRULE_LOWER_RIGHT;
>     }
> -   /* XXX clamp max depends on AA vs. non-AA */
>  
>     /* _NEW_POINT */
>     sf->sf7.sprite_point = ctx->Point.PointSprite;
> -   sf->sf7.point_size = CLAMP(rintf(CLAMP(ctx->Point.Size,
> -                                          ctx->Point.MinSize,
> -                                          ctx->Point.MaxSize)), 1.0f, 255.0f) *
> -                        (1<<3);
> +
> +   float point_sz;
> +   point_sz = CLAMP(ctx->Point.Size, ctx->Point.MinSize, ctx->Point.MaxSize);
> +   point_sz = CLAMP(point_sz, 0.125f, 255.875f);
> +   sf->sf7.point_size = U_FIXED(point_sz, 3);
> +
>     /* _NEW_PROGRAM | _NEW_POINT */
>     sf->sf7.use_point_size_state = !(ctx->VertexProgram.PointSizeEnabled ||
>  				    ctx->Point._Attenuated);
-- 
Br,

Andres