[Mesa-dev] [PATCH 04/12] anv: Add func anv_image_has_hiz()

[Chad Versace]

On Tue 13 Sep 2016, Nanley Chery wrote:
> On Wed, Sep 07, 2016 at 03:51:14PM -0700, Chad Versace wrote:
> > On Wed 07 Sep 2016, Nanley Chery wrote:
> > > On Fri, Sep 02, 2016 at 11:42:24AM -0700, Chad Versace wrote:
> > > > On Thu 01 Sep 2016, Jason Ekstrand wrote:
> > > > > On Wed, Aug 31, 2016 at 8:29 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> > > > > 
> > > > >     From: Chad Versace <chad.versace at intel.com>
> > > > > 
> > > > >     Nanley Chery (amend):
> > > > >      - Remove wip! tag
> > > > > 
> > > > >     Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > > > >     ---
> > > > >      src/intel/vulkan/anv_private.h | 10 ++++++++++
> > > > >      1 file changed, 10 insertions(+)
> > > > > 
> > > > 
> > > > 
> > > > >     +static inline bool
> > > > >     +anv_image_has_hiz(const struct anv_image *image)
> > > > >     +{
> > > > >     +   /* We must check the usage because anv_image::hiz_surface belongs to
> > > > >     +    * a union.
> > > > >     +    */
> > > > >     +   return (image->usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) &&
> > > > > 
> > > > > 
> > > > > Would checking (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT) be more
> > > > > appropriate?
> > > > 
> > > > I agree. VK_IMAGE_ASPECT_DEPTH_BIT makes more sense.
> > > > 
> > > > Also, that's what the documentation for anv_image says, quoted below:
> > > >    
> > > >    struct anv_image {
> > > >       ...
> > > > 
> > > >       /**
> > > >        * Image subsurfaces
> > > >        *
> > > >        * For each foo, anv_image::foo_surface is valid if and only if
> > > >        * anv_image::aspects has a foo aspect.
> > > >        *
> > > >        * ...
> > > >        */
> > > >       union {
> > > >          struct anv_surface color_surface;
> > > >    
> > > >          struct {
> > > >             struct anv_surface depth_surface;
> > > >             struct anv_surface stencil_surface;                                            
> > > >          };
> > > >       };
> > > >    };
> > > >    
> > > 
> > > Sure. Thanks for the documentation quote.
> > > 
> > > A HiZ surface is created for a depth image if both usage and aspect conditions
> > > are satisfied. Would it be better for me to add the aspect check instead of
> > > replacing the usage check with it?
> > 
> > I see. You want to avoid allocating the HiZ surface if it's never
> > rendered as a depth attachment. 
> > 
> > So yes, your suggestion sounds good to me.
> 
> I'll actually leave it out if you don't mind. The usage check isn't
> required to get the right result.

Sure. As long as the aspect check is present, then it's good with me.