[Mesa-dev] [PATCH 5/7] intel: Directly implement blit glBlitFramebuffer instead of awkward reuse.

Kenneth Graunke kenneth at whitecape.org
Wed Jun 5 23:05:45 PDT 2013


On 06/05/2013 10:14 AM, Eric Anholt wrote:
> This gets us support for blitting to attachment types other than
> textures.
> ---
>   src/mesa/drivers/dri/intel/intel_fbo.c      | 129 +++++++++++++++-------------
>   src/mesa/drivers/dri/intel/intel_tex.h      |   7 --
>   src/mesa/drivers/dri/intel/intel_tex_copy.c |   2 +-
>   3 files changed, 69 insertions(+), 69 deletions(-)

A couple of comments below...

> diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c b/src/mesa/drivers/dri/intel/intel_fbo.c
> index 9f892a9..9a24a55 100644
> --- a/src/mesa/drivers/dri/intel/intel_fbo.c
> +++ b/src/mesa/drivers/dri/intel/intel_fbo.c
> @@ -737,76 +737,83 @@ intel_validate_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb)
>    *         normal path.
>    */
>   static GLbitfield
> -intel_blit_framebuffer_copy_tex_sub_image(struct gl_context *ctx,
> -                                          GLint srcX0, GLint srcY0,
> -                                          GLint srcX1, GLint srcY1,
> -                                          GLint dstX0, GLint dstY0,
> -                                          GLint dstX1, GLint dstY1,
> -                                          GLbitfield mask, GLenum filter)
> +intel_blit_framebuffer_with_blitter(struct gl_context *ctx,
> +                                    GLint srcX0, GLint srcY0,
> +                                    GLint srcX1, GLint srcY1,
> +                                    GLint dstX0, GLint dstY0,
> +                                    GLint dstX1, GLint dstY1,
> +                                    GLbitfield mask, GLenum filter)
>   {
> +   struct intel_context *intel = intel_context(ctx);
> +
>      if (mask & GL_COLOR_BUFFER_BIT) {
>         GLint i;
>         const struct gl_framebuffer *drawFb = ctx->DrawBuffer;
>         const struct gl_framebuffer *readFb = ctx->ReadBuffer;
> -      const struct gl_renderbuffer_attachment *drawAtt;
> -      struct intel_renderbuffer *srcRb =
> -         intel_renderbuffer(readFb->_ColorReadBuffer);
> +      struct gl_renderbuffer *src_rb = readFb->_ColorReadBuffer;
> +      struct intel_renderbuffer *src_irb = intel_renderbuffer(src_rb);
> +
> +      if (!src_irb) {
> +         perf_debug("glBlitFramebuffer(): missing src renderbuffer.  "
> +                    "Falling back to software rendering.\n");
> +         return mask;
> +      }
>
>         /* If the source and destination are the same size with no mirroring,
>          * the rectangles are within the size of the texture and there is no
> -       * scissor then we can use glCopyTexSubimage2D to implement the blit.
> -       * This will end up as a fast hardware blit on some drivers.
> +       * scissor, then we can probably use the blit engine.
>          */
> -      const GLboolean use_intel_copy_texsubimage =
> -         srcX0 - srcX1 == dstX0 - dstX1 &&
> -         srcY0 - srcY1 == dstY0 - dstY1 &&
> -         srcX1 >= srcX0 &&
> -         srcY1 >= srcY0 &&
> -         srcX0 >= 0 && srcX1 <= readFb->Width &&
> -         srcY0 >= 0 && srcY1 <= readFb->Height &&
> -         dstX0 >= 0 && dstX1 <= drawFb->Width &&
> -         dstY0 >= 0 && dstY1 <= drawFb->Height &&
> -         !ctx->Scissor.Enabled;
> -
> -      /* Verify that all the draw buffers can be blitted using
> -       * intel_copy_texsubimage().
> +      if (!(srcX0 - srcX1 == dstX0 - dstX1 &&
> +            srcY0 - srcY1 == dstY0 - dstY1 &&
> +            srcX1 >= srcX0 &&
> +            srcY1 >= srcY0 &&
> +            srcX0 >= 0 && srcX1 <= readFb->Width &&
> +            srcY0 >= 0 && srcY1 <= readFb->Height &&
> +            dstX0 >= 0 && dstX1 <= drawFb->Width &&
> +            dstY0 >= 0 && dstY1 <= drawFb->Height &&
> +            !ctx->Scissor.Enabled)) {
> +         perf_debug("glBlitFramebuffer(): non-1:1 blit.  "
> +                    "Falling back to software rendering.\n");
> +         return mask;
> +      }
> +
> +      /* Blit to all active draw buffers.  We don't do any pre-checking,
> +       * because we assume that copying to MRTs is rare, and failure midway
> +       * through copying is even more rare.  Given that feedback loops in
> +       * glFramebufferBlit() are undefined, we can safely fail out after
> +       * having partially completed our copies.

The first part of this comment makes sense: yes, it's obviously rare.
The second part I feel could use a little more explanation:

If we fail midway through a blit, we return without clearing 
BUFFER_BITS_COLOR from the mask.  The caller will fall back to the next 
method (i.e. meta_BlitFramebuffer) and use that to blit /all/ RTs - even 
ones we already successfully blit.  Given that feedback loops in 
glFramebufferBlit() are undefined, we can safely fail out after having 
partially completed our copies.

Unless there's some kind of blending, it doesn't even seem 
undefined...meta should just overwrite the colors again, and everything 
will work out.

>          */
>         for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) {
> -         int idx = ctx->DrawBuffer->_ColorDrawBufferIndexes[i];
> -         if (idx == -1)
> -            continue;
> -         drawAtt = &drawFb->Attachment[idx];
> -
> -         if (srcRb && drawAtt && drawAtt->Texture &&
> -             use_intel_copy_texsubimage)
> -            continue;
> -         else
> +         struct gl_renderbuffer *dst_rb = ctx->DrawBuffer->_ColorDrawBuffers[i];
> +         struct intel_renderbuffer *dst_irb = intel_renderbuffer(dst_rb);
> +
> +         if (!dst_irb) {
> +            perf_debug("glBlitFramebuffer(): missing dst renderbuffer.  "
> +                       "Falling back to software rendering.\n");
>               return mask;
> -      }
> +         }
>
> -      /* Blit to all active draw buffers */
> -      for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) {
> -         int idx = ctx->DrawBuffer->_ColorDrawBufferIndexes[i];
> -         if (idx == -1)
> -            continue;
> -         drawAtt = &drawFb->Attachment[idx];
> -
> -         {
> -            const struct gl_texture_object *texObj = drawAtt->Texture;
> -            const GLuint dstLevel = drawAtt->TextureLevel;
> -            const GLenum target = texObj->Target;
> -
> -            struct gl_texture_image *texImage =
> -               _mesa_select_tex_image(ctx, texObj, target, dstLevel);
> -
> -            if (!intel_copy_texsubimage(intel_context(ctx),
> -                                        intel_texture_image(texImage),
> -                                        dstX0, dstY0,
> -                                        srcRb,
> -                                        srcX0, srcY0,
> -                                        srcX1 - srcX0, /* width */
> -                                        srcY1 - srcY0))
> -               return mask;
> +         gl_format src_format = _mesa_get_srgb_format_linear(src_rb->Format);
> +         gl_format dst_format = _mesa_get_srgb_format_linear(dst_rb->Format);
> +         if (src_format != dst_format) {
> +            perf_debug("glBlitFramebuffer(): unsupported blit from %s to %s.  "
> +                       "Falling back to software rendering.\n",
> +                       _mesa_get_format_name(src_format),
> +                       _mesa_get_format_name(dst_format));
> +            return mask;
> +         }
> +
> +         if (!intel_miptree_blit(intel,
> +                                 src_irb->mt,
> +                                 src_irb->mt_level, src_irb->mt_layer,
> +                                 srcX0, srcY0, src_rb->Name == 0,
> +                                 dst_irb->mt,
> +                                 dst_irb->mt_level, dst_irb->mt_layer,
> +                                 dstX0, dstY0, dst_rb->Name == 0,
> +                                 dstX1 - dstX0, dstY1 - dstY0, GL_COPY)) {
> +            perf_debug("glBlitFramebuffer(): unknown blit failure.  "
> +                       "Falling back to software rendering.\n");
> +            return mask;
>            }
>         }
>
> @@ -832,10 +839,10 @@ intel_blit_framebuffer(struct gl_context *ctx,
>   #endif
>
>      /* Try glCopyTexSubImage2D approach which uses the BLT. */

I'd change this to:
/* Try using the BLT engine. */

With those comment updates, this is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> -   mask = intel_blit_framebuffer_copy_tex_sub_image(ctx,
> -                                                    srcX0, srcY0, srcX1, srcY1,
> -                                                    dstX0, dstY0, dstX1, dstY1,
> -                                                    mask, filter);
> +   mask = intel_blit_framebuffer_with_blitter(ctx,
> +                                              srcX0, srcY0, srcX1, srcY1,
> +                                              dstX0, dstY0, dstX1, dstY1,
> +                                              mask, filter);
>      if (mask == 0x0)
>         return;
>
> diff --git a/src/mesa/drivers/dri/intel/intel_tex.h b/src/mesa/drivers/dri/intel/intel_tex.h
> index e038115..a08fdf4 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex.h
> +++ b/src/mesa/drivers/dri/intel/intel_tex.h
> @@ -68,13 +68,6 @@ bool
>   intel_tex_image_s8z24_create_renderbuffers(struct intel_context *intel,
>   					   struct intel_texture_image *image);
>
> -bool intel_copy_texsubimage(struct intel_context *intel,
> -                            struct intel_texture_image *intelImage,
> -                            GLint dstx, GLint dsty,
> -                            struct intel_renderbuffer *irb,
> -                            GLint x, GLint y,
> -                            GLsizei width, GLsizei height);
> -
>   bool
>   intel_texsubimage_tiled_memcpy(struct gl_context *ctx,
>                                  GLuint dims,
> diff --git a/src/mesa/drivers/dri/intel/intel_tex_copy.c b/src/mesa/drivers/dri/intel/intel_tex_copy.c
> index 4f01c58..f9e03fa 100644
> --- a/src/mesa/drivers/dri/intel/intel_tex_copy.c
> +++ b/src/mesa/drivers/dri/intel/intel_tex_copy.c
> @@ -48,7 +48,7 @@
>   #define FILE_DEBUG_FLAG DEBUG_TEXTURE
>
>
> -bool
> +static bool
>   intel_copy_texsubimage(struct intel_context *intel,
>                          struct intel_texture_image *intelImage,
>                          GLint dstx, GLint dsty,
>



More information about the mesa-dev mailing list