[Mesa-dev] [PATCH 08/30] i965: Inline renderbuffer_att_set_needs_depth_resolve

[Jason Ekstrand]

On Fri, Jun 2, 2017 at 2:41 PM, Chad Versace <chadversary at 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);
> >     }
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170602/548f4970/attachment.html>