[Mesa-dev] [PATCH 09/22] anv/cmd_buffer: Don't partially fast-clear image layers

[Jason Ekstrand]

On Thu, May 11, 2017 at 10:13 AM, Nanley Chery <nanleychery at gmail.com>
wrote:

> On Tue, May 02, 2017 at 05:23:49PM -0700, Jason Ekstrand wrote:
> > On Tue, May 2, 2017 at 4:37 PM, Nanley Chery <nanleychery at gmail.com>
> wrote:
> >
> > > On Tue, May 02, 2017 at 04:25:42PM -0700, Jason Ekstrand wrote:
> > > > On Thu, Apr 27, 2017 at 11:32 AM, Nanley Chery <
> nanleychery at gmail.com>
> > > > wrote:
> > > >
> > > > > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > > > > ---
> > > > >  src/intel/vulkan/genX_cmd_buffer.c | 18 +++++++++++++++---
> > > > >  1 file changed, 15 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > > > index 0ea378fde2..a981b00f67 100644
> > > > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > > > @@ -216,7 +216,7 @@ color_is_zero_one(VkClearColorValue value,
> enum
> > > > > isl_format format)
> > > > >  }
> > > > >
> > > > >  static void
> > > > > -color_attachment_compute_aux_usage(struct anv_device *device,
> > > > > +color_attachment_compute_aux_usage(struct anv_cmd_buffer * const
> > > > > cmd_buffer,
> > > > >
> > > >
> > > > I t may be better to just pass in the framebuffer and attachment
> index
> > > > rather than the whole command buffer.  Slso, I think you're getting
> a bit
> > > > over-excited with the constness. :-)
> > > >
> > > >
> > >
> > > The command buffer is used to look up the render pass in the next
> patch.
> > > If you'd like me to pass in the render pass and framebuffer I could do
> > > that alternatively.
> > >
> >
> > Yeah, that seems better.
> >
> >
>
> The final result will be:
>         color_attachment_compute_aux_usage(struct anv_device * device,
>                                            struct anv_framebuffer *
> framebuffer,
>                                            struct anv_render_pass * pass,
>                                            struct anv_attachment_state
> *att_state,
>                                            const uint32_t att, VkRect2D
> render_area,
>                                            union isl_color_value
> *fast_clear_color)
>

That's not a problem.  Function arguments are cheap.


>
> Is that okay? Looks like I should pass in the cmd_state instead of the
> framebuffer and render_pass.
>
> > > At one point I considered creating a keyboard shortcut to make typing
> > > 'const' easier :). I'm open to hearing your thoughts about our use of
> it
> > > though.
> > >
> >
> > Mostly, every time I see a "struct foo * const thing", I mentally trip
> over
> > C pointer constness syntax trying to figure out what it means.  To me,
> the
> > mental cost is high enough that it's not worth it for the tiny bit of
> extra
> > safety.  It's also more typing.  In general, I tend to disagree with the
> > philosophy of "anything that can be const should be."  I think constness
> is
> > useful for communicating whether or not a function modifies a pointer and
> > if you're doing complicated calculations with piles of variables and you
> > want to make it clear (and compile-checked!) that you don't overwrite one
> > of them.  Outside of that, I'd rather just drop the clutter.  There have
> > been very few times when I've actually found a bug that would have been
> > solved by const when it hasn't been one of those two cases.
> >
> > --Jason
>
> Gotcha.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170511/c661f1aa/attachment.html>