[Mesa-dev] [PATCH 1/7] mesa: Hide weirdness of 1D_ARRAY textures from Driver.CopyTexSubImage().
Brian Paul
brianp at vmware.com
Thu Jun 6 12:56:07 PDT 2013
On 06/05/2013 10:14 AM, Eric Anholt wrote:
> Intel had brokenness here, and I'd like to continue moving Mesa toward
> hiding 1D_ARRAY's ridiculousness inside of the core, like we did with
> MapTextureImage. Fixes copyteximage 1D_ARRAY on intel.
>
> There's still an impedance mismatch in meta when falling back to read and
> texsubimage, since texsubimage expects coordinates into 1D_ARRAY as
> (width, slice, 0) instead of (width, 0, slice).
> ---
> src/mesa/drivers/common/meta.c | 13 ++++++--
> src/mesa/drivers/common/meta.h | 2 +-
> src/mesa/drivers/dri/intel/intel_tex_copy.c | 7 +++--
> src/mesa/drivers/dri/radeon/radeon_tex_copy.c | 6 ++--
> src/mesa/main/dd.h | 7 ++++-
> src/mesa/main/teximage.c | 42 ++++++++++++++++++++++----
> src/mesa/state_tracker/st_cb_texture.c | 43 +++++----------------------
> 7 files changed, 68 insertions(+), 52 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index 1250bd3..2bf42c0 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -3851,9 +3851,16 @@ _mesa_meta_CopyTexSubImage(struct gl_context *ctx, GLuint dims,
> */
> _mesa_meta_begin(ctx, MESA_META_PIXEL_STORE);
>
> - ctx->Driver.TexSubImage(ctx, dims, texImage,
> - xoffset, yoffset, zoffset, width, height, 1,
> - format, type, buf, &ctx->Unpack);
> + if (texImage->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
> + assert(yoffset == 0);
> + ctx->Driver.TexSubImage(ctx, dims, texImage,
> + xoffset, zoffset, 0, width, 1, 1,
> + format, type, buf, &ctx->Unpack);
> + } else {
> + ctx->Driver.TexSubImage(ctx, dims, texImage,
> + xoffset, yoffset, zoffset, width, height, 1,
> + format, type, buf, &ctx->Unpack);
> + }
>
> _mesa_meta_end(ctx);
>
> diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
> index 823be14..1639e4a 100644
> --- a/src/mesa/drivers/common/meta.h
> +++ b/src/mesa/drivers/common/meta.h
> @@ -117,7 +117,7 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum target,
> extern void
> _mesa_meta_CopyTexSubImage(struct gl_context *ctx, GLuint dims,
> struct gl_texture_image *texImage,
> - GLint xoffset, GLint yoffset, GLint zoffset,
> + GLint xoffset, GLint yoffset, GLint slice,
> struct gl_renderbuffer *rb,
> GLint x, GLint y,
> GLsizei width, GLsizei height);
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_copy.c b/src/mesa/drivers/dri/intel/intel_tex_copy.c
> index d8e65ba..3ab66d9 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_copy.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_copy.c
> @@ -135,13 +135,14 @@ intel_copy_texsubimage(struct intel_context *intel,
> static void
> intelCopyTexSubImage(struct gl_context *ctx, GLuint dims,
> struct gl_texture_image *texImage,
> - GLint xoffset, GLint yoffset, GLint zoffset,
> + GLint xoffset, GLint yoffset, GLint slice,
> struct gl_renderbuffer *rb,
> GLint x, GLint y,
> GLsizei width, GLsizei height)
> {
> struct intel_context *intel = intel_context(ctx);
> - if (dims != 3) {
> +
> + if (slice == 0) {
> #ifndef I915
> /* Try BLORP first. It can handle almost everything. */
> if (brw_blorp_copytexsubimage(intel, rb, texImage, x, y,
> @@ -160,7 +161,7 @@ intelCopyTexSubImage(struct gl_context *ctx, GLuint dims,
> /* Finally, fall back to meta. This will likely be slow. */
> perf_debug("%s - fallback to swrast\n", __FUNCTION__);
> _mesa_meta_CopyTexSubImage(ctx, dims, texImage,
> - xoffset, yoffset, zoffset,
> + xoffset, yoffset, slice,
> rb, x, y, width, height);
> }
>
> diff --git a/src/mesa/drivers/dri/radeon/radeon_tex_copy.c b/src/mesa/drivers/dri/radeon/radeon_tex_copy.c
> index a6c406b..675eb78 100644
> --- a/src/mesa/drivers/dri/radeon/radeon_tex_copy.c
> +++ b/src/mesa/drivers/dri/radeon/radeon_tex_copy.c
> @@ -136,7 +136,7 @@ do_copy_texsubimage(struct gl_context *ctx,
> void
> radeonCopyTexSubImage(struct gl_context *ctx, GLuint dims,
> struct gl_texture_image *texImage,
> - GLint xoffset, GLint yoffset, GLint zoffset,
> + GLint xoffset, GLint yoffset, GLint slice,
> struct gl_renderbuffer *rb,
> GLint x, GLint y,
> GLsizei width, GLsizei height)
> @@ -144,7 +144,7 @@ radeonCopyTexSubImage(struct gl_context *ctx, GLuint dims,
> radeonContextPtr radeon = RADEON_CONTEXT(ctx);
> radeon_prepare_render(radeon);
>
> - if (dims != 2 || !do_copy_texsubimage(ctx,
> + if (slice != 0 || !do_copy_texsubimage(ctx,
> radeon_tex_obj(texImage->TexObject),
> (radeon_texture_image *)texImage,
> xoffset, yoffset,
> @@ -154,7 +154,7 @@ radeonCopyTexSubImage(struct gl_context *ctx, GLuint dims,
> "Falling back to sw for glCopyTexSubImage2D\n");
>
> _mesa_meta_CopyTexSubImage(ctx, dims, texImage,
> - xoffset, yoffset, zoffset,
> + xoffset, yoffset, slice,
> rb, x, y, width, height);
> }
> }
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 13c7a83..846d5d0 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -249,10 +249,15 @@ struct dd_function_table {
>
> /**
> * Called by glCopyTex[Sub]Image[123]D().
> + *
> + * In the case of 1D array textures, the driver will be called to copy each
> + * appropriate scanline from the rb to each destination slice. For 3D or
> + * other array textures, only one slice may be copied, but @slice may be
> + * nonzero.
> */
> void (*CopyTexSubImage)(struct gl_context *ctx, GLuint dims,
> struct gl_texture_image *texImage,
> - GLint xoffset, GLint yoffset, GLint zoffset,
> + GLint xoffset, GLint yoffset, GLint slice,
> struct gl_renderbuffer *rb,
> GLint x, GLint y,
> GLsizei width, GLsizei height);
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 9aaa63f..edd8019 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -3428,6 +3428,37 @@ get_copy_tex_image_source(struct gl_context *ctx, gl_format texFormat)
> }
> }
>
> +static void
> +copytexsubimage_by_slice(struct gl_context *ctx,
> + struct gl_texture_image *texImage,
> + GLuint dims,
> + GLint xoffset, GLint yoffset, GLint zoffset,
> + struct gl_renderbuffer *rb,
> + GLint x, GLint y,
> + GLsizei width, GLsizei height)
> +{
> + if (texImage->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
> + int slice;
> +
> + /* For 1D arrays, we copy each scanline of the source rectangle into the
> + * next array slice.
> + */
> + assert(zoffset == 0);
> +
> + for (slice = 0; slice < height; slice++) {
> + if (yoffset + slice >= texImage->Height)
> + break;
> +
> + ctx->Driver.CopyTexSubImage(ctx, 2, texImage,
> + xoffset, 0, yoffset + slice,
> + rb, x, y, width, 1);
Should that be 'y + slice'? Otherwise I think we're always copying from
the same Y position.
> + }
> + } else {
> + ctx->Driver.CopyTexSubImage(ctx, dims, texImage,
> + xoffset, yoffset, zoffset,
> + rb, x, y, width, height);
> + }
> +}
>
>
> /**
> @@ -3516,8 +3547,9 @@ copyteximage(struct gl_context *ctx, GLuint dims,
> struct gl_renderbuffer *srcRb =
> get_copy_tex_image_source(ctx, texImage->TexFormat);
>
> - ctx->Driver.CopyTexSubImage(ctx, dims, texImage, dstX, dstY, dstZ,
> - srcRb, srcX, srcY, width, height);
> + copytexsubimage_by_slice(ctx, texImage, dims,
> + dstX, dstY, dstZ,
> + srcRb, srcX, srcY, width, height);
> }
>
> check_gen_mipmap(ctx, target, texObj, level);
> @@ -3609,9 +3641,9 @@ copytexsubimage(struct gl_context *ctx, GLuint dims, GLenum target, GLint level,
> struct gl_renderbuffer *srcRb =
> get_copy_tex_image_source(ctx, texImage->TexFormat);
>
> - ctx->Driver.CopyTexSubImage(ctx, dims, texImage,
> - xoffset, yoffset, zoffset,
> - srcRb, x, y, width, height);
> + copytexsubimage_by_slice(ctx, texImage, dims,
> + xoffset, yoffset, zoffset,
> + srcRb, x, y, width, height);
>
> check_gen_mipmap(ctx, target, texObj, level);
>
> diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
> index 56dbe85..68c334e 100644
> --- a/src/mesa/state_tracker/st_cb_texture.c
> +++ b/src/mesa/state_tracker/st_cb_texture.c
> @@ -1133,7 +1133,7 @@ fallback_copy_texsubimage(struct gl_context *ctx,
> struct st_renderbuffer *strb,
> struct st_texture_image *stImage,
> GLenum baseFormat,
> - GLint destX, GLint destY, GLint destZ,
> + GLint destX, GLint destY, GLint slice,
> GLint srcX, GLint srcY,
> GLsizei width, GLsizei height)
> {
> @@ -1154,14 +1154,6 @@ fallback_copy_texsubimage(struct gl_context *ctx,
> srcY = strb->Base.Height - srcY - height;
> }
>
> - if (stImage->pt->target == PIPE_TEXTURE_1D_ARRAY) {
> - /* Move y/height to z/depth for 1D array textures. */
> - destZ = destY;
> - destY = 0;
> - dst_depth = dst_height;
> - dst_height = 1;
> - }
> -
> map = pipe_transfer_map(pipe,
> strb->texture,
> strb->rtt_level,
> @@ -1178,7 +1170,7 @@ fallback_copy_texsubimage(struct gl_context *ctx,
> transfer_usage = PIPE_TRANSFER_WRITE;
>
> texDest = st_texture_image_map(st, stImage, transfer_usage,
> - destX, destY, destZ,
> + destX, destY, slice,
> dst_width, dst_height, dst_depth);
>
> if (baseFormat == GL_DEPTH_COMPONENT ||
> @@ -1292,7 +1284,7 @@ fallback_copy_texsubimage(struct gl_context *ctx,
> static void
> st_CopyTexSubImage(struct gl_context *ctx, GLuint dims,
> struct gl_texture_image *texImage,
> - GLint destX, GLint destY, GLint destZ,
> + GLint destX, GLint destY, GLint slice,
> struct gl_renderbuffer *rb,
> GLint srcX, GLint srcY, GLsizei width, GLsizei height)
> {
> @@ -1306,7 +1298,7 @@ st_CopyTexSubImage(struct gl_context *ctx, GLuint dims,
> enum pipe_format dst_format;
> GLboolean do_flip = (st_fb_orientation(ctx->ReadBuffer) == Y_0_TOP);
> unsigned bind;
> - GLint srcY0, srcY1, yStep;
> + GLint srcY0, srcY1;
>
> if (!strb || !strb->surface || !stImage->pt) {
> debug_printf("%s: null strb or stImage\n", __FUNCTION__);
> @@ -1351,12 +1343,10 @@ st_CopyTexSubImage(struct gl_context *ctx, GLuint dims,
> if (do_flip) {
> srcY1 = strb->Base.Height - srcY - height;
> srcY0 = srcY1 + height;
> - yStep = -1;
> }
> else {
> srcY0 = srcY;
> srcY1 = srcY0 + height;
> - yStep = 1;
> }
>
> /* Blit the texture.
> @@ -1377,39 +1367,20 @@ st_CopyTexSubImage(struct gl_context *ctx, GLuint dims,
> blit.dst.level = stObj->pt != stImage->pt ? 0 : texImage->Level;
> blit.dst.box.x = destX;
> blit.dst.box.y = destY;
> - blit.dst.box.z = stImage->base.Face + destZ;
> + blit.dst.box.z = stImage->base.Face + slice;
> blit.dst.box.width = width;
> blit.dst.box.height = height;
> blit.dst.box.depth = 1;
> blit.mask = st_get_blit_mask(rb->_BaseFormat, texImage->_BaseFormat);
> blit.filter = PIPE_TEX_FILTER_NEAREST;
> -
> - /* 1D array textures need special treatment.
> - * Blit rows from the source to layers in the destination. */
> - if (texImage->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
> - int y, layer;
> -
> - for (y = srcY0, layer = 0; layer < height; y += yStep, layer++) {
> - blit.src.box.y = y;
> - blit.src.box.height = 1;
> - blit.dst.box.y = 0;
> - blit.dst.box.height = 1;
> - blit.dst.box.z = destY + layer;
> -
> - pipe->blit(pipe, &blit);
> - }
> - }
> - else {
> - /* All the other texture targets. */
> - pipe->blit(pipe, &blit);
> - }
> + pipe->blit(pipe, &blit);
> return;
>
> fallback:
> /* software fallback */
> fallback_copy_texsubimage(ctx,
> strb, stImage, texImage->_BaseFormat,
> - destX, destY, destZ,
> + destX, destY, slice,
> srcX, srcY, width, height);
> }
Thanks for updating the state tracker code. You removed the code above
on the premise that height will always be 1 if we're copying to a 1D
array texture, right? Maybe we should assert that just to be safe.
Reviewed-by: Brian Paul <brianp at vmware.com>
More information about the mesa-dev
mailing list