[Mesa-dev] [PATCH 1/7] mesa: Hide weirdness of 1D_ARRAY textures from Driver.CopyTexSubImage().
Paul Berry
stereotype441 at gmail.com
Thu Jun 6 12:47:06 PDT 2013
On 5 June 2013 10:14, Eric Anholt <eric at anholt.net> 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.
>
I'm having trouble following this comment, especially the second sentence.
Would this be clearer?
"This function should copy a rectangular region in the rb to a single
destination slice, specified by @slice. In the case of 1D array textures
(where one GL call can potentially affect multiple destination slices),
core mesa takes care of calling this function multiple times, once for each
scanline to be copied."
> */
> 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;
>
Shouldn't the error check in error_check_subtexture_dimensions() prevent
this from ever occurring? If so, I think this should be an assertion.
Both of these comments are minor, so in any case, this patch is
Reviewed-by: Paul Berry <stereotype441 at gmail.com>
> +
> + ctx->Driver.CopyTexSubImage(ctx, 2, texImage,
> + xoffset, 0, yoffset + slice,
> + rb, x, y, width, 1);
> + }
> + } 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);
> }
>
> --
> 1.8.3.rc0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130606/abc89494/attachment-0001.html>
More information about the mesa-dev
mailing list