On Mon, Feb 12, 2018 at 5:11 PM, Nanley Chery <nanleych...@gmail.com> wrote:
> On Mon, Feb 12, 2018 at 04:35:20PM -0800, Jason Ekstrand wrote:
> > Previously, we just used all the channels regardless of the format.
> > This is less than ideal because some channels may have undefined values
> > and this should be ok from the client's perspective. Even though the
> > driver should do the correct thing regardless of what is in the
> > undefined value, it makes things less deterministic. In particular, the
> > driver may choose to fast-clear or not based on undefined values. This
> > level of nondeterminism is bad.
> >
> > Cc: mesa-sta...@lists.freedesktop.org
> > ---
> > src/intel/vulkan/genX_cmd_buffer.c | 47 ++++++++++++++++--------------
> --------
> > 1 file changed, 20 insertions(+), 27 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index 99854eb..a574024 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -202,24 +202,6 @@ add_image_view_relocs(struct anv_cmd_buffer
> *cmd_buffer,
> > }
> > }
> >
> > -static bool
> > -color_is_zero_one(VkClearColorValue value, enum isl_format format)
> > -{
> > - if (isl_format_has_int_channel(format)) {
> > - for (unsigned i = 0; i < 4; i++) {
> > - if (value.int32[i] != 0 && value.int32[i] != 1)
> > - return false;
> > - }
> > - } else {
> > - for (unsigned i = 0; i < 4; i++) {
> > - if (value.float32[i] != 0.0f && value.float32[i] != 1.0f)
> > - return false;
> > - }
> > - }
> > -
> > - return true;
> > -}
> > -
> > static void
> > color_attachment_compute_aux_usage(struct anv_device * device,
> > struct anv_cmd_state * cmd_state,
> > @@ -294,13 +276,26 @@ color_attachment_compute_aux_usage(struct
> anv_device * device,
> >
> > assert(iview->image->planes[0].aux_surface.isl.usage &
> ISL_SURF_USAGE_CCS_BIT);
> >
> > + const struct isl_format_layout *view_fmtl =
> > + isl_format_get_layout(iview->planes[0].isl.format);
> > + union isl_color_value clear_color = {};
>
> Is this initializer valid?
>
It's a GCC extension (also supported by clang), but yes.
> > +
> > +#define COPY_CLEAR_COLOR_CHANNEL(c, i) \
> > + if (view_fmtl->channels.c.bits) \
> > + clear_color.u32[i] = att_state->clear_value.color.uint32[i]
> > +
> > + COPY_CLEAR_COLOR_CHANNEL(r, 0);
> > + COPY_CLEAR_COLOR_CHANNEL(g, 1);
> > + COPY_CLEAR_COLOR_CHANNEL(b, 2);
> > + COPY_CLEAR_COLOR_CHANNEL(a, 3);
> > +
> > +#undef COPY_CLEAR_COLOR_CHANNEL
> > +
> > att_state->clear_color_is_zero_one =
> > - color_is_zero_one(att_state->clear_value.color,
> iview->planes[0].isl.format);
> > + isl_color_value_is_zero_one(*fast_clear_color,
>
> Should this be clear_color?
>
Yes it should. Fixed locally.
> > + iview->planes[0].isl.format);
> > att_state->clear_color_is_zero =
> > - att_state->clear_value.color.uint32[0] == 0 &&
> > - att_state->clear_value.color.uint32[1] == 0 &&
> > - att_state->clear_value.color.uint32[2] == 0 &&
> > - att_state->clear_value.color.uint32[3] == 0;
> > + isl_color_value_is_zero(*fast_clear_color,
> iview->planes[0].isl.format);
> >
>
> Should this be clear_color?
>
Yes it should. Fixed locally. This caused a lot of fails. I don't know
how I didn't catch it. :(
Do you want a v2?
--Jason
>
> -Nanley
>
> > if (att_state->pending_clear_aspects == VK_IMAGE_ASPECT_COLOR_BIT) {
> > /* Start by getting the fast clear type. We use the first subpass
> > @@ -358,10 +353,8 @@ color_attachment_compute_aux_usage(struct
> anv_device * device,
> > "LOAD_OP_CLEAR. Only fast-clearing the first
> slice");
> > }
> >
> > - if (att_state->fast_clear) {
> > - memcpy(fast_clear_color->u32, att_state->clear_value.color.
> uint32,
> > - sizeof(fast_clear_color->u32));
> > - }
> > + if (att_state->fast_clear)
> > + *fast_clear_color = clear_color;
> > } else {
> > att_state->fast_clear = false;
> > }
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
