Re: [Mesa-dev] [PATCH v4] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN7
-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
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.
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?
+ /* 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.
+ * 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
Re: [Mesa-dev] [PATCH v4] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN7
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
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) {
+ /* For lines less than 1 pixel thick, 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 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] [PATCH v4] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN7
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) {
+ /* For lines less than 1 pixel thick, 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 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
