[Mesa-dev] [PATCH] radv: ignore the loadOp if the first use of an attachment is a resolve

[Samuel Pitoiset]

On 5/26/19 2:50 PM, Bas Nieuwenhuizen wrote:
> On Wed, May 22, 2019 at 11:20 AM Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> Based on ANV.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/amd/vulkan/radv_meta_resolve.c | 21 ++++++++++++---------
>>   1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_meta_resolve.c b/src/amd/vulkan/radv_meta_resolve.c
>> index ade5d438438..6a8abce1ddb 100644
>> --- a/src/amd/vulkan/radv_meta_resolve.c
>> +++ b/src/amd/vulkan/radv_meta_resolve.c
>> @@ -618,15 +618,6 @@ radv_cmd_buffer_resolve_subpass(struct radv_cmd_buffer *cmd_buffer)
>>          struct radv_meta_saved_state saved_state;
>>          enum radv_resolve_method resolve_method = RESOLVE_HW;
>>
>> -       /* FINISHME(perf): Skip clears for resolve attachments.
>> -        *
>> -        * From the Vulkan 1.0 spec:
>> -        *
>> -        *    If the first use of an attachment in a render pass is as a resolve
>> -        *    attachment, then the loadOp is effectively ignored as the resolve is
>> -        *    guaranteed to overwrite all pixels in the render area.
>> -        */
>> -
>>          if (!subpass->has_resolve)
>>                  return;
>>
>> @@ -637,6 +628,18 @@ radv_cmd_buffer_resolve_subpass(struct radv_cmd_buffer *cmd_buffer)
>>                  if (dest_att.attachment == VK_ATTACHMENT_UNUSED)
>>                          continue;
>>
>> +               if (cmd_buffer->state.attachments[dest_att.attachment].pending_clear_aspects) {
>> +                       /* From the Vulkan 1.0 spec:
>> +                        *
>> +                        *    If the first use of an attachment in a render
>> +                        *    pass is as a resolve attachment, then the loadOp
>> +                        *    is effectively ignored as the resolve is
>> +                        *    guaranteed to overwrite all pixels in the render
>> +                        *    area.
>> +                       */
>> +                       cmd_buffer->state.attachments[dest_att.attachment].pending_clear_aspects = 0;
>> +               }
>> +
> You can drop the if statement. Also this seems like a correctness
> issue, as we only cleared the first time it was used as a color
> attachment, which might happen after a resolve?

Yeah, looks good to me.

I will remove the if statement and update the comment too because the 
spec no longer says that since VK_KHR_depth_stencil_resolve.

Though, we should still make sure that clears don't happen after resolves.

>
>>                  struct radv_image *dst_img = cmd_buffer->state.framebuffer->attachments[dest_att.attachment].attachment->image;
>>                  struct radv_image *src_img = cmd_buffer->state.framebuffer->attachments[src_att.attachment].attachment->image;
>>
>> --
>> 2.21.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev