[Mesa-dev] [PATCH 07/10] meta: Add an accelerated glCopyTexSubImage using glBlitFramebuffer.
Kenneth Graunke
kenneth at whitecape.org
Mon Mar 10 00:29:09 PDT 2014
On 03/04/2014 02:17 PM, Eric Anholt wrote:
> We have to be a little careful here, because BlitFB itself has a fallback
> path that calls CopyTexSubImage(), so we reach over into its state to tell
> it to skip that case.
Right...to expand on this...
Mesa enables GL_EXT_framebuffer_blit universally, so all drivers support
BlitFramebuffer.
But blitframebuffer_texture cannot blit from renderbuffers without
BindRenderbufferTexImage, which is new and not supported by most
drivers. (It may also require ARB_framebuffer_object? Not sure...) So
drivers like i915/r100/r200/nouveau_vieux need the
BlitFB-via-CopyTexSubImage fallback.
The existing CopyTexImage support doesn't handle MSAA. Adding the
CopyTexSubImage-via-BlitFB path adds support for that, while leveraging
existing code. i965 should be able to handle everything via
blitframebuffer_texture; other drivers don't handle MSAA so the existing
copytexsubimage code should be fine.
It's pretty confusing trying to keep track of which paths implement
which subset of features, and which drivers have the extensions required
to use various paths in various cases. I don't know if we can refactor
the extension checking code to clarify this, or add more comments, or
what, but something would be nice. Not saying you have to do that here;
just adding to the "future work" section. :)
> ---
> src/mesa/drivers/common/meta.c | 177 ++++++++++++++++++++++++++----------
> src/mesa/drivers/common/meta.h | 1 +
> src/mesa/drivers/common/meta_blit.c | 3 +-
> 3 files changed, 130 insertions(+), 51 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index 24eb0a3..90e49fd 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -37,6 +37,7 @@
> #include "main/arbprogram.h"
> #include "main/arrayobj.h"
> #include "main/blend.h"
> +#include "main/blit.h"
> #include "main/bufferobj.h"
> #include "main/buffers.h"
> #include "main/colortab.h"
> @@ -93,6 +94,41 @@ static void meta_glsl_generate_mipmap_cleanup(struct gen_mipmap_state *mipmap);
> static void meta_decompress_cleanup(struct decompress_state *decompress);
> static void meta_drawpix_cleanup(struct drawpix_state *drawpix);
>
> +static void
> +meta_framebuffer_texture_layer(GLenum fb_target,
> + GLenum attachment,
> + const struct gl_texture_image *texImage,
> + int layer)
> +{
> + struct gl_texture_object *texObj = texImage->TexObject;
> + int level = texImage->Level;
> +
> + switch (texObj->Target) {
> + case GL_TEXTURE_1D:
> + _mesa_FramebufferTexture1D(fb_target, attachment,
> + texObj->Target, texObj->Name, level);
> + break;
> + case GL_TEXTURE_1D_ARRAY:
> + case GL_TEXTURE_2D_ARRAY:
> + case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
> + case GL_TEXTURE_CUBE_MAP_ARRAY:
> + case GL_TEXTURE_3D:
> + _mesa_FramebufferTextureLayer(fb_target, attachment,
> + texObj->Name, level, layer);
> + break;
> + case GL_TEXTURE_CUBE_MAP:
> + _mesa_FramebufferTexture2D(fb_target, attachment,
> + (GL_TEXTURE_CUBE_MAP_POSITIVE_X +
> + texImage->Face),
> + texObj->Name, level);
> + break;
> + default:
> + _mesa_FramebufferTexture2D(fb_target, attachment,
> + texObj->Target, texObj->Name, level);
> + break;
> + }
> +}
Gah, sorry...I didn't see your email about my bind_fbo_image hook until
after pushing. We should definitely have one function. I see mine
takes texObject and cube face enums, while yours takes a texImage and
infers the face. I guess mine requires translating the cube enums, and
yours requires calling _mesa_select_tex_image. Not sure which is
better...whatever you want to do here is fine.
> +
> GLuint
> _mesa_meta_compile_shader_with_debug(struct gl_context *ctx, GLenum target,
> const GLcharARB *source)
> @@ -2436,26 +2472,8 @@ _mesa_meta_check_generate_mipmap_fallback(struct gl_context *ctx, GLenum target,
> _mesa_GenFramebuffers(1, &mipmap->FBO);
> _mesa_BindFramebuffer(GL_FRAMEBUFFER_EXT, mipmap->FBO);
>
> - if (target == GL_TEXTURE_1D) {
> - _mesa_FramebufferTexture1D(GL_FRAMEBUFFER_EXT,
> - GL_COLOR_ATTACHMENT0_EXT,
> - target, texObj->Name, srcLevel);
> - }
> -#if 0
> - /* other work is needed to enable 3D mipmap generation */
> - else if (target == GL_TEXTURE_3D) {
> - GLint zoffset = 0;
> - _mesa_FramebufferTexture3D(GL_FRAMEBUFFER_EXT,
> - GL_COLOR_ATTACHMENT0_EXT,
> - target, texObj->Name, srcLevel, zoffset);
> - }
> -#endif
> - else {
> - /* 2D / cube */
> - _mesa_FramebufferTexture2D(GL_FRAMEBUFFER_EXT,
> - GL_COLOR_ATTACHMENT0_EXT,
> - target, texObj->Name, srcLevel);
> - }
> + meta_framebuffer_texture_layer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
> + baseImage, 0);
>
> status = _mesa_CheckFramebufferStatus(GL_FRAMEBUFFER_EXT);
>
> @@ -2848,7 +2866,7 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum target,
> _mesa_unlock_texture(ctx, texObj);
>
> for (dstLevel = baseLevel + 1; dstLevel <= maxLevel; dstLevel++) {
> - const struct gl_texture_image *srcImage;
> + const struct gl_texture_image *srcImage, *dstImage;
> const GLuint srcLevel = dstLevel - 1;
> GLsizei srcWidth, srcHeight, srcDepth;
> GLsizei dstWidth, dstHeight, dstDepth;
> @@ -2889,34 +2907,13 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum target,
> */
> break;
> }
> + dstImage = _mesa_select_tex_image(ctx, texObj, faceTarget, dstLevel);
>
> /* limit minification to src level */
> _mesa_TexParameteri(target, GL_TEXTURE_MAX_LEVEL, srcLevel);
>
> - /* Set to draw into the current dstLevel */
> - if (target == GL_TEXTURE_1D) {
> - _mesa_FramebufferTexture1D(GL_FRAMEBUFFER_EXT,
> - GL_COLOR_ATTACHMENT0_EXT,
> - target,
> - texObj->Name,
> - dstLevel);
> - }
> - else if (target == GL_TEXTURE_3D) {
> - GLint zoffset = 0; /* XXX unfinished */
> - _mesa_FramebufferTexture3D(GL_FRAMEBUFFER_EXT,
> - GL_COLOR_ATTACHMENT0_EXT,
> - target,
> - texObj->Name,
> - dstLevel, zoffset);
> - }
> - else {
> - /* 2D / cube */
> - _mesa_FramebufferTexture2D(GL_FRAMEBUFFER_EXT,
> - GL_COLOR_ATTACHMENT0_EXT,
> - faceTarget,
> - texObj->Name,
> - dstLevel);
> - }
> + meta_framebuffer_texture_layer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
> + dstImage, 0);
>
> _mesa_DrawBuffer(GL_COLOR_ATTACHMENT0_EXT);
>
> @@ -3001,6 +2998,83 @@ get_temp_image_type(struct gl_context *ctx, mesa_format format)
> }
>
> /**
> + * Attempts to wrap the destination texture in an FBO and use
> + * glBlitFramebuffer() to implement glCopyTexSubImage().
> + */
> +static bool
> +copytexsubimage_using_blit_framebuffer(struct gl_context *ctx, GLuint dims,
> + struct gl_texture_image *texImage,
> + GLint xoffset,
> + GLint yoffset,
> + GLint zoffset,
> + struct gl_renderbuffer *rb,
> + GLint x, GLint y,
> + GLsizei width, GLsizei height)
> +{
> + struct gl_texture_object *texObj = texImage->TexObject;
> + GLuint fbo;
> + bool success = false;
> + GLbitfield mask;
> + GLenum status;
> +
> + if (!ctx->Extensions.ARB_framebuffer_object)
> + return false;
> +
> + _mesa_unlock_texture(ctx, texObj);
> +
> + _mesa_meta_begin(ctx, MESA_META_ALL);
> +
> + _mesa_GenFramebuffers(1, &fbo);
> + _mesa_BindFramebuffer(GL_DRAW_FRAMEBUFFER, fbo);
> +
> + if (rb->_BaseFormat == GL_DEPTH_STENCIL ||
> + rb->_BaseFormat == GL_DEPTH_COMPONENT) {
> + meta_framebuffer_texture_layer(GL_DRAW_FRAMEBUFFER, GL_DEPTH_ATTACHMENT,
> + texImage, zoffset);
> + mask = GL_DEPTH_BUFFER_BIT;
> +
> + if (rb->_BaseFormat == GL_DEPTH_STENCIL &&
> + texImage->_BaseFormat == GL_DEPTH_STENCIL) {
> + meta_framebuffer_texture_layer(GL_DRAW_FRAMEBUFFER,
> + GL_STENCIL_ATTACHMENT,
> + texImage, zoffset);
> + mask |= GL_STENCIL_BUFFER_BIT;
> + }
> + _mesa_DrawBuffer(GL_NONE);
> + } else {
> + meta_framebuffer_texture_layer(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
> + texImage, zoffset);
> + mask = GL_COLOR_BUFFER_BIT;
> + _mesa_DrawBuffer(GL_COLOR_ATTACHMENT0_EXT);
> + }
> +
> + status = _mesa_CheckFramebufferStatus(GL_DRAW_FRAMEBUFFER);
> + if (status != GL_FRAMEBUFFER_COMPLETE)
> + goto out;
> +
> + ctx->Meta->Blit.no_ctsi_fallback = true;
> + /* Update clip state. */
> + _mesa_update_state(ctx);
Maybe I'm blind, but what clip state? I don't see any code here or in
BlitFramebuffer altering that, other than meta_begin. I'm a bit
concerned that either a) this is unnecessary, or b) we need it more places.
> + /* We skip the core BlitFramebuffer checks for format consistency, which
> + * are too strict for CopyTexImage. We know meta will be fine with format
> + * changes.
> + */
> + _mesa_meta_BlitFramebuffer(ctx, x, y,
> + x + width, y + height,
> + xoffset, yoffset,
> + xoffset + width, yoffset + height,
> + mask, GL_NEAREST);
> + ctx->Meta->Blit.no_ctsi_fallback = false;
> + success = true;
> +
> + out:
> + _mesa_lock_texture(ctx, texObj);
> + _mesa_DeleteFramebuffers(1, &fbo);
> + _mesa_meta_end(ctx);
> + return success;
> +}
> +
> +/**
> * Helper for _mesa_meta_CopyTexSubImage1/2/3D() functions.
> * Have to be careful with locking and meta state for pixel transfer.
> */
> @@ -3017,11 +3091,14 @@ _mesa_meta_CopyTexSubImage(struct gl_context *ctx, GLuint dims,
> GLint bpp;
> void *buf;
>
> - /* The gl_renderbuffer is part of the interface for
> - * dd_function_table::CopyTexSubImage, but this implementation does not use
> - * it.
> - */
> - (void) rb;
> + if (copytexsubimage_using_blit_framebuffer(ctx, dims,
> + texImage,
> + xoffset, yoffset, zoffset,
> + rb,
> + x, y,
> + width, height)) {
> + return;
> + }
>
> /* Choose format/type for temporary image buffer */
> format = _mesa_get_format_base_format(texImage->TexFormat);
> diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
> index 104849a..00553e2 100644
> --- a/src/mesa/drivers/common/meta.h
> +++ b/src/mesa/drivers/common/meta.h
> @@ -252,6 +252,7 @@ struct blit_state
> struct blit_shader_table shaders;
> GLuint msaa_shaders[BLIT_MSAA_SHADER_COUNT];
> struct temp_texture depthTex;
> + bool no_ctsi_fallback;
> };
>
>
> diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
> index 907c2cd..9756d86 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -701,7 +701,8 @@ _mesa_meta_BlitFramebuffer(struct gl_context *ctx,
> if (!use_glsl_version)
> _mesa_set_enable(ctx, tex->Target, GL_TRUE);
>
> - if (mask & GL_COLOR_BUFFER_BIT) {
> + if ((mask & GL_COLOR_BUFFER_BIT) &&
> + !ctx->Meta->Blit.no_ctsi_fallback) {
> const struct gl_framebuffer *readFb = ctx->ReadBuffer;
> const struct gl_renderbuffer *colorReadRb = readFb->_ColorReadBuffer;
> const GLenum rb_base_format =
>
I was going to suggest an early return, but I guess the
non-blitframebuffer_texture GL_DEPTH_BUFFER_BIT support doesn't actually
use CTSI...so we're probably fine.
Other than my concern about clipping, this looks good to me. Sorry for
making you rebase...
--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140310/e4eb4efd/attachment.pgp>
More information about the mesa-dev
mailing list