[Mesa-dev,09/11] i965: Fix non-AA wide line rendering with fractional line widths

Submitted by Eduardo Lima Mitev on Feb. 10, 2015, 3:40 p.m.

Details

Message ID 1423582848-18526-10-git-send-email-elima@igalia.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Eduardo Lima Mitev Feb. 10, 2015, 3:40 p.m.
From: Iago Toral Quiroga <itoral@igalia.com>

From 14.5.2.2. Wide Lines  of the OpenGL spec 4.5:

"(...)Let w be the width rounded to the nearest integer (...). If the
line segment has endpoints given by (x0,y0) and (x1,y1) in window
coordinates, the segment with endpoints (x0,y0-(w-1)/2) and
(x1,y1-(w-1/2)) is rasterized, (...)"

The hardware it not rounding the line width, so we should do it.

Also, we should be careful not to go beyond the hardware limits
for the line width after it gets rounded. Gen6-7 define a maximum line
width slightly below 8.0, so we should advertise a maximum line
width lower than 7.5 to make sure that 7.0 is the maximum integer
line width that we can select. Since the line width granularity in these
platforms is 0.125, we choose 7.375. Other platforms advertise rounded
maximum line widths, so those are fine.

Fixes the following 3 dEQP tests:
dEQP-GLES3.functional.rasterization.primitives.lines_wide
dEQP-GLES3.functional.rasterization.fbo.texture_2d.primitives.lines_wide
dEQP-GLES3.functional.rasterization.fbo.rbo_singlesample.primitives.lines_wide
---
 src/mesa/drivers/dri/i965/brw_context.c   | 4 ++--
 src/mesa/drivers/dri/i965/gen6_sf_state.c | 8 ++++++--
 src/mesa/drivers/dri/i965/gen7_sf_state.c | 8 ++++++--
 src/mesa/drivers/dri/i965/gen8_sf_state.c | 8 ++++++--
 4 files changed, 20 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index f5b0624..65b489e 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -428,8 +428,8 @@  brw_initialize_context_constants(struct brw_context *brw)
       ctx->Const.MaxLineWidthAA = 40.0;
       ctx->Const.LineWidthGranularity = 0.125;
    } else if (brw->gen >= 6) {
-      ctx->Const.MaxLineWidth = 7.875;
-      ctx->Const.MaxLineWidthAA = 7.875;
+      ctx->Const.MaxLineWidth = 7.375;
+      ctx->Const.MaxLineWidthAA = 7.375;
       ctx->Const.LineWidthGranularity = 0.125;
    } else {
       ctx->Const.MaxLineWidth = 7.0;
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 7f0bab8..4d7714e 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -361,8 +361,12 @@  upload_sf_state(struct brw_context *brw)
 
    /* _NEW_LINE */
    {
-      uint32_t line_width_u3_7 =
-         U_FIXED(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth), 7);
+      /* OpenGL dictates that line width should be rounded to the nearest
+       * integer
+       */
+      float line_width =
+         (int) CLAMP(ctx->Line.Width + 0.5, 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;
diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c b/src/mesa/drivers/dri/i965/gen7_sf_state.c
index 6644010..3ecac96 100644
--- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
@@ -192,8 +192,12 @@  upload_sf_state(struct brw_context *brw)
 
    /* _NEW_LINE */
    {
-      uint32_t line_width_u3_7 =
-         U_FIXED(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth), 7);
+      /* OpenGL dictates that line width should be rounded to the nearest
+       * integer
+       */
+      float line_width =
+         (int) CLAMP(ctx->Line.Width + 0.5, 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;
diff --git a/src/mesa/drivers/dri/i965/gen8_sf_state.c b/src/mesa/drivers/dri/i965/gen8_sf_state.c
index 713ee5f..94eac47 100644
--- a/src/mesa/drivers/dri/i965/gen8_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_sf_state.c
@@ -154,8 +154,12 @@  upload_sf(struct brw_context *brw)
        dw1 |= GEN6_SF_VIEWPORT_TRANSFORM_ENABLE;
 
    /* _NEW_LINE */
-   uint32_t line_width_u3_7 =
-      U_FIXED(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth), 7);
+   /* OpenGL dictates that line width should be rounded to the nearest
+    * integer
+    */
+   float line_width =
+      (int) CLAMP(ctx->Line.Width + 0.5, 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;
    if (brw->gen >= 9 || brw->is_cherryview) {

Comments

On Tuesday, February 10, 2015 04:40:46 PM Eduardo Lima Mitev wrote:
> From: Iago Toral Quiroga <itoral@igalia.com>
> 
> From 14.5.2.2. Wide Lines  of the OpenGL spec 4.5:
> 
> "(...)Let w be the width rounded to the nearest integer (...). If the
> line segment has endpoints given by (x0,y0) and (x1,y1) in window
> coordinates, the segment with endpoints (x0,y0-(w-1)/2) and
> (x1,y1-(w-1/2)) is rasterized, (...)"
> 
> The hardware it not rounding the line width, so we should do it.
> 
> Also, we should be careful not to go beyond the hardware limits
> for the line width after it gets rounded. Gen6-7 define a maximum line
> width slightly below 8.0, so we should advertise a maximum line
> width lower than 7.5 to make sure that 7.0 is the maximum integer
> line width that we can select. Since the line width granularity in these
> platforms is 0.125, we choose 7.375. Other platforms advertise rounded
> maximum line widths, so those are fine.
> 
> Fixes the following 3 dEQP tests:
> dEQP-GLES3.functional.rasterization.primitives.lines_wide
> dEQP-GLES3.functional.rasterization.fbo.texture_2d.primitives.lines_wide
> dEQP-GLES3.functional.rasterization.fbo.rbo_singlesample.primitives.lines_wide
> ---
>  src/mesa/drivers/dri/i965/brw_context.c   | 4 ++--
>  src/mesa/drivers/dri/i965/gen6_sf_state.c | 8 ++++++--
>  src/mesa/drivers/dri/i965/gen7_sf_state.c | 8 ++++++--
>  src/mesa/drivers/dri/i965/gen8_sf_state.c | 8 ++++++--
>  4 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index f5b0624..65b489e 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -428,8 +428,8 @@ brw_initialize_context_constants(struct brw_context *brw)
>        ctx->Const.MaxLineWidthAA = 40.0;
>        ctx->Const.LineWidthGranularity = 0.125;
>     } else if (brw->gen >= 6) {
> -      ctx->Const.MaxLineWidth = 7.875;
> -      ctx->Const.MaxLineWidthAA = 7.875;
> +      ctx->Const.MaxLineWidth = 7.375;
> +      ctx->Const.MaxLineWidthAA = 7.375;
>        ctx->Const.LineWidthGranularity = 0.125;
>     } else {
>        ctx->Const.MaxLineWidth = 7.0;
> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> index 7f0bab8..4d7714e 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> @@ -361,8 +361,12 @@ upload_sf_state(struct brw_context *brw)
>  
>     /* _NEW_LINE */
>     {
> -      uint32_t line_width_u3_7 =
> -         U_FIXED(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth), 7);
> +      /* OpenGL dictates that line width should be rounded to the nearest
> +       * integer
> +       */
> +      float line_width =
> +         (int) CLAMP(ctx->Line.Width + 0.5, 0.0, ctx->Const.MaxLineWidth);

Nice catch!  It hadn't even occurred to me that GL would require the
line width to be rounded.

I think this would be a bit clearer as:

float line_width =
   roundf(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth));

With that changed in all three places, this is:
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>