<div dir="ltr">On 12 March 2013 12:53, Paul Berry <span dir="ltr"><<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div class="h5">On 12 March 2013 12:28, Eric Anholt <span dir="ltr"><<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>></span> wrote:<br>
</div></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div>Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>> writes:<br>
<br>
> Fast depth clears have the same depth/stencil alignment requirements<br>
> as other drawing operations.  Therefore, we need to call<br>
> brw_workaround_depthstencil_alignment() from both the clear and<br>
> drawing paths.<br>
><br>
> Without this fix, we get image corruption if the following conditions<br>
> hold: (a) the first ever drawing operation to a depth miplevel (or the<br>
> first drawing operation after having used the texture for sampling) is<br>
> a clear, (b) the depth miplevel has a size that is eligible for fast<br>
> depth clears, and (c) the depth miplevel has an offset within the<br>
> miptree that isn't 8x8 aligned.<br>
><br>
> Fixes piglit "depthstencil-render-miplevels" tests with size 273.<br>
><br>
> NOTE: This is a candidate for stable branches<br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_clear.c | 6 +++++-<br>
>  1 file changed, 5 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_clear.c b/src/mesa/drivers/dri/i965/brw_clear.c<br>
> index 53d8e54..cde1a06 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_clear.c<br>
> +++ b/src/mesa/drivers/dri/i965/brw_clear.c<br>
> @@ -40,6 +40,8 @@<br>
>  #include "intel_mipmap_tree.h"<br>
>  #include "intel_regions.h"<br>
><br>
> +#include "brw_context.h"<br>
> +<br>
>  #define FILE_DEBUG_FLAG DEBUG_BLIT<br>
><br>
>  static const char *buffer_names[] = {<br>
> @@ -219,7 +221,8 @@ brw_fast_clear_depth(struct gl_context *ctx)<br>
>  static void<br>
>  brw_clear(struct gl_context *ctx, GLbitfield mask)<br>
>  {<br>
> -   struct intel_context *intel = intel_context(ctx);<br>
> +   struct brw_context *brw = brw_context(ctx);<br>
> +   struct intel_context *intel = &brw->intel;<br>
><br>
>     if (!_mesa_check_conditional_render(ctx))<br>
>        return;<br>
> @@ -229,6 +232,7 @@ brw_clear(struct gl_context *ctx, GLbitfield mask)<br>
>     }<br>
><br>
>     intel_prepare_render(intel);<br>
> +   brw_workaround_depthstencil_alignment(brw);<br>
<br>
</div></div>It seems like this should be happening in brw_fast_clear(), either<br>
before before calling blorp or inside of it, instead of in the potential<br>
caller of brw_fast_clear().  Makes sense, though.<br></blockquote><div><br></div></div></div><div>Chad made the same comment to me in person yesterday.  The reason I put it here is to accommodate patch 2/2 (which allows brw_workaround_depthstencil_alignment to avoid an unnecessary copy when clearing the whole miplevel).  If I move the call to brw_workaround_depthstencil_alignment into brw_fast_clear_depth(), then the unnecessary copy will only be avoided when doing depth clears.  If I leave it here, the unnecessary copy will be avoided for all clears.<br>

</div><div class="im"><div><br></div></div></div></div></div></blockquote><div><br></div><div style>Correction: when I wrote this I momentarily forgot that the workaround is only needed for depth and stencil buffers.  So leaving the call to brw_workaround_depthstencil_alignment here allows us to avoid the unnecessary copy for both depth and stencil clears, not just depth clears.  I still think it's worth it, but it's a far less convincing case. <br>
</div></div></div></div>