[Mesa-dev] [PATCH 2/2] i965: Avoid unnecessary copy when depthstencil workaround invoked by clear.

[Eric Anholt]

Paul Berry <stereotype441 at gmail.com> writes:

> Since apps typically begin rendering with a call to glClear(), it is
> likely that when brw_workaround_depthstencil_alignment() moves a
> miplevel to a temporary buffer, it can avoid doing a blit, since the
> contents of the miplevel are about to be erased.
>
> This patch adds the necessary plumbing to determine when
> brw_workaround_depthstencil_alignment() is being called as a
> consequence of glClear(), and avoids the unnecessary blit when it is
> safe to do so.
> ---
>  src/mesa/drivers/dri/i965/brw_clear.c            |  4 +++-
>  src/mesa/drivers/dri/i965/brw_context.h          |  3 ++-
>  src/mesa/drivers/dri/i965/brw_draw.c             |  2 +-
>  src/mesa/drivers/dri/i965/brw_misc_state.c       | 26 +++++++++++++++++++-----
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  2 +-
>  src/mesa/drivers/dri/intel/intel_fbo.c           | 10 +++++++--
>  src/mesa/drivers/dri/intel/intel_fbo.h           |  3 ++-
>  7 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clear.c b/src/mesa/drivers/dri/i965/brw_clear.c
> index cde1a06..e740f65 100644
> --- a/src/mesa/drivers/dri/i965/brw_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> @@ -223,6 +223,8 @@ brw_clear(struct gl_context *ctx, GLbitfield mask)
>  {
>     struct brw_context *brw = brw_context(ctx);
>     struct intel_context *intel = &brw->intel;
> +   struct gl_framebuffer *fb = ctx->DrawBuffer;
> +   bool partial_clear = ctx->Scissor.Enabled && !noop_scissor(ctx, fb);
>  
>     if (!_mesa_check_conditional_render(ctx))
>        return;
> @@ -232,7 +234,7 @@ brw_clear(struct gl_context *ctx, GLbitfield mask)
>     }
>  
>     intel_prepare_render(intel);
> -   brw_workaround_depthstencil_alignment(brw);
> +   brw_workaround_depthstencil_alignment(brw, partial_clear ? 0 : mask);

Once this is moved into brw_fast_clear(), the partial_clear check is
already done for you so the logic doesn't need to get duplicated.


>  void
> -brw_workaround_depthstencil_alignment(struct brw_context *brw)
> +brw_workaround_depthstencil_alignment(struct brw_context *brw,
> +                                      GLbitfield clear_mask)
>  {
>     struct intel_context *intel = &brw->intel;
>     struct gl_context *ctx = &intel->ctx;
> @@ -341,10 +343,24 @@ brw_workaround_depthstencil_alignment(struct brw_context *brw)
>     struct intel_mipmap_tree *stencil_mt = get_stencil_miptree(stencil_irb);
>     uint32_t tile_x = 0, tile_y = 0, stencil_tile_x = 0, stencil_tile_y = 0;
>     uint32_t stencil_draw_x = 0, stencil_draw_y = 0;
> +   bool invalidate_depth = clear_mask & GL_DEPTH_BUFFER_BIT;
> +   bool invalidate_stencil = clear_mask & GL_STENCIL_BUFFER_BIT;
>  
>     if (depth_irb)
>        depth_mt = depth_irb->mt;
>  
> +   if (depth_irb && invalidate_depth
> +       && _mesa_is_depthstencil_format(
> +              _mesa_get_format_base_format(depth_mt->format))
> +       && !depth_mt->stencil_mt) {

The only _mesa_is_depthstencil_format() returned by
_mesa_get_format_base_format() is GL_DEPTH_STENCIL, so calling that
seems kinda overkill.

If depth_mt->stencil_mt, then depth_mt->format's base format will not be
GL_DEPTH_STENCIL.  I'm concerned that you're going to lose the
depth_mt->stencil_mt contents of a gl-level packed depth/stencil texture
that's backed by separate stencil.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130312/f99e3b75/attachment.pgp>