<div dir="ltr">On 12 March 2013 16:33, Eric Anholt <span dir="ltr"><<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class=""><div class="im">Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> writes:<br>
>  void<br>
> -brw_workaround_depthstencil_alignment(struct brw_context *brw)<br>
> +brw_workaround_depthstencil_alignment(struct brw_context *brw,<br>
> +                                      GLbitfield clear_mask)<br>
>  {<br>
>     struct intel_context *intel = &brw->intel;<br>
>     struct gl_context *ctx = &intel->ctx;<br>
> @@ -341,10 +343,24 @@ brw_workaround_depthstencil_alignment(struct brw_context *brw)<br>
>     struct intel_mipmap_tree *stencil_mt = get_stencil_miptree(stencil_irb);<br>
>     uint32_t tile_x = 0, tile_y = 0, stencil_tile_x = 0, stencil_tile_y = 0;<br>
>     uint32_t stencil_draw_x = 0, stencil_draw_y = 0;<br>
> +   bool invalidate_depth = clear_mask & GL_DEPTH_BUFFER_BIT;<br>
> +   bool invalidate_stencil = clear_mask & GL_STENCIL_BUFFER_BIT;<br>
><br>
>     if (depth_irb)<br>
>        depth_mt = depth_irb->mt;<br>
><br>
> +   if (depth_irb && invalidate_depth<br>
> +       && _mesa_is_depthstencil_format(<br>
> +              _mesa_get_format_base_format(depth_mt->format))<br>
> +       && !depth_mt->stencil_mt) {<br>
<br>
</div></div>The only _mesa_is_depthstencil_format() returned by<br>
_mesa_get_format_base_format() is GL_DEPTH_STENCIL, so calling that<br>
seems kinda overkill.<br></blockquote><div><br></div><div>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).<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
If depth_mt->stencil_mt, then depth_mt->format's base format will not be<br>
GL_DEPTH_STENCIL.  I'm concerned that you're going to lose the<br>
depth_mt->stencil_mt contents of a gl-level packed depth/stencil texture<br>
that's backed by separate stencil.<br>
</blockquote></div><br></div><div class="gmail_extra">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:<br>
<br></div><div class="gmail_extra">- Client creates a GL_DEPTH_STENCIL texture on a platform such as Gen7 that uses separate stencil<br></div><div class="gmail_extra">- Client executes a glClear(GL_DEPTH_BIT) on a miplevel that needs the workaround, expecting stencil data to be preserved<br>
<br></div><div class="gmail_extra">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.<br>
<br>I'll write a piglit test to exercise exactly this situation, and post a v2 of the patch if necessary.</div></div>