[Piglit] [Mesa-dev] [PATCH] ext_framebuffer_multisample: check for num_samples<=1

[Paul Berry]

On 28 January 2013 09:00, Brian Paul <brian.e.paul at gmail.com> wrote:

> On Mon, Jan 28, 2013 at 9:17 AM, Marek Olšák <maraeo at gmail.com> wrote:
> > On Mon, Jan 28, 2013 at 4:54 PM, Brian Paul <brianp at vmware.com> wrote:
> >> [cross-posting this reply to the mesa-dev list]
> >>
> >>
> >> On 01/27/2013 09:07 AM, Brian Paul wrote:
> >>>
> >>> On Sun, Jan 27, 2013 at 2:51 AM, Eric Anholt<eric at anholt.net>  wrote:
> >>>>
> >>>> Brian Paul<brianp at vmware.com>  writes:
> >>>>
> >>>>> Even if GL_ARB_multisample is advertised, it's OK for the max number
> of
> >>>>> samples to be zero.
> >>>>
> >>>>
> >>>> Huh?  The EXT_framebuffer_multisample spec says minmax of MAX_SAMPLES
> is
> >>>> 1.
> >>>
> >>>
> >>> And GL_ARB_framebuffer_object says it's zero.
> >>>
> >>> It looks like the piglit test is checking the wrong extension.  At
> >>> line 364 we're calling
> >>> piglit_require_extension("GL_ARB_framebuffer_object").  I don't see
> >>> any checks for GL_EXT_framebuffer_multisample.  I was looking at
> >>> GL_MAX_SAMPLES in the former, not the later.
> >>>
> >>> I guess the proper fix is to just check for the proper extension.
> >>
> >>
> >>
> >> There's several things going on here.
> >>
> >> First, the piglit test needs a
> >> piglit_require_extension("GL_EXT_framebuffer_multisample") check.
> >>
> >> Second, I think we need to disable advertising this extension for
> swrast,
> >> softpipe and llvmpipe.
> >>
> >> Third, I think we need to decide what GL_MAX_SAMPLES=1 really means.
> >>
> >> I've been reading the various multisample-related extensions and none of
> >> them really says anything special about GL_MAX_SAMPLES=1.  But in some
> >> places (like the GLSL spec) you'll see the phrase "single-sampling" but
> it
> >> seems to mean the non-multisample / conventional rasterization case.
> >>
> >> My thinking is that it would be theoretically possible to have
> single-sample
> >> multisampling (perhaps with a different sample position per pixel).
> >>
> >> In any case, regardless of the number of samples per pixel, we still
> must
> >> obey centroid sampling mode (must interpolate at a point that lies in
> both
> >> the pixel and in the primitive being
> >> rendered).  swrast, softpipe and llvmpipe don't support that so we
> really
> >> shouldn't advertise GL_MAX_SAMPLES=1 (it should be zero) so
> >> GL_EXT_framebuffer_multisample shouldn't be advertised for those
> drivers.
> >>
> >> Most likely, the user's going to be disappointed if they want
> multisampling
> >> in their app and the driver only gives him GL_MAX_SAMPLES=1.
> >>
> >> Does this sound reasonable?
> >
> > I think that not advertising ARB_framebufer_object (which adds
> > GL_MAX_SAMPLES) is a much worse solution than returning GL_MAX_SAMPLES
> > <= 1.
>
> Huh?  I wasn't suggesting disabling GL_ARB_framebuffer_object.  I'm
> saying advertise GL_MAX_SAMPLES=0 and disable
> GL_EXT_framebuffer_multisample in the software drivers.
>

This seems reasonable to me.  We have a similar situation in the i965
driver, since we only support multisampling on Gen6 and above, and what we
currently do in that situation is kind of hokey (we advertise
GL_EXT_framebuffer_multisample and set GL_MAX_SAMPLES=1, but then when the
app calls glRenderbufferStorage(samples=1), we turn around and give them a
single-sampled buffer).  I much prefer Brian's solution.

I'll make a patch that modifies the i965 driver to do this too.

Paul
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130128/7417b4b6/attachment.html>