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

[Paul Berry]

On 12 March 2013 16:33, Eric Anholt <eric at anholt.net> wrote:

> Paul Berry <stereotype441 at gmail.com> writes:
> >  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.
>

Good point.  I'll fix that before pushing.  I'll also make a follow-up
patch to fix the function I borrowed this test from
(intel_miptree_create_layout).


>
> 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.
>

I think you inverted your logic there (if depth_mt->stencil_mt, then
depth_mt->format's base format *will* be GL_DEPTH_STENCIL).  Am I correct
in inferring that the cases you're worried about are cases like:

- Client creates a GL_DEPTH_STENCIL texture on a platform such as Gen7 that
uses separate stencil
- Client executes a glClear(GL_DEPTH_BIT) on a miplevel that needs the
workaround, expecting stencil data to be preserved

I think you may be right to worry about this--previously I had assumed that
calling intel_renderbuffer_move_to_temp(depth_mt) on a depth/stencil
texture backed by separate stencil would only relocate the depth miptree
layer, and leave the stencil miptree layer alone.  But rereading the code
again, I'm less certain of that.

I'll write a piglit test to exercise exactly this situation, and post a v2
of the patch if necessary.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130313/517fd1c6/attachment.html>