On Tue, Oct 13, 2015 at 09:14:29PM -0700, Matt Turner wrote:
> On Tue, Oct 13, 2015 at 9:12 PM, Matt Turner <matts...@gmail.com> wrote:
> > On Tue, Oct 13, 2015 at 8:50 PM, Ben Widawsky
> > <benjamin.widaw...@intel.com> wrote:
> >> The impetus for this patch comes from a seemingly benign statement within
> >> the
> >> spec (quoted within the patch). For me, this patch was at some point
> >> critical
> >> for getting stable piglit results (though this did not seem to be the case
> >> on a
> >> branch Chad was working on).
> >>
> >> It is very important for clearing multiple color buffer attachments and
> >> can be
> >> observed in the following piglit tests:
> >> spec/arb_framebuffer_object/fbo-drawbuffers-none glclear
> >> spec/ext_framebuffer_multisample/blit-multiple-render-targets 0
> >>
> >> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> >> ---
> >> src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 97
> >> +++++++++++++++++++++----
> >> 1 file changed, 84 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> >> b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> >> index 7bf52f0..9e6711e 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> >> @@ -427,6 +427,74 @@ use_rectlist(struct brw_context *brw, bool enable)
> >> brw->ctx.NewDriverState |= BRW_NEW_FRAGMENT_PROGRAM;
> >> }
> >>
> >> +/**
> >> + * Individually fast clear each color buffer attachment. On previous gens
> >> this
> >> + * isn't required. The motivation for this comes from one line (which
> >> seems to
> >> + * be specific to SKL+). The list item is in section titled _MCS Buffer
> >> for
> >> + * Render Target(s)_
> >> + *
> >> + * "Since only one RT is bound with a clear pass, only one RT can be
> >> cleared
> >> + * at a time. To clear multiple RTs, multiple clear passes are
> >> required."
> >> + *
> >> + * The code follows the same idea as the resolve code which creates a
> >> fake FBO
> >> + * to avoid interfering with too much of the GL state.
> >> + */
> >> +static void
> >> +fast_clear_attachments(struct brw_context *brw,
> >> + struct gl_framebuffer *fb,
> >> + uint32_t fast_clear_buffers,
> >> + struct rect fast_clear_rect)
> >> +{
> >> + assert(brw->gen >= 9);
> >> + struct gl_context *ctx = &brw->ctx;
> >> +
> >> + GLuint old_fb = ctx->DrawBuffer->Name;
> >> +
> >> + for (unsigned buf = 0; buf < fb->_NumColorDrawBuffers; buf++) {
> >> + struct gl_renderbuffer *rb = fb->_ColorDrawBuffers[buf];
> >> + struct intel_renderbuffer *irb = intel_renderbuffer(rb);
> >> + GLuint fbo, rbo;
> >> + int index = fb->_ColorDrawBufferIndexes[buf];
> >> +
> >> + if (!((1 << index) & fast_clear_buffers))
> >
> > Small suggestion: I think this would read better as
> > ((fast_clear_buffers & (1 << index)) != 0.
>
> Oops, sorry -- of course I meant ((fast_clear_buffers & (1 << index)) == 0.
I like this better too, but I do feel it's a bit contradictory to compare to 0
here when you asked me to not compare to true in an earlier patch :P
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
