i965: Use a minimum point width of 1.0.

Submitted by Kenneth Graunke on Nov. 17, 2018, 9:40 a.m.

Details

Message ID 20181117094002.11013-1-kenneth@whitecape.org
State New
Headers show
Series "i965: Use a minimum point width of 1.0." ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Kenneth Graunke Nov. 17, 2018, 9:40 a.m.
We advertise 1.0 as the minimum point width size, and we probably ought
to clamp gl_PointSize to the actual range we advertise ([1,255]).  In
particular, we don't seem to rasterize any points if the shader outputs
a point size smaller than 1.0, and that seems rather sketchy.

This fixes Piglit's vs-point_size-zero test, which writes 0 for
gl_PointSize and expects something to be rendered.  However, that test
may be technically incorrect - according to the OpenGL spec, "If the
value written to gl_PointSize is less than or equal to zero, results are
undefined."  Presumably "undefined" includes "don't render any points".

Of course, what the test expects is pretty /reasonable/ behavior, even
if not mandatory, so we may as well do what it wants...
---
 src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c
index 5acd0922922..0a0add97d80 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -1547,7 +1547,7 @@  genX(upload_clip_state)(struct brw_context *brw)
       if (!brw_is_drawing_points(brw) && !brw_is_drawing_lines(brw))
          clip.ViewportXYClipTestEnable = true;
 
-      clip.MinimumPointWidth = 0.125;
+      clip.MinimumPointWidth = 1.0;
       clip.MaximumPointWidth = 255.875;
       clip.MaximumVPIndex = viewport_count - 1;
       if (_mesa_geometric_layers(fb) == 0)

Comments

Do smaller point sizes make sense when multisampling?

On November 17, 2018 03:40:10 Kenneth Graunke <kenneth@whitecape.org> wrote:

> We advertise 1.0 as the minimum point width size, and we probably ought
> to clamp gl_PointSize to the actual range we advertise ([1,255]).  In
> particular, we don't seem to rasterize any points if the shader outputs
> a point size smaller than 1.0, and that seems rather sketchy.
>
> This fixes Piglit's vs-point_size-zero test, which writes 0 for
> gl_PointSize and expects something to be rendered.  However, that test
> may be technically incorrect - according to the OpenGL spec, "If the
> value written to gl_PointSize is less than or equal to zero, results are
> undefined."  Presumably "undefined" includes "don't render any points".
>
> Of course, what the test expects is pretty /reasonable/ behavior, even
> if not mandatory, so we may as well do what it wants...
> ---
> src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c 
> b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index 5acd0922922..0a0add97d80 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -1547,7 +1547,7 @@ genX(upload_clip_state)(struct brw_context *brw)
>       if (!brw_is_drawing_points(brw) && !brw_is_drawing_lines(brw))
>          clip.ViewportXYClipTestEnable = true;
>
> -      clip.MinimumPointWidth = 0.125;
> +      clip.MinimumPointWidth = 1.0;
>       clip.MaximumPointWidth = 255.875;
>       clip.MaximumVPIndex = viewport_count - 1;
>       if (_mesa_geometric_layers(fb) == 0)
> --
> 2.19.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Ah, that's probably where 0.125 comes from - 1/8th for 8x MSAA.
Though we advertise 1.0 as the minimum MS point width, as well.

I suppose we could try and advertise 0.125 as the minimum MSAA point
width in GLES, and use 0.125 for MSAA and 1.0 for single sampled RTs.

Advertising in GL is a bit wonky, it only has "smooth point" limits,
which are actually round points...but it kind of conflates smooth and
multisampled...

On Saturday, November 17, 2018 6:57:04 AM PST Jason Ekstrand wrote:
> Do smaller point sizes make sense when multisampling?
> 
> On November 17, 2018 03:40:10 Kenneth Graunke <kenneth@whitecape.org> wrote:
> 
> > We advertise 1.0 as the minimum point width size, and we probably ought
> > to clamp gl_PointSize to the actual range we advertise ([1,255]).  In
> > particular, we don't seem to rasterize any points if the shader outputs
> > a point size smaller than 1.0, and that seems rather sketchy.
> >
> > This fixes Piglit's vs-point_size-zero test, which writes 0 for
> > gl_PointSize and expects something to be rendered.  However, that test
> > may be technically incorrect - according to the OpenGL spec, "If the
> > value written to gl_PointSize is less than or equal to zero, results are
> > undefined."  Presumably "undefined" includes "don't render any points".
> >
> > Of course, what the test expects is pretty /reasonable/ behavior, even
> > if not mandatory, so we may as well do what it wants...
> > ---
> > src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c 
> > b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > index 5acd0922922..0a0add97d80 100644
> > --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> > +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > @@ -1547,7 +1547,7 @@ genX(upload_clip_state)(struct brw_context *brw)
> >       if (!brw_is_drawing_points(brw) && !brw_is_drawing_lines(brw))
> >          clip.ViewportXYClipTestEnable = true;
> >
> > -      clip.MinimumPointWidth = 0.125;
> > +      clip.MinimumPointWidth = 1.0;
> >       clip.MaximumPointWidth = 255.875;
> >       clip.MaximumVPIndex = viewport_count - 1;
> >       if (_mesa_geometric_layers(fb) == 0)
> > --
> > 2.19.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 
> 
>