[virglrenderer-devel] [PATCH 3/5] vrend: store offsets into backing ivo for mipmaps and use it when reading data back (v2)

Gurchetan Singh gurchetansingh at chromium.org
Fri Jun 15 22:50:22 UTC 2018


Patches 1, 2, 4, and 5 are:

Reviewed-by: Gurchetan Singh <gurchetansingh at chromium.org>



On Fri, Jun 15, 2018 at 10:51 AM Gert Wollny <gert.wollny at collabora.com>
wrote:

> In the copy fallback, when a texture can not be rendered, the data that
> resides
> in the backing iovec needs to be used. For the non-zero levels of mip-map
> textures
> the data is located at an offset. This patch adds storing this offset and
> using it
> when data is read from the backing iovec and updating the dst iov. We
> limit the
> mip-map levels for which this is done to 1-17, which is enough to cover
> 32kx32k textures. The patch also fixes the stride when accessing mip-map
> levels.
>
> Fixes:
>
> dEQP-GLES3.functional.texture.specification.teximage3d_depth.depth_component24_2d_array
>
> dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth_component32f_2d_array
>
> dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth_component24_2d_array
>
> dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth_component16_2d_array
>
> dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth32f_stencil8_2d_array
>
> dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth24_stencil8_2d_array
>
> v2: * rebase and remove unused variables
>     * also correct offset when writing to the destination backing iovec
>
> Signed-off-by: Jakob Bornecrantz <jakob at collabora.com> (v1)
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
>  src/vrend_renderer.c | 31 ++++++++++++++++++++++++++-----
>  src/vrend_renderer.h |  4 ++++
>  2 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index e35cc3f..bce84d2 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -5405,6 +5405,20 @@ static int vrend_renderer_transfer_write_iov(struct
> vrend_context *ctx,
>           x = info->box->x;
>           y = invert ? (int)res->base.height0 - info->box->y -
> info->box->height : info->box->y;
>
> +
> +         /* mipmaps are usually passed in one iov, and we need to keep
> the offset
> +          * into the data in case we want to read back the data of a
> surface
> +          * that can not be rendered. Since we can not assume that the
> whole texture
> +          * is filled, we evaluate the offset for origin (0,0,0). Since
> it is also
> +          * possible that a resource is reused and resized update the
> offset every time.
> +          * We assume that level 0 has offset 0.
> +          */
> +         if (info->level> 0 && info->level <= MAX_READBACK_MIPMAP_LEVELS)
> {
>

Why don't you just have a case for level 0?  Right now it's a little
confusing to follow.  It's also consistent with
EGL_EXT_image_dma_buf_import offsets.



> +            int64_t level_height = u_minify(res->base.height0,
> info->level);
> +            res->mipmap_offsets[info->level-1] = info->offset -
> +                                               ((info->box->z *
> level_height + y) * stride + x * elsize);
>

I don't fully understand this calculation.
>From  virgl_texture_transfer_map it seems info->offset represents the start
of the transfer from with given {x, y, z} coordination in the texture.  So:
info->offset == info->box->z * level_height + y * level_stride + x * elsize

So won't mipmap_offsets always be zero?

Also nit: parens would make more sense like this -- info->offset
- (info->box->z * level_height + y * stride + x * elsize);


> +         }
> +
>           if (res->base.format == (enum
> pipe_format)VIRGL_FORMAT_Z24X8_UNORM) {
>              /* we get values from the guest as 24-bit scaled integers
>                 but we give them to the host GL and it interprets them
> @@ -6088,8 +6102,6 @@ static void vrend_resource_copy_fallback(struct
> vrend_resource *src_res,
>
>     box = *src_box;
>     box.depth = vrend_get_texture_depth(src_res, src_level);
> -
> -   src_stride = util_format_get_stride(src_res->base.format,
> src_res->base.width0);
>     dst_stride = util_format_get_stride(dst_res->base.format,
> dst_res->base.width0);
>
>     /* this is ugly need to do a full GetTexImage */
> @@ -6111,12 +6123,21 @@ static void vrend_resource_copy_fallback(struct
> vrend_resource *src_res,
>      * iovec to have the data we need, otherwise we can use glGetTexture
>      */
>     if (vrend_state.use_gles) {
> -      read_transfer_data(&src_res->base, src_res->iov, src_res->num_iovs,
> -                         tptr, src_stride, &box, src_level, 0, false);
> +      uint64_t src_offset = 0;
> +      uint64_t dst_offset = 0;
> +      if (src_level > 0 && src_level <= MAX_READBACK_MIPMAP_LEVELS) {
> +         src_offset = src_res->mipmap_offsets[src_level-1];
> +         dst_offset = dst_res->mipmap_offsets[src_level-1];
> +      }
> +
> +      src_stride = util_format_get_nblocksx(src_res->base.format,
> +
> u_minify(src_res->base.width0, src_level)) * elsize;
> +      read_transfer_data(&src_res->base, src_res->iov, src_res->num_iovs,
> tptr,
> +                         src_stride, &box, src_level, src_offset, false);
>        /* In on GLES Sync the iov that backs the dst resource because
>         * we might need it in a chain copy A->B, B->C */
>        write_transfer_data(&dst_res->base, dst_res->iov,
> dst_res->num_iovs, tptr,
> -                          dst_stride, &box, src_level, 0, false);
> +                          dst_stride, &box, src_level, dst_offset, false);
>        /* we get values from the guest as 24-bit scaled integers
>           but we give them to the host GL and it interprets them
>           as 32-bit scaled integers, so we need to scale them here */
> diff --git a/src/vrend_renderer.h b/src/vrend_renderer.h
> index 042d439..258c439 100644
> --- a/src/vrend_renderer.h
> +++ b/src/vrend_renderer.h
> @@ -44,6 +44,9 @@ struct virgl_gl_ctx_param {
>  extern int vrend_dump_shaders;
>  struct vrend_context;
>
> +/* Number of mipmap levels for which to keep the backing iov offsets */
> +#define MAX_READBACK_MIPMAP_LEVELS 16
> +
>  struct vrend_resource {
>     struct pipe_resource base;
>     GLuint id;
> @@ -62,6 +65,7 @@ struct vrend_resource {
>     char *ptr;
>     struct iovec *iov;
>     uint32_t num_iovs;
> +   uint64_t mipmap_offsets[MAX_READBACK_MIPMAP_LEVELS];
>  };
>
>  /* assume every format is sampler friendly */
> --
> 2.17.1
>
> _______________________________________________
> virglrenderer-devel mailing list
> virglrenderer-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/virglrenderer-devel/attachments/20180615/ff12b324/attachment-0001.html>


More information about the virglrenderer-devel mailing list