[Mesa-dev] [RFC PATCH 06/12] gen7 depth surface: calculate minimum array element being rendered

[Paul Berry]

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

> In layered rendering this will be 0. Otherwise it will be the
> selected slice.
>
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/gen7_blorp.cpp    |    6 ++++++
>  src/mesa/drivers/dri/i965/gen7_misc_state.c |    9 +++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> index 7df78f6..3d1710e 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -659,6 +659,7 @@ gen7_blorp_emit_depth_stencil_config(struct
> brw_context *brw,
>     uint32_t tile_mask_x, tile_mask_y;
>     uint32_t surftype;
>     int depth = MAX2(params->depth.mt->logical_depth0, 1);
> +   int min_array_element;
>     GLenum gl_target = params->depth.mt->target;
>     int lod;
>
> @@ -678,6 +679,11 @@ gen7_blorp_emit_depth_stencil_config(struct
> brw_context *brw,
>        break;
>     }
>
> +   min_array_element = params->depth.layer;
> +   if (params->depth.mt->num_samples > 1) {
> +      min_array_element /= params->depth.mt->num_samples;
>

Can we add a comment here explaining why this is necessary (maybe something
like "The value of params->depth.layer comes from the 'layer' argument of
intel_hiz_exec's 'layer' parameter, which is a physical layer.  To convert
that to a logical layer, we have to divide by num_samples."

In a future clean-up series I'd be interested in trying to change things so
that intel_hiz_exec's 'layer' parameter is a logical layer, so that we
don't have to do this division.  But I don't want to hold up your patch
series waiting for that clean-up.


> +   }
> +
>     lod = params->depth.level - params->depth.mt->first_level;
>
>     /* 3DSTATE_DEPTH_BUFFER */
> diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> index 404d7d2..87fbfaa 100644
> --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> @@ -45,6 +45,7 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
>     struct gl_framebuffer *fb = ctx->DrawBuffer;
>     uint32_t surftype;
>     int depth = 1;
> +   int min_array_element;
>     GLenum gl_target = GL_TEXTURE_2D;
>     int lod;
>     bool layered = false;
> @@ -84,6 +85,14 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
>        break;
>     }
>
> +   if (layered || !irb) {
> +      min_array_element = 0;
> +   } else if (irb->mt->num_samples > 1) {
> +      min_array_element = irb->mt_layer / irb->mt->num_samples;
>

A similar comment would be nice here, to explain that irb->mt_layer is a
physical layer, and we need to divide by num_samples to convert to a
logical layer.

With the extra comments, this patch is:

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


> +   } else {
> +      min_array_element = irb->mt_layer;
> +   }
> +
>     lod = irb ? irb->mt_level - irb->mt->first_level : 0;
>
>     /* _NEW_DEPTH, _NEW_STENCIL, _NEW_BUFFERS */
> --
> 1.7.10.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130717/4f35f9c4/attachment.html>