> -----Original Message----- > From: Matt Turner [mailto:matts...@gmail.com] > Sent: Thursday, April 23, 2015 12:55 AM > To: Predut, Marius > Cc: mesa-dev@lists.freedesktop.org > Subject: Re: [Mesa-dev] [PATCH v4] i965/aa: fixing anti-aliasing bug for > thinnest width lines - GEN7 > > On Thu, Apr 2, 2015 at 4:30 AM, Predut, Marius <marius.pre...@intel.com> > wrote: > > Please "review by" feedback please :-) > > > > -----Original Message----- > > From: Predut, Marius > > Sent: Thursday, March 19, 2015 9:34 PM > > To: mesa-dev@lists.freedesktop.org > > Cc: Predut, Marius > > Subject: [Mesa-dev][PATCH v4] i965/aa: fixing anti-aliasing bug for thinnest > width lines - GEN7 > > I was confused by the fact that there were separate Gen6 and Gen7 > patches. That's fine, though I probably wouldn't bother doing it > myself, but if that's what you want you definitely should send them as > a 2 patch series. > > Having just two nearly identically named patches is prone to confusion. >
I had prepared ,developed and tests the patches on the 2 different machines(SNB and IVB) this is the reason for. Also the patches can be applied independently , only the fix is identical. > > On SNB and IVB hw, for 1 pixel line thickness or less, the general anti- > aliasing algorithm give up - 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 section 6.3.12.1 Zero-Width > (Cosmetic) Line Rasterization. > > > > v2: Daniel Stone: Fix = used instead of == in an if-statement. > > v3: Ian Romanick: Use "._Enabled" flag insteed ".Enabled". > > Add code comments. re-word wrap the commit message. > > Add a complete bugzillia list. > > Improve the hardcoded values to produce better results. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28832 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=9951 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=27007 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=60797 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=15006 > > > > Signed-off-by: Marius Predut <marius.pre...@intel.com> > > --- > > src/mesa/drivers/dri/i965/gen7_sf_state.c | 21 ++++++++++++++++++--- > > 1 file changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c > b/src/mesa/drivers/dri/i965/gen7_sf_state.c > > index c9815b0..e33cb79 100644 > > --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c > > +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c > > @@ -198,9 +198,24 @@ upload_sf_state(struct brw_context *brw) > > float line_width = > > roundf(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth)); > > uint32_t line_width_u3_7 = U_FIXED(line_width, 7); > > - /* TODO: line width of 0 is not allowed when MSAA enabled */ > > - if (line_width_u3_7 == 0) > > - line_width_u3_7 = 1; > > + /* Line width of 0 is not allowed when MSAA enabled */ > > + if (ctx->Multisample._Enabled) { > > + if (line_width_u3_7 == 0) > > + line_width_u3_7 = 1; > > + } else if (ctx->Line.SmoothFlag && ctx->Line.Width <= 1.49) { > > I don't know what 1.49 is. Presumably you're checking for values that > line widths that round-to-nearest to 1.0? If so, shouldn't that be > ctx->Line.Width < 1.5? Yep, I use your version.( I supposed nobody thinking for lines width with the 3 decimals value :-) > > > + /* For lines less than 1 pixel thick, the general > > But then the comment says "less than 1 pixel thick", so I don't > understand -- and it's not helping that I don't really know anything > about this code to begin with. :) > > Presumably the comment in the commit message saying "for 1 pixel line > thickness or less" is more correct. > > For what it's worth, I did confirm that this patch makes the > line-aa-width test in piglit go from fail -> pass on my Haswell. Typo fixed. > > > + * 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 section 6.3.12.1 Zero-Width (Cosmetic) Line > > + * Rasterization. > > + */ > > + line_width_u3_7 = 0; > > + } > > dw2 |= line_width_u3_7 << GEN6_SF_LINE_WIDTH_SHIFT; > > } > > if (ctx->Line.SmoothFlag) { > > -- > > 1.7.9.5 > > > > _______________________________________________ > > 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
