[Mesa-dev] [RFC PATCH] i965/msaa: Disable unsupported formats.

[Paul Berry]

On 15 June 2012 14:44, Chad Versace <chad.versace at linux.intel.com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 06/15/2012 11:12 AM, Paul Berry wrote:
> > Due to hardware limitations, MSAA is unsupported on Gen6 for formats
> > containing >64 bits of data per pixel.  From the Sandy Bridge PRM,
> > vol4 part1, p72 ("Surface Format"):
> >
> >     If Number of Multisamples is set to a value other than
> >     MULTISAMPLECOUNT_1, this field cannot be set to the following
> >     formats:
> >     - any format with greater than 64 bits per element
> >     - any compressed texture format (BC*)
> >     - any YCRCB* format
> >
> > Gen7 has a similar, but less stringent limitation: formats with >64
> > bits of data per pixel only support 4x MSAA.
> >
> > This patch causes the unsupported formats to report
> > GL_FRAMEBUFFER_UNSUPPORTED.
> >
> > Fixes piglit "multisample-formats" tests on Gen6.
> > ---
> >
> > It is not 100% clear to me whether this approach is compliant with the
> > spec.  On the one hand, the spec requires formats RGBA32F, RGBA32I,
> > and RGBA32UI to be color-renderable.  On the other hand, the spec
> > allows for the implementation to report GL_FRAMEBUFFER_UNSUPPORTED if
> > the set of attached images violates "an implementation-dependent set
> > of restrictions", which seems to leave a lot of leeway.
> >
> > If we decide that it's not ok to report GL_FRAMEBUFFER_UNSUPPORTED for
> > MSAA buffers using these formats, then we'll have to figure out some
> > other way to work around the hardware's lack of support for MSAA on
> > these formats, and I haven't thought of a good way to do that.  The
> > alternatives I've considered (substitute another buffer format, or
> > substitute a non-multisampled renderbuffer) are definitely
> > non-spec-compliant, and also fraught with implementation difficulties.
> >
> > So for the moment my inclination is to go through with this patch as
> > is, and if we later discover that it causes trouble for a client
> > application, we can consider other options.  I'm curious to hear what
> > others think.
>
> After releasing GL3.0, but before implementing real MSAA, we lied to the
> app.
> If the app requested an MSAA buffer, we said "ok" and just created a
> single-sample
> buffer.
>
> Can we do that for these formats? It may be better to do that than report
> GL_FRAMEBUFFER_UNSUPPORTED. I'm unsure what's best, though.
>

Thanks for the suggestion.  At the risk of giving off the impression that I
already have my mind made up, I *did* consider this approach, and here's
why I rejected it: all attachments to a framebuffer need to have the same
sample count.  So if we secretly substitute a single-sampled RGBA32F buffer
in place of the 4x RGBA32F buffer the client program requests, we'll also
have to secretly substitute single-sampled depth and stencil buffers, as
well as single-sampled versions of all the other color buffers the client
program might have attached to the same framebuffer (because a framebuffer
can have multiple color attachments).  We can't do this substitution until
drawing is done (or the client program calls glCheckFramebufferStatus()),
because between draws, the client program might have incompatible
renderbuffers attached to the same framebuffer as a transitory condition.
 Getting the logic right for secretly swapping in single-sampled
framebuffers, at the time of the draw call, is going to be tricky.

More difficulties arise if the client program tries to re-use some, but not
all, of the renderbuffers in the same framebuffer.  Let's say that after
doing some rendering using an RGBA32F/DEPTH24_STENCIL8 combination, the
user detaches the RGBA32F renderbuffer and attaches a 4x RGBA8 renderbuffer
in its place.  Do we now magically transform the single-sampled
DEPTH24_STENCIL8 buffer (which the client thinks is multisampled) into a
truly multisampled DEPTH24_STENCIL8 buffer?  When do we do the switch?  Do
we try to preserve the contents of the depth/stencil buffer when we do the
switch?

Things get worse for Gen7, which supports these formats for 4x MSAA but not
for 8x MSAA?  Do we substitute in 4x MSAA in the place of 8x MSAA?  If so,
then what happens if the client program tries to blit from a 4x depth
buffer (which they think is 8x) to a depth buffer that truly is 8x?  The
client program would expect that to work, but we can't do it, because the
true sample counts don't match.

These kinds of questions make me really scared of this alternative.  None
of these kinds of questions arose during the period after releasing GL 3.0
when we were deliberately lying to the app, because we were lying uniformly
for all renderbuffers.  Also we were deliberately lying as a stopgap
solution, so that apps that required GL 3.0 but didn't require MSAA would
be able to run while we were finishing the MSAA implementation.
 Deliberately lying as a permanent solution to a hardware limitation seems
harder to justify.

Finally, I'm not entirely convinced that my proposed solution goes against
the spec.  The part of the spec that lists RGBA32F as a "required" color
buffer format is very far away from the part of the spec that specifies how
GL_FRAMEBUFFER_UNSUPPORTED is supposed to behave, and you have to follow a
lot of cross-references and make some assumptions in order to connect them.
 Also, the spec lists RGBA32I and RGBA32UI in the same place as it lists
RGBA32F as "required" color buffer formats, but my nVidia system reports
GL_FRAMEBUFFER_UNSUPPORTED for those (regardless of whether MSAA is in
use), so there is some precedent for reporting GL_FRAMEBUFFER_UNSUPPORTED
even on supposedly "required" formats.

Having said all that (and I said a lot more than I expected to), I'm still
trying to keep an open mind.  One thing that would change my mind pretty
quick, for example, would be if we knew about a popular OpenGL app that
assumed it could do MSAA rendering using an RGBA32F buffer, and didn't have
a fallback available in case the framebuffer was incomplete :)

Paul
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120615/12ecd15b/attachment-0001.html>