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

2015-10-21 Thread Ville Syrjälä 
On Tue, Oct 20, 2015 at 02:02:21PM +0300, Ville Syrjälä wrote:
> On Tue, Oct 20, 2015 at 08:15:32AM +, Predut, Marius wrote:
> > > -Original Message-
> > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > > Sent: Monday, October 19, 2015 6:04 PM
> > > To: Predut, Marius
> > > Cc: mesa-dev@lists.freedesktop.org; Iago Toral Quiroga
> > > Subject: Re: [Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug for 
> > > thinnest
> > > width lines
> > > 
> > > On Thu, Oct 15, 2015 at 06:03:34PM +0300, Ville Syrjälä wrote:
> > > > On Thu, Oct 15, 2015 at 02:19:09PM +, Predut, Marius wrote:
> > > > > > -Original Message-
> > > > > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > > > > > Sent: Wednesday, October 07, 2015 1:53 PM
> > > > > > To: Predut, Marius
> > > > > > Cc: mesa-dev@lists.freedesktop.org
> > > > > > Subject: Re: [Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug
> > > > > > for thinnest width lines
> > > > > >
> > > > > > 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.pre...@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;
> > > > &

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

2015-10-20 Thread Ville Syrjälä 
On Tue, Oct 20, 2015 at 08:15:32AM +, Predut, Marius wrote:
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > Sent: Monday, October 19, 2015 6:04 PM
> > To: Predut, Marius
> > Cc: mesa-dev@lists.freedesktop.org; Iago Toral Quiroga
> > Subject: Re: [Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug for 
> > thinnest
> > width lines
> > 
> > On Thu, Oct 15, 2015 at 06:03:34PM +0300, Ville Syrjälä wrote:
> > > On Thu, Oct 15, 2015 at 02:19:09PM +, Predut, Marius wrote:
> > > > > -Original Message-
> > > > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > > > > Sent: Wednesday, October 07, 2015 1:53 PM
> > > > > To: Predut, Marius
> > > > > Cc: mesa-dev@lists.freedesktop.org
> > > > > Subject: Re: [Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug
> > > > > for thinnest width lines
> > > > >
> > > > > 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.pre...@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).
> > > >
> > &

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

2015-10-20 Thread Predut, Marius 
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Monday, October 19, 2015 6:04 PM
> To: Predut, Marius
> Cc: mesa-dev@lists.freedesktop.org; Iago Toral Quiroga
> Subject: Re: [Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug for thinnest
> width lines
> 
> On Thu, Oct 15, 2015 at 06:03:34PM +0300, Ville Syrjälä wrote:
> > On Thu, Oct 15, 2015 at 02:19:09PM +, Predut, Marius wrote:
> > > > -Original Message-
> > > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > > > Sent: Wednesday, October 07, 2015 1:53 PM
> > > > To: Predut, Marius
> > > > Cc: mesa-dev@lists.freedesktop.org
> > > > Subject: Re: [Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug
> > > > for thinnest width lines
> > > >
> > > > 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.pre...@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).
> > >
> > > Ville ,Thanks for review!
> > > On this seem not too much docs, here can use experiments or docs for next
> GEN+.
> > >
> > > >
> > > > 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)
>

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

2015-10-19 Thread Ville Syrjälä 
On Thu, Oct 15, 2015 at 06:03:34PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 15, 2015 at 02:19:09PM +, Predut, Marius wrote:
> > > -Original Message-
> > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > > Sent: Wednesday, October 07, 2015 1:53 PM
> > > To: Predut, Marius
> > > Cc: mesa-dev@lists.freedesktop.org
> > > Subject: Re: [Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug for 
> > > thinnest
> > > width lines
> > > 
> > > 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.pre...@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).
> > 
> > Ville ,Thanks for review!
> > On this seem not too much docs, here can use experiments or docs for next 
> > GEN+.
> > 
> > > 
> > > 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.
> > 
> > Do you used this and it works for you? (I mean if you did a

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

2015-10-15 Thread Predut, Marius 
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Wednesday, October 07, 2015 1:53 PM
> To: Predut, Marius
> Cc: mesa-dev@lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug for thinnest
> width lines
> 
> 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.pre...@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).

Ville ,Thanks for review!
On this seem not too much docs, here can use experiments or docs for next GEN+.

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

Do you used this and it works for you? (I mean if you did a test on your PNV 
platform)
I have some comments on the Bugzilla related to SmoothFlag flag.(on 2015-06-04).
On my tests seems the flag is set only if call glLineWidth (lineWidth), 
lineWidth != 1.

> 
> 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).

First I intend only to fix de bug , then add extra fixes like CLAMP.
CLAMP was not done before and it can be subject on next patch series.


> 
> > lis4 |= width << S4_LINE_WIDTH_SHIFT;
> >
> > if (lis4 != i915->state.Ctx[I915_CTXREG_LIS4]) {
> > --
> > 1.9.1
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> --
> Ville Syrjälä
> Intel OTC
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-10-15 Thread Ville Syrjälä 
On Thu, Oct 15, 2015 at 02:19:09PM +, Predut, Marius wrote:
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > Sent: Wednesday, October 07, 2015 1:53 PM
> > To: Predut, Marius
> > Cc: mesa-dev@lists.freedesktop.org
> > Subject: Re: [Mesa-dev] [PATCH] i915/aa: fixing anti-aliasing bug for 
> > thinnest
> > width lines
> > 
> > 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.pre...@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).
> 
> Ville ,Thanks for review!
> On this seem not too much docs, here can use experiments or docs for next 
> GEN+.
> 
> > 
> > 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.
> 
> Do you used this and it works for you? (I mean if you did a test on your PNV 
> platform)

Didn't do any actual testing yet. I've been meaning to, but just been
too busy with other stuff. I can try to test on pnv today, and maybe on
830 and 85x on the weekend. Hmm, I wonder if the test even works on
gl1?

> I have some comments on the Bugzilla related to SmoothFlag flag.(on 
> 2015-06-04).
> On my tests seems the flag is set only if call glLineWidth (lineWidth), 
> lineWidth != 1.
> 
> > 
> > The clamp part could even ve moved to some central place so that all drivers

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

2015-10-07 Thread 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 
> ---
>  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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Ville Syrjälä
Intel OTC
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-10-05 Thread Marius Predut 
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 
---
 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;
+   }
lis4 |= width << S4_LINE_WIDTH_SHIFT;
 
if (lis4 != i915->state.Ctx[I915_CTXREG_LIS4]) {
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-08-27 Thread Eduardo Lima Mitev 
On 07/30/2015 05:05 PM, Predut, Marius wrote:
 Was sent 2 month ago, still not review/upstream
 
 -Original Message-
 From: Predut, Marius 
 Sent: Thursday, July 30, 2015 7:04 PM
 To: mesa-dev@lists.freedesktop.org
 Cc: Predut, Marius
 Subject: [Mesa-dev][PATCH] i915/aa: fixing anti-aliasing bug for thinnest 
 width lines
 
 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 bspec G45: Volume 2: 3D/Media,
 7.3.13.1 Zero-Width (Cosmetic) Line Rasterization section.
 
 This patch follow the same rules as patches fixing the
 https://bugs.freedesktop.org/show_bug.cgi?id=28832
 bug.
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90367
 
 Signed-off-by: Marius Predut marius.pre...@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 5f10b84..6cd342c 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
 + * bspec G45: Volume 2: 3D/Media,
 + * 7.3.13.1 Zero-Width (Cosmetic) Line Rasterization section
 + */
 +width = 0;
 +   }
 lis4 |= width  S4_LINE_WIDTH_SHIFT;
  

Wrong indentation inside the if clause. Please be consistent with the
rest of the file.

The patch looks fine, but I have been unable to confirm that it fixes
the related piglit test because I don't have access to an i915 device.

 if (lis4 != i915-state.Ctx[I915_CTXREG_LIS4]) {
 --
 1.9.1
 
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev
 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-07-30 Thread Marius Predut 
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
bspec G45: Volume 2: 3D/Media,
7.3.13.1 Zero-Width (Cosmetic) Line Rasterization section.

This patch follow the same rules as patches fixing the
https://bugs.freedesktop.org/show_bug.cgi?id=28832
bug.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90367

Signed-off-by: Marius Predut marius.pre...@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 5f10b84..6cd342c 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
+ * bspec G45: Volume 2: 3D/Media,
+ * 7.3.13.1 Zero-Width (Cosmetic) Line Rasterization section
+ */
+width = 0;
+   }
lis4 |= width  S4_LINE_WIDTH_SHIFT;
 
if (lis4 != i915-state.Ctx[I915_CTXREG_LIS4]) {
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-07-30 Thread Predut, Marius 
Was sent 2 month ago, still not review/upstream

-Original Message-
From: Predut, Marius 
Sent: Thursday, July 30, 2015 7:04 PM
To: mesa-dev@lists.freedesktop.org
Cc: Predut, Marius
Subject: [Mesa-dev][PATCH] i915/aa: fixing anti-aliasing bug for thinnest width 
lines

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 bspec G45: Volume 2: 3D/Media,
7.3.13.1 Zero-Width (Cosmetic) Line Rasterization section.

This patch follow the same rules as patches fixing the
https://bugs.freedesktop.org/show_bug.cgi?id=28832
bug.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90367

Signed-off-by: Marius Predut marius.pre...@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 5f10b84..6cd342c 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
+ * bspec G45: Volume 2: 3D/Media,
+ * 7.3.13.1 Zero-Width (Cosmetic) Line Rasterization section
+ */
+width = 0;
+   }
lis4 |= width  S4_LINE_WIDTH_SHIFT;
 
if (lis4 != i915-state.Ctx[I915_CTXREG_LIS4]) {
--
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev