[Mesa-dev] [PATCH v2 08/13] gen7 fbo: make unmatched depth/stencil configs return unsupported

[Paul Berry]

On 19 July 2013 17:32, Jordan Justen <jordan.l.justen at intel.com> wrote:

> For gen >= 7, we will use the lod/minimum-array-element fields to
> support layered rendering. This means that we must restrict
> the depth & stencil attachments to match in various more retrictive
> ways. (Now the width, height, depth, LOD and layer must match)
>
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Paul Berry <stereotype441 at gmail.com>
>

Since this is the commit we're going to bisect back to if a client program
ever runs into trouble for violating these restrictions, it would be nice
to have a little bit more exposition here in the commit message.  In
particular it's not obvious why the width, height, and depth must match.
It also would be nice to say something about what our fallback plan is if
this the new restrictions prove too onerous.  How about adding some text
like this to the commit message?

The reason width, height, and depth must match is that the hardware has a
single set of width, height, and depth settings (in 3DSTATE_DEPTH_BUFFER)
that affect both the depth and stencil buffers.  Since these controls
determine the miptree layout, they need to be set correctly in order for
lod and minimum-array-element to work properly.  So the only way rendering
can work is if the width, height, and depth match.

In the future, if this restriction proves to be a problem (say because some
crucial client application relies on rendering to different levels/layers
of stencil and depth buffers), then we can always work around the
restriction by copying depth and/or stencil data to a temporary buffer
prior to rendering (much in the same way that
brw_workaround_depthstencil_alignment() does today for gen < 7), but
hopefully that won't be necessary.


With the addition of the above text (or something like it), this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>


> ---
>  src/mesa/drivers/dri/i965/intel_fbo.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> b/src/mesa/drivers/dri/i965/intel_fbo.c
> index e746cb4..1826a29 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -569,6 +569,22 @@ intel_validate_framebuffer(struct gl_context *ctx,
> struct gl_framebuffer *fb)
>     }
>
>     if (depth_mt && stencil_mt) {
> +      if (brw->gen >= 7) {
> +         /* For gen >= 7, we are using the lod/minimum-array-element
> fields
> +          * and supportting layered rendering. This means that we must
> restrict
> +          * the depth & stencil attachments to match in various more
> retrictive
> +          * ways. (width, height, depth, LOD and layer)
> +          */
>
+        if (depth_mt->physical_width0 != stencil_mt->physical_width0 ||
> +             depth_mt->physical_height0 != stencil_mt->physical_height0 ||
> +             depth_mt->physical_depth0 != stencil_mt->physical_depth0 ||
> +             depthRb->mt_level != stencilRb->mt_level ||
> +            depthRb->mt_layer != stencilRb->mt_layer) {
> +           fbo_incomplete(fb,
> +                           "FBO incomplete: depth and stencil must match
> in"
> +                           "width, height, depth, LOD and layer\n");
> +        }
> +      }
>        if (depth_mt == stencil_mt) {
>          /* For true packed depth/stencil (not faked on
> prefers-separate-stencil
>           * hardware) we need to be sure they're the same level/layer,
> since
> --
> 1.7.10.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130729/e1a56fa9/attachment.html>