On Fri, Jun 2, 2017 at 2:41 PM, Chad Versace <chadvers...@chromium.org>
wrote:
> On Fri 26 May 2017, Jason Ekstrand wrote:
> > ---
> > src/mesa/drivers/dri/i965/brw_clear.c | 12 ++++++++++--
> > src/mesa/drivers/dri/i965/brw_draw.c | 12 +++++++++++-
> > src/mesa/drivers/dri/i965/intel_fbo.c | 15 ---------------
> > src/mesa/drivers/dri/i965/intel_fbo.h | 3 ---
> > 4 files changed, 21 insertions(+), 21 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_clear.c
> b/src/mesa/drivers/dri/i965/brw_clear.c
> > index adaf250..9f4a7e6 100644
> > --- a/src/mesa/drivers/dri/i965/brw_clear.c
> > +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> > @@ -207,7 +207,7 @@ brw_fast_clear_depth(struct gl_context *ctx)
> > brw_emit_pipe_control_flush(brw, PIPE_CONTROL_DEPTH_STALL);
> > }
> >
> > - if (fb->MaxNumLayers > 0) {
> > + if (depth_att->Layered) {
> > for (unsigned layer = 0; layer < depth_irb->layer_count; layer++)
> {
> > intel_hiz_exec(brw, mt, depth_irb->mt_level,
> > depth_irb->mt_layer + layer,
>
> There's a hidden 'else' branch here. My comment below applies to it too.
>
> > @@ -236,7 +236,15 @@ brw_fast_clear_depth(struct gl_context *ctx)
> > /* Now, the HiZ buffer contains data that needs to be resolved to
> the depth
> > * buffer.
> > */
> > - intel_renderbuffer_att_set_needs_depth_resolve(depth_att);
> > + if (fb->MaxNumLayers > 0) {
> > + for (unsigned layer = 0; layer < depth_irb->layer_count; layer++)
> {
> > + intel_miptree_slice_set_needs_depth_resolve(mt,
> depth_irb->mt_level,
> > +
> depth_irb->mt_layer + layer);
> > + }
> > + } else {
> > + intel_miptree_slice_set_needs_depth_resolve(mt,
> depth_irb->mt_level,
> > + depth_irb->mt_layer);
> > + }
>
> I believe you could simplify this by eliminating the 'else' branch. As
> long as depth_irb->layer_count == 1 for non-layered renderbuffers (and
> I really hope it is), then the 'for' loop does the right thing.
>
Sure. I was sort-of trying to avoid functional changes. That said... I'm
happy to make the change. Lots of stuff would suddenly get cleaner. Mind
if that's a follow-on patch to the series?
> >
> > return true;
> > }
> > diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> b/src/mesa/drivers/dri/i965/brw_draw.c
> > index 23a3c6c..f728731 100644
> > --- a/src/mesa/drivers/dri/i965/brw_draw.c
> > +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> > @@ -373,7 +373,17 @@ brw_postdraw_set_buffers_need_resolve(struct
> brw_context *brw)
> > if (back_irb)
> > back_irb->need_downsample = true;
> > if (depth_irb && brw_depth_writes_enabled(brw)) {
> > - intel_renderbuffer_att_set_needs_depth_resolve(depth_att);
> > + if (depth_att->Layered) {
> > + for (unsigned layer = 0; layer < depth_irb->layer_count;
> layer++) {
> > + intel_miptree_slice_set_needs_depth_resolve(depth_irb->mt,
> > +
> depth_irb->mt_level,
> > +
> depth_irb->mt_layer + layer);
> > + }
> > + } else {
>
> And this branch too.
>
> > + intel_miptree_slice_set_needs_depth_resolve(depth_irb->mt,
> > +
> depth_irb->mt_level,
> > +
> depth_irb->mt_layer);
> > + }
> > brw_render_cache_set_add_bo(brw, depth_irb->mt->bo);
> > }
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
