[Mesa-dev] [PATCH] i965: do not round line width when multisampling or antialiasing are enabled
Ian Romanick
idr at freedesktop.org
Tue Jun 9 08:27:15 PDT 2015
On 06/08/2015 11:49 PM, Iago Toral Quiroga wrote:
> In commit fe74fee8fa721a we rounded the line width to the nearest integer to
> match the GLES3 spec requirements stated in section 13.4.2.1, but that seems
> to break a dEQP test that renders wide lines in some multisampling scenarios.
>
> Ian noted that the Open 4.4 spec has the following similar text:
>
> "The actual width of non-antialiased lines is determined by rounding the
> supplied width to the nearest integer, then clamping it to the
> implementation-dependent maximum non-antialiased line width."
>
> and suggested that when ES removed antialiased lines, they removed
> "non-antialised" from that paragraph but probably should not have.
>
> Going by that note, this patch restricts the quantization implemented in
> fe74fee8fa721a only to regular aliased lines. This seems to fix the broken
> test without causing regressions in any of the dEQP line rasterization tests
> (dEQP-GLES3.functional.rasterization.*line*).
>
> Fixes:
> dEQP-GLES3.functional.rasterization.fbo.rbo_multisample_max.primitives.lines_wide
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90749
> ---
> src/mesa/drivers/dri/i965/gen6_sf_state.c | 5 +++--
> src/mesa/drivers/dri/i965/gen7_sf_state.c | 6 ++++--
> src/mesa/drivers/dri/i965/gen8_sf_state.c | 5 +++--
We haven't been running dEQP on earlier GENs, but I suspect they'll
have the same issue. Since this same code is duplicated in 3 places
already, perhaps it should be put in a utility function in one of the
header files.
> 3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> index e445ce2..b674268 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> @@ -364,8 +364,9 @@ upload_sf_state(struct brw_context *brw)
> /* OpenGL dictates that line width should be rounded to the nearest
> * integer
> */
With these changes, the above comment is no longer true. You should
add the spec quotation from the commit message here.
> - float line_width =
> - roundf(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth));
> + float line_width = CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth);
> + if (!ctx->Multisample._Enabled && !ctx->Line.SmoothFlag)
> + line_width = CLAMP(roundf(line_width), 0.0, ctx->Const.MaxLineWidth);
The lack of whitespace (blank lines) makes this really hard to read. I
think you can fix my whitespace issues and Ken's too-many-clamps issues
by changing this to:
const float line_width = CLAMP(!ctx->Multisample._Enabled && !ctx->Line.SmoothFlag
? roundf(line_width) : line_width,
0.0, ctx->Const.MaxLineWidth);
But, again, I think putting this line and the next line in a utility
function would be best of all... unless Ken disagrees. ;)
> uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
>
> /* Line width of 0 is not allowed when MSAA enabled */
> diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> index 58e3337..07b4394 100644
> --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> @@ -195,9 +195,11 @@ upload_sf_state(struct brw_context *brw)
> /* OpenGL dictates that line width should be rounded to the nearest
> * integer
> */
> - float line_width =
> - roundf(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth));
> + float line_width = CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth);
> + if (!ctx->Multisample._Enabled && !ctx->Line.SmoothFlag)
> + line_width = CLAMP(roundf(line_width), 0.0, ctx->Const.MaxLineWidth);
> uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
> +
> /* Line width of 0 is not allowed when MSAA enabled */
> if (ctx->Multisample._Enabled) {
> if (line_width_u3_7 == 0)
> diff --git a/src/mesa/drivers/dri/i965/gen8_sf_state.c b/src/mesa/drivers/dri/i965/gen8_sf_state.c
> index 52a21b6..6c765f0 100644
> --- a/src/mesa/drivers/dri/i965/gen8_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_sf_state.c
> @@ -157,8 +157,9 @@ upload_sf(struct brw_context *brw)
> /* OpenGL dictates that line width should be rounded to the nearest
> * integer
> */
> - float line_width =
> - roundf(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth));
> + float line_width = CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth);
> + if (!ctx->Multisample._Enabled && !ctx->Line.SmoothFlag)
> + line_width = CLAMP(roundf(line_width), 0.0, ctx->Const.MaxLineWidth);
> uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
> if (line_width_u3_7 == 0)
> line_width_u3_7 = 1;
>
More information about the mesa-dev
mailing list