<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 2, 2017 at 2:41 PM, Chad Versace <span dir="ltr"><<a href="mailto:chadversary@chromium.org" target="_blank">chadversary@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri 26 May 2017, Jason Ekstrand wrote:<br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_<wbr>clear.c | 12 ++++++++++--<br>
>  src/mesa/drivers/dri/i965/brw_<wbr>draw.c  | 12 +++++++++++-<br>
>  src/mesa/drivers/dri/i965/<wbr>intel_fbo.c | 15 ---------------<br>
>  src/mesa/drivers/dri/i965/<wbr>intel_fbo.h |  3 ---<br>
>  4 files changed, 21 insertions(+), 21 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_clear.c b/src/mesa/drivers/dri/i965/<wbr>brw_clear.c<br>
> index adaf250..9f4a7e6 100644<br>
> --- a/src/mesa/drivers/dri/i965/<wbr>brw_clear.c<br>
> +++ b/src/mesa/drivers/dri/i965/<wbr>brw_clear.c<br>
> @@ -207,7 +207,7 @@ brw_fast_clear_depth(struct gl_context *ctx)<br>
>         brw_emit_pipe_control_flush(<wbr>brw, PIPE_CONTROL_DEPTH_STALL);<br>
>     }<br>
><br>
> -   if (fb->MaxNumLayers > 0) {<br>
> +   if (depth_att->Layered) {<br>
>        for (unsigned layer = 0; layer < depth_irb->layer_count; layer++) {<br>
>           intel_hiz_exec(brw, mt, depth_irb->mt_level,<br>
>                          depth_irb->mt_layer + layer,<br>
<br>
</span>There's a hidden 'else' branch here. My comment below applies to it too.<br>
<span class=""><br>
> @@ -236,7 +236,15 @@ brw_fast_clear_depth(struct gl_context *ctx)<br>
>     /* Now, the HiZ buffer contains data that needs to be resolved to the depth<br>
>      * buffer.<br>
>      */<br>
> -   intel_renderbuffer_att_set_<wbr>needs_depth_resolve(depth_att)<wbr>;<br>
> +   if (fb->MaxNumLayers > 0) {<br>
> +      for (unsigned layer = 0; layer < depth_irb->layer_count; layer++) {<br>
> +         intel_miptree_slice_set_needs_<wbr>depth_resolve(mt, depth_irb->mt_level,<br>
> +                                                     depth_irb->mt_layer + layer);<br>
> +      }<br>
> +   } else {<br>
> +      intel_miptree_slice_set_needs_<wbr>depth_resolve(mt, depth_irb->mt_level,<br>
> +                                                  depth_irb->mt_layer);<br>
> +   }<br>
<br>
</span>I believe you could simplify this by eliminating the 'else' branch. As<br>
long as depth_irb->layer_count == 1 for non-layered renderbuffers (and<br>
I really hope it is), then the 'for' loop does the right thing.<span class=""><br></span></blockquote><div><br></div><div>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?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
><br>
>     return true;<br>
>  }<br>
> diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_draw.c b/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> index 23a3c6c..f728731 100644<br>
> --- a/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> +++ b/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> @@ -373,7 +373,17 @@ brw_postdraw_set_buffers_need_<wbr>resolve(struct brw_context *brw)<br>
>     if (back_irb)<br>
>        back_irb->need_downsample = true;<br>
>     if (depth_irb && brw_depth_writes_enabled(brw)) {<br>
> -      intel_renderbuffer_att_set_<wbr>needs_depth_resolve(depth_att)<wbr>;<br>
> +      if (depth_att->Layered) {<br>
> +         for (unsigned layer = 0; layer < depth_irb->layer_count; layer++) {<br>
> +            intel_miptree_slice_set_needs_<wbr>depth_resolve(depth_irb->mt,<br>
> +                                                        depth_irb->mt_level,<br>
> +                                                        depth_irb->mt_layer + layer);<br>
> +         }<br>
> +      } else {<br>
<br>
</span>And this branch too.<br>
<div class="HOEnZb"><div class="h5"><br>
> +         intel_miptree_slice_set_needs_<wbr>depth_resolve(depth_irb->mt,<br>
> +                                                     depth_irb->mt_level,<br>
> +                                                     depth_irb->mt_layer);<br>
> +      }<br>
>        brw_render_cache_set_add_bo(<wbr>brw, depth_irb->mt->bo);<br>
>     }<br>
</div></div></blockquote></div><br></div></div>