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