[Mesa-dev] [PATCH 04/18] anv: Update the HiZ sampling helper

[Jason Ekstrand]

On Tue, Feb 28, 2017 at 8:02 AM, Nanley Chery <nanleychery at gmail.com> wrote:

> On Mon, Feb 27, 2017 at 08:48:48PM -0800, Jason Ekstrand wrote:
> > On Feb 27, 2017 5:20 PM, "Nanley Chery" <nanleychery at gmail.com> wrote:
> >
> > Validate the inputs and actually verify that this image has a depth
> > buffer.
> >
> > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > ---
> >  src/intel/vulkan/anv_blorp.c   | 1 +
> >  src/intel/vulkan/anv_image.c   | 7 +++----
> >  src/intel/vulkan/anv_private.h | 7 +++++--
> >  3 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> > index 2cde3b7689..05250de06a 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -1276,6 +1276,7 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer
> > *cmd_buffer)
> >                 clear_with_hiz = false;
> >              } else if (gen == 8 &&
> >                         anv_can_sample_with_hiz(cmd_
> > buffer->device->info.gen,
> > +                                               iview->aspect_mask,
> >                                                 iview->image->samples)) {
> >                 /* Only gen9+ supports returning ANV_HZ_FC_VAL when
> > sampling a
> >                  * fast-cleared portion of a HiZ buffer. Testing has
> > revealed
> > diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> > index 716cdf3a38..f9f264094f 100644
> > --- a/src/intel/vulkan/anv_image.c
> > +++ b/src/intel/vulkan/anv_image.c
> > @@ -502,7 +502,6 @@ anv_layout_to_aux_usage(const uint8_t gen, const
> struct
> > anv_image * const image,
> >         layout == VK_IMAGE_LAYOUT_PREINITIALIZED)
> >        layout = future_layout;
> >
> > -   const bool has_depth = aspects & VK_IMAGE_ASPECT_DEPTH_BIT;
> >     const bool color_aspect = aspects == VK_IMAGE_ASPECT_COLOR_BIT;
> >
> >     /* The following switch currently only handles depth stencil aspects.
> > @@ -538,7 +537,7 @@ anv_layout_to_aux_usage(const uint8_t gen, const
> struct
> > anv_image * const image,
> >        assert(!color_aspect);
> >        /* Fall-through */
> >     case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL:
> > -      if (has_depth && anv_can_sample_with_hiz(gen, image->samples))
> > +      if (anv_can_sample_with_hiz(gen, aspects, image->samples))
> >           return ISL_AUX_USAGE_HIZ;
> >        else
> >           return ISL_AUX_USAGE_NONE;
> > @@ -699,8 +698,8 @@ anv_CreateImageView(VkDevice _device,
> >     float red_clear_color = 0.0f;
> >     enum isl_aux_usage surf_usage = image->aux_usage;
> >     if (image->aux_usage == ISL_AUX_USAGE_HIZ) {
> > -      if (iview->aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT &&
> > -          anv_can_sample_with_hiz(device->info.gen, image->samples)) {
> > +      if (anv_can_sample_with_hiz(device->info.gen, iview->aspect_mask,
> > +                                  image->samples)) {
> >           /* When a HiZ buffer is sampled on gen9+, ensure that
> >            * the constant fast clear value is set in the surface state.
> >            */
> > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> > index 5ce9027f88..b21a9fb359 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -1661,9 +1661,12 @@ struct anv_image {
> >
> >  /* Returns true if a HiZ-enabled depth buffer can be sampled from. */
> >  static inline bool
> > -anv_can_sample_with_hiz(uint8_t gen, uint32_t samples)
> > +anv_can_sample_with_hiz(uint8_t gen, VkImageAspectFlags aspect_mask,
> > +                        uint32_t samples)
> >  {
> > -   return gen >= 8 && samples == 1;
> > +   /* Validate the inputs. */
> > +   assert(gen >= 7 && aspect_mask && samples);
> > +   return gen >= 8 && aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT && samples
> > == 1;
> >
> >
> > I think you want parents around the "aspect & DEPTH"
> >
>
> According to the C11 spec (footnote 85 found under section 6.5
> Expressions), the parenthesis are not necessary because the bitwise AND
> operator has higher precedence than the logical AND operator.
>

I figured but the compiler likes to warn about those things and I really
don't like having to look at the spec in order to figure out what a line of
code means. :-)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170228/3d547763/attachment-0001.html>