On 12 March 2013 12:53, Paul Berry <stereotype...@gmail.com> wrote:
> On 12 March 2013 12:28, Eric Anholt <e...@anholt.net> wrote:
>
>> Paul Berry <stereotype...@gmail.com> writes:
>>
>> > Fast depth clears have the same depth/stencil alignment requirements
>> > as other drawing operations. Therefore, we need to call
>> > brw_workaround_depthstencil_alignment() from both the clear and
>> > drawing paths.
>> >
>> > Without this fix, we get image corruption if the following conditions
>> > hold: (a) the first ever drawing operation to a depth miplevel (or the
>> > first drawing operation after having used the texture for sampling) is
>> > a clear, (b) the depth miplevel has a size that is eligible for fast
>> > depth clears, and (c) the depth miplevel has an offset within the
>> > miptree that isn't 8x8 aligned.
>> >
>> > Fixes piglit "depthstencil-render-miplevels" tests with size 273.
>> >
>> > NOTE: This is a candidate for stable branches
>> > ---
>> > src/mesa/drivers/dri/i965/brw_clear.c | 6 +++++-
>> > 1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/brw_clear.c
>> b/src/mesa/drivers/dri/i965/brw_clear.c
>> > index 53d8e54..cde1a06 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_clear.c
>> > +++ b/src/mesa/drivers/dri/i965/brw_clear.c
>> > @@ -40,6 +40,8 @@
>> > #include "intel_mipmap_tree.h"
>> > #include "intel_regions.h"
>> >
>> > +#include "brw_context.h"
>> > +
>> > #define FILE_DEBUG_FLAG DEBUG_BLIT
>> >
>> > static const char *buffer_names[] = {
>> > @@ -219,7 +221,8 @@ brw_fast_clear_depth(struct gl_context *ctx)
>> > static void
>> > brw_clear(struct gl_context *ctx, GLbitfield mask)
>> > {
>> > - struct intel_context *intel = intel_context(ctx);
>> > + struct brw_context *brw = brw_context(ctx);
>> > + struct intel_context *intel = &brw->intel;
>> >
>> > if (!_mesa_check_conditional_render(ctx))
>> > return;
>> > @@ -229,6 +232,7 @@ brw_clear(struct gl_context *ctx, GLbitfield mask)
>> > }
>> >
>> > intel_prepare_render(intel);
>> > + brw_workaround_depthstencil_alignment(brw);
>>
>> It seems like this should be happening in brw_fast_clear(), either
>> before before calling blorp or inside of it, instead of in the potential
>> caller of brw_fast_clear(). Makes sense, though.
>>
>
> 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.
>
>
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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
