<div dir="ltr">On 5 June 2013 10:14, Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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); + } </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> _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. </blockquote><div> </div><div>I'm having trouble following this comment, especially the second sentence. Would this be clearer? </div><div>"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." </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"> */ 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; </blockquote><div> </div><div>Shouldn't the error check in error_check_subtexture_dimensions() prevent this from ever occurring? If so, I think this should be an assertion. </div><div> Both of these comments are minor, so in any case, this patch is </div><div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </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"> + + 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 <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>