[Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug for thinnest width lines

[Ville Syrjälä]

On Mon, Oct 05, 2015 at 07:55:24PM +0300, Marius Predut wrote:
> On PNV platform, for 1 pixel line thickness or less,
> the general anti-aliasing algorithm gives up, and a garbage line is generated.
> Setting a Line Width of 0.0 specifies the rasterization
> of the "thinnest" (one-pixel-wide), non-antialiased lines.
> Lines rendered with zero Line Width are rasterized using
> Grid Intersection Quantization rules as specified by
> 2.8.4.1 Zero-Width (Cosmetic) Line Rasterization from volume 1f of the GEN3
> docs.
> The patch was tested on Intel Atom CPU N455.
> 
> This patch follow the same rules as patches fixing the
> https://bugs.freedesktop.org/show_bug.cgi?id=28832
> bug.
> 
> v1: Eduardo Lima Mitev:  Wrong indentation inside the if clause.
> v2: Ian Romanick: comments fix.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90367
> 
> Signed-off-by: Marius Predut <marius.predut at intel.com>
> ---
>  src/mesa/drivers/dri/i915/i915_state.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i915/i915_state.c b/src/mesa/drivers/dri/i915/i915_state.c
> index 4c83073..897eb59 100644
> --- a/src/mesa/drivers/dri/i915/i915_state.c
> +++ b/src/mesa/drivers/dri/i915/i915_state.c
> @@ -599,6 +599,21 @@ i915LineWidth(struct gl_context * ctx, GLfloat widthf)
>     
>     width = (int) (widthf * 2);
>     width = CLAMP(width, 1, 0xf);
> +
> +   if (ctx->Line.Width < 1.5 || widthf < 1.5) {
> +     /* For 1 pixel line thickness or less, the general
> +      * anti-aliasing algorithm gives up, and a garbage line is
> +      * generated.  Setting a Line Width of 0.0 specifies the
> +      * rasterization of the "thinnest" (one-pixel-wide),
> +      * non-antialiased lines.
> +      *
> +      * Lines rendered with zero Line Width are rasterized using
> +      * Grid Intersection Quantization rules as specified by
> +      * volume 1f of the GEN3 docs,
> +      * 2.8.4.1 Zero-Width (Cosmetic) Line Rasterization. 
> +      */
> +      width = 0;
> +   }

I went to do some spec reading, and while I can't confirm the AA <= 1.0
problem (no mention in the spec about such things), I can see this fix
alone isn't sufficient to satisfy the spec (we lack the round to nearest
integer for non-aa for instance).

I think what we'd want is a small helper. i965 has one, although
that one looks quite messy. I think this is how I'd write the helper for
i915:

unsigned intel_line_width(ctx)
{
	float line_width = ctx->Line.Width;

	if (ctx->Line.SmoothFlag)
		line_width = CLAMP(line_width, MinAA, MaxAA);
	else
		line_width = CLAMP(roundf(line_width), Min, Max);

	/*
	 * blah
	 */
	if (line_width < 1.5f)
		line_width = 0.0f

	return U_FIXED(line_width, 1);
}

and then use it for both gen2 and gen3 state setup.

The clamp part could even ve moved to some central place so that all
drivers could share it, or I suppose we could stash the appropriately
rounded and clamped line width into the context as ctx->Line._Width.

Oh and BTW, the gen4/5 line width handling in i965 looks busted too
(only gen6+ got fixed).

>     lis4 |= width << S4_LINE_WIDTH_SHIFT;
>  
>     if (lis4 != i915->state.Ctx[I915_CTXREG_LIS4]) {
> -- 
> 1.9.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Ville Syrjälä
Intel OTC