[Mesa-dev] [PATCH v3] anv/blorp: multisample resolve all attachment layers

Iago Toral itoral at igalia.com
Wed Feb 21 07:09:41 UTC 2018


Hi Nanley,

thanks for having a look at this, you're right that we should use the
framebuffer dimensions to decide on the number of layers to resolve. 

I'll send a new version with the fix.

Iago

On Tue, 2018-02-20 at 15:18 -0800, Nanley Chery wrote:
> On Thu, Feb 15, 2018 at 09:40:16AM +0100, Iago Toral Quiroga wrote:
> > We were only resolving the first.
> > 
> > v2:
> >   - Do not require that the number of layers on dst and src are an
> >     exact match, it is okay if the dst has more layers so long as
> >     it has at least the same that we are going to resolve.
> >   - Do not always resolve array_len layers, we should resolve
> >     only from base_array_layer to array_len.
> > 
> > v3:
> >   - v2 was assuming that array_len represented the total number of
> >     layers in the image, but it represents the number of layers
> >     starting at the base array ayer.
> > 
> > Fixes new CTS tests for multisampled layered rendering:
> > dEQP-VK.renderpass.multisample_resolve.layers_*
> > ---
> >  src/intel/vulkan/anv_blorp.c | 30 +++++++++++++++++++-----------
> >  1 file changed, 19 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/intel/vulkan/anv_blorp.c
> > b/src/intel/vulkan/anv_blorp.c
> > index d38b343671..df566773a4 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -1543,25 +1543,33 @@ anv_cmd_buffer_resolve_subpass(struct
> > anv_cmd_buffer *cmd_buffer)
> >           get_blorp_surf_for_anv_image(cmd_buffer->device,
> > dst_iview->image,
> >                                        VK_IMAGE_ASPECT_COLOR_BIT,
> >                                        dst_aux_usage, &dst_surf);
> > +
> > +         uint32_t base_src_layer = src_iview-
> > >planes[0].isl.base_array_layer;
> > +         uint32_t base_dst_layer = dst_iview-
> > >planes[0].isl.base_array_layer;
> > +         uint32_t num_layers = src_iview->planes[0].isl.array_len;
> 
> num_layers should be equal to fb->layers. As seen in the definition
> of
> renderArea in the Vulkan spec, resolve operations are limited to the
> renderArea, which extends to all layers of the framebuffer.
> 
>    renderArea is the render area that is affected by the render pass
>    instance. The effects of attachment load, store and multisample
> resolve
>    operations are restricted to the pixels whose x and y coordinates
> fall
>    within the render area on all attachments. The render area extends
> to
>    all layers of framebuffer.
> 
> > +         assert(num_layers <= dst_iview->planes[0].isl.array_len);
> > +
> 
> This assertion is false. The spec allows having an arrayed
> multisample
> source image view and a non-arrayed single-sampled destination image
> view as long as the framebuffer is non-arrayed.
> 
>    Each element of pAttachments must have dimensions at least as
> large as
>    the corresponding framebuffer dimension
> 
> -Nanley
> 
> >           anv_cmd_buffer_mark_image_written(cmd_buffer, dst_iview-
> > >image,
> >                                             VK_IMAGE_ASPECT_COLOR_B
> > IT,
> >                                             dst_surf.aux_usage,
> >                                             dst_iview-
> > >planes[0].isl.base_level,
> > -                                           dst_iview-
> > >planes[0].isl.base_array_layer, 1);
> > +                                           base_dst_layer,
> > num_layers);
> >  
> >           assert(!src_iview->image->format->can_ycbcr);
> >           assert(!dst_iview->image->format->can_ycbcr);
> >  
> > -         resolve_surface(&batch,
> > -                         &src_surf,
> > -                         src_iview->planes[0].isl.base_level,
> > -                         src_iview-
> > >planes[0].isl.base_array_layer,
> > -                         &dst_surf,
> > -                         dst_iview->planes[0].isl.base_level,
> > -                         dst_iview-
> > >planes[0].isl.base_array_layer,
> > -                         render_area.offset.x,
> > render_area.offset.y,
> > -                         render_area.offset.x,
> > render_area.offset.y,
> > -                         render_area.extent.width,
> > render_area.extent.height);
> > +         for (uint32_t i = 0; i < num_layers; i++) {
> > +            resolve_surface(&batch,
> > +                            &src_surf,
> > +                            src_iview->planes[0].isl.base_level,
> > +                            base_src_layer + i,
> > +                            &dst_surf,
> > +                            dst_iview->planes[0].isl.base_level,
> > +                            base_dst_layer + i,
> > +                            render_area.offset.x,
> > render_area.offset.y,
> > +                            render_area.offset.x,
> > render_area.offset.y,
> > +                            render_area.extent.width,
> > render_area.extent.height);
> > +         }
> >        }
> >  
> >        blorp_batch_finish(&batch);
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 


More information about the mesa-dev mailing list