<div dir="ltr">Patches 1, 2, 4, and 5 are:<div><br></div><div><div>Reviewed-by: Gurchetan Singh <<a href="mailto:gurchetansingh@chromium.org">gurchetansingh@chromium.org</a>></div><div><br></div><div><br></div><div><br></div><div class="gmail_quote"><div dir="ltr">On Fri, Jun 15, 2018 at 10:51 AM Gert Wollny <<a href="mailto:gert.wollny@collabora.com" target="_blank">gert.wollny@collabora.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">In the copy fallback, when a texture can not be rendered, the data that resides<br>
in the backing iovec needs to be used. For the non-zero levels of mip-map textures<br>
the data is located at an offset. This patch adds storing this offset and using it<br>
when data is read from the backing iovec and updating the dst iov. We limit the<br>
mip-map levels for which this is done to 1-17, which is enough to cover<br>
32kx32k textures. The patch also fixes the stride when accessing mip-map levels.<br>
<br>
Fixes:<br>
dEQP-GLES3.functional.texture.specification.teximage3d_depth.depth_component24_2d_array<br>
dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth_component32f_2d_array<br>
dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth_component24_2d_array<br>
dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth_component16_2d_array<br>
dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth32f_stencil8_2d_array<br>
dEQP-GLES3.functional.texture.specification.texsubimage3d_depth.depth24_stencil8_2d_array<br>
<br>
v2: * rebase and remove unused variables<br>
* also correct offset when writing to the destination backing iovec<br>
<br>
Signed-off-by: Jakob Bornecrantz <<a href="mailto:jakob@collabora.com" target="_blank">jakob@collabora.com</a>> (v1)<br>
Signed-off-by: Gert Wollny <<a href="mailto:gert.wollny@collabora.com" target="_blank">gert.wollny@collabora.com</a>><br>
---<br>
src/vrend_renderer.c | 31 ++++++++++++++++++++++++++-----<br>
src/vrend_renderer.h | 4 ++++<br>
2 files changed, 30 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c<br>
index e35cc3f..bce84d2 100644<br>
--- a/src/vrend_renderer.c<br>
+++ b/src/vrend_renderer.c<br>
@@ -5405,6 +5405,20 @@ static int vrend_renderer_transfer_write_iov(struct vrend_context *ctx,<br>
x = info->box->x;<br>
y = invert ? (int)res->base.height0 - info->box->y - info->box->height : info->box->y;<br>
<br>
+<br>
+ /* mipmaps are usually passed in one iov, and we need to keep the offset<br>
+ * into the data in case we want to read back the data of a surface<br>
+ * that can not be rendered. Since we can not assume that the whole texture<br>
+ * is filled, we evaluate the offset for origin (0,0,0). Since it is also<br>
+ * possible that a resource is reused and resized update the offset every time.<br>
+ * We assume that level 0 has offset 0.<br>
+ */<br>
+ if (info->level> 0 && info->level <= MAX_READBACK_MIPMAP_LEVELS) {<br></blockquote><div><br></div><div><div>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.</div></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ int64_t level_height = u_minify(res->base.height0, info->level);<br>
+ res->mipmap_offsets[info->level-1] = info->offset -<br>
+ ((info->box->z * level_height + y) * stride + x * elsize);<br>
</blockquote><div><br></div><div>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:</div><div>info->offset == info->box->z * level_height + y * level_stride + x * elsize</div><div><br></div><div>So won't mipmap_offsets always be zero?</div><div><br></div><div>Also nit: parens would make more sense like this -- info->offset - (info->box->z * level_height + y * stride + x * elsize);</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ }<br>
+<br>
if (res->base.format == (enum pipe_format)VIRGL_FORMAT_Z24X8_UNORM) {<br>
/* we get values from the guest as 24-bit scaled integers<br>
but we give them to the host GL and it interprets them<br>
@@ -6088,8 +6102,6 @@ static void vrend_resource_copy_fallback(struct vrend_resource *src_res,<br>
<br>
box = *src_box;<br>
box.depth = vrend_get_texture_depth(src_res, src_level);<br>
-<br>
- src_stride = util_format_get_stride(src_res->base.format, src_res->base.width0);<br>
dst_stride = util_format_get_stride(dst_res->base.format, dst_res->base.width0);<br>
<br>
/* this is ugly need to do a full GetTexImage */<br>
@@ -6111,12 +6123,21 @@ static void vrend_resource_copy_fallback(struct vrend_resource *src_res,<br>
* iovec to have the data we need, otherwise we can use glGetTexture<br>
*/<br>
if (vrend_state.use_gles) {<br>
- read_transfer_data(&src_res->base, src_res->iov, src_res->num_iovs,<br>
- tptr, src_stride, &box, src_level, 0, false);<br>
+ uint64_t src_offset = 0;<br>
+ uint64_t dst_offset = 0;<br>
+ if (src_level > 0 && src_level <= MAX_READBACK_MIPMAP_LEVELS) {<br>
+ src_offset = src_res->mipmap_offsets[src_level-1];<br>
+ dst_offset = dst_res->mipmap_offsets[src_level-1];<br>
+ }<br>
+<br>
+ src_stride = util_format_get_nblocksx(src_res->base.format,<br>
+ u_minify(src_res->base.width0, src_level)) * elsize;<br>
+ read_transfer_data(&src_res->base, src_res->iov, src_res->num_iovs, tptr,<br>
+ src_stride, &box, src_level, src_offset, false);<br>
/* In on GLES Sync the iov that backs the dst resource because<br>
* we might need it in a chain copy A->B, B->C */<br>
write_transfer_data(&dst_res->base, dst_res->iov, dst_res->num_iovs, tptr,<br>
- dst_stride, &box, src_level, 0, false);<br>
+ dst_stride, &box, src_level, dst_offset, false);<br>
/* we get values from the guest as 24-bit scaled integers<br>
but we give them to the host GL and it interprets them<br>
as 32-bit scaled integers, so we need to scale them here */<br>
diff --git a/src/vrend_renderer.h b/src/vrend_renderer.h<br>
index 042d439..258c439 100644<br>
--- a/src/vrend_renderer.h<br>
+++ b/src/vrend_renderer.h<br>
@@ -44,6 +44,9 @@ struct virgl_gl_ctx_param {<br>
extern int vrend_dump_shaders;<br>
struct vrend_context;<br>
<br>
+/* Number of mipmap levels for which to keep the backing iov offsets */<br>
+#define MAX_READBACK_MIPMAP_LEVELS 16<br>
+<br>
struct vrend_resource {<br>
struct pipe_resource base;<br>
GLuint id;<br>
@@ -62,6 +65,7 @@ struct vrend_resource {<br>
char *ptr;<br>
struct iovec *iov;<br>
uint32_t num_iovs;<br>
+ uint64_t mipmap_offsets[MAX_READBACK_MIPMAP_LEVELS];<br>
};<br>
<br>
/* assume every format is sampler friendly */<br>
-- <br>
2.17.1<br>
<br>
_______________________________________________<br>
virglrenderer-devel mailing list<br>
<a href="mailto:virglrenderer-devel@lists.freedesktop.org" target="_blank">virglrenderer-devel@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel</a><br>
</blockquote></div></div></div>