[Mesa-dev] [PATCH 3/3] swrast: Convert the simple glFramebufferBlit path to MapRenderbuffer.
Brian Paul
brian.e.paul at gmail.com
Tue Dec 13 13:22:49 PST 2011
On Tue, Dec 13, 2011 at 1:53 PM, Eric Anholt <eric at anholt.net> wrote:
> There is one significant functional change here: the simple blit now
> only handles exact matches of formats. From the
> ARB_framebuffer_object spec:
>
> "If the color formats of the read and draw framebuffers do not
> match, and <mask> includes COLOR_BUFFER_BIT, pixel groups are
> converted to match the destination format as in CopyPixels"
>
> The previous code would generally do this, unless the RB DataTypes
> didn't match, in which case it assertion failed. Rather than adding
> special-case format conversion code to the simple path, fall back to
> the slow paths which will need to handle format conversion as well.
> ---
> src/mesa/swrast/s_blit.c | 188 +++++++++++++++++++++++----------------------
> 1 files changed, 96 insertions(+), 92 deletions(-)
>
> diff --git a/src/mesa/swrast/s_blit.c b/src/mesa/swrast/s_blit.c
> index 803ad2e..6f10c3a 100644
> --- a/src/mesa/swrast/s_blit.c
> +++ b/src/mesa/swrast/s_blit.c
> @@ -454,7 +454,7 @@ blit_linear(struct gl_context *ctx,
> * Simple case: Blit color, depth or stencil with no scaling or flipping.
> * XXX we could easily support vertical flipping here.
> */
> -static void
> +static GLboolean
> simple_blit(struct gl_context *ctx,
> GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
> GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1,
> @@ -463,9 +463,10 @@ simple_blit(struct gl_context *ctx,
> struct gl_renderbuffer *readRb, *drawRb;
> const GLint width = srcX1 - srcX0;
> const GLint height = srcY1 - srcY0;
> - GLint row, srcY, dstY, yStep;
> - GLint comps, bytesPerRow;
> - void *rowBuffer;
> + GLint row;
> + GLint bytesPerRow, srcStride, dstStride;
> + void *temp = NULL;
> + GLubyte *srcMap, *dstMap;
>
> /* only one buffer */
> ASSERT(_mesa_bitcount(buffer) == 1);
> @@ -478,85 +479,93 @@ simple_blit(struct gl_context *ctx,
> ASSERT(srcX1 - srcX0 == dstX1 - dstX0);
> ASSERT(srcY1 - srcY0 == dstY1 - dstY0);
>
> - /* From the GL_ARB_framebuffer_object spec:
> - *
> - * "If the source and destination buffers are identical, and the source
> - * and destination rectangles overlap, the result of the blit operation
> - * is undefined."
> - *
> - * However, we provide the expected result anyway by flipping the order of
> - * the memcpy of rows.
> - */
> - if (srcY0 > dstY0) {
> - /* src above dst: copy bottom-to-top */
> - yStep = 1;
> - srcY = srcY0;
> - dstY = dstY0;
> - }
> - else {
> - /* src below dst: copy top-to-bottom */
> - yStep = -1;
> - srcY = srcY1 - 1;
> - dstY = dstY1 - 1;
> - }
> -
> switch (buffer) {
> case GL_COLOR_BUFFER_BIT:
> readRb = ctx->ReadBuffer->_ColorReadBuffer;
> drawRb = ctx->DrawBuffer->_ColorDrawBuffers[0];
> - comps = 4;
> break;
> case GL_DEPTH_BUFFER_BIT:
> - readRb = ctx->ReadBuffer->_DepthBuffer;
> - drawRb = ctx->DrawBuffer->_DepthBuffer;
> - comps = 1;
> + readRb = ctx->ReadBuffer->Attachment[BUFFER_DEPTH].Renderbuffer;
> + drawRb = ctx->DrawBuffer->Attachment[BUFFER_DEPTH].Renderbuffer;
> break;
> case GL_STENCIL_BUFFER_BIT:
> - readRb = ctx->ReadBuffer->_StencilBuffer;
> - drawRb = ctx->DrawBuffer->_StencilBuffer;
> - comps = 1;
> + readRb = ctx->ReadBuffer->Attachment[BUFFER_STENCIL].Renderbuffer;
> + drawRb = ctx->DrawBuffer->Attachment[BUFFER_STENCIL].Renderbuffer;
> break;
> default:
> _mesa_problem(ctx, "unexpected buffer in simple_blit()");
> - return;
> + return GL_TRUE;
> }
>
> - ASSERT(readRb->DataType == drawRb->DataType);
> + if (readRb->Format != drawRb->Format)
> + return GL_FALSE;
>
> - /* compute bytes per row */
> - switch (readRb->DataType) {
> - case GL_UNSIGNED_BYTE:
> - bytesPerRow = comps * width * sizeof(GLubyte);
> - break;
> - case GL_UNSIGNED_SHORT:
> - bytesPerRow = comps * width * sizeof(GLushort);
> - break;
> - case GL_UNSIGNED_INT:
> - bytesPerRow = comps * width * sizeof(GLuint);
> - break;
> - case GL_FLOAT:
> - bytesPerRow = comps * width * sizeof(GLfloat);
> - break;
> - default:
> - _mesa_problem(ctx, "unexpected buffer type in simple_blit");
> - return;
> + /* This path does direct memcpy of the data, and if the buffers are
> + * depth/stencil, it would end up stomping over the other one of depth or
> + * stencil even though it wasn't asked to.
> + */
> + if (_mesa_is_format_packed_depth_stencil(readRb->Format))
> + return GL_FALSE;
> +
> + bytesPerRow = _mesa_get_format_bytes(readRb->Format) * width;
> +
> + ctx->Driver.MapRenderbuffer(ctx, readRb, srcX0, srcY0, width, height,
> + GL_MAP_READ_BIT, &srcMap, &srcStride);
> + if (!srcMap) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBlitFrameBufferEXT");
> + return GL_TRUE;
> + }
> +
> + /* From the GL_ARB_framebuffer_object spec:
> + *
> + * "If the source and destination buffers are identical, and the source
> + * and destination rectangles overlap, the result of the blit operation
> + * is undefined."
> + *
> + * However, we provide the expected result anyway by copying to a temporary,
> + * since we can't do nested MapRenderbuffer()s on the same RB.
> + */
> + if (drawRb == readRb) {
> + temp = malloc(bytesPerRow * height);
> + if (!temp) {
> + ctx->Driver.UnmapRenderbuffer(ctx, readRb);
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBlitFrameBufferEXT");
> + return GL_TRUE;
> + }
> +
> + for (row = 0; row < height; row++) {
> + memcpy(temp + bytesPerRow * row, srcMap, bytesPerRow);
> +
> + srcMap += srcStride;
> + }
> +
> + ctx->Driver.UnmapRenderbuffer(ctx, readRb);
> + srcMap = temp;
> + srcStride = bytesPerRow;
> }
>
> - /* allocate the row buffer */
> - rowBuffer = malloc(bytesPerRow);
> - if (!rowBuffer) {
> + ctx->Driver.MapRenderbuffer(ctx, drawRb, dstX0, dstY0, width, height,
> + GL_MAP_WRITE_BIT, &dstMap, &dstStride);
> + if (!dstMap) {
> _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBlitFrameBufferEXT");
> - return;
> + return GL_TRUE;
> }
>
> for (row = 0; row < height; row++) {
> - readRb->GetRow(ctx, readRb, width, srcX0, srcY, rowBuffer);
> - drawRb->PutRow(ctx, drawRb, width, dstX0, dstY, rowBuffer, NULL);
> - srcY += yStep;
> - dstY += yStep;
> + memcpy(dstMap, srcMap, bytesPerRow);
> +
> + srcMap += srcStride;
> + dstMap += dstStride;
> + }
> +
> + if (temp) {
> + free(temp);
> + } else {
> + ctx->Driver.UnmapRenderbuffer(ctx, readRb);
> }
> + ctx->Driver.UnmapRenderbuffer(ctx, drawRb);
>
> - free(rowBuffer);
> + return GL_TRUE;
> }
>
>
> @@ -575,6 +584,7 @@ _swrast_BlitFramebuffer(struct gl_context *ctx,
> GL_STENCIL_BUFFER_BIT
> };
> GLint i;
> + GLboolean is_one_to_one;
>
> if (!_mesa_clip_blit(ctx, &srcX0, &srcY0, &srcX1, &srcY1,
> &dstX0, &dstY0, &dstX1, &dstY1)) {
> @@ -584,41 +594,35 @@ _swrast_BlitFramebuffer(struct gl_context *ctx,
> if (SWRAST_CONTEXT(ctx)->NewState)
> _swrast_validate_derived(ctx);
>
> - swrast_render_start(ctx);
> -
> - if (srcX1 - srcX0 == dstX1 - dstX0 &&
> - srcY1 - srcY0 == dstY1 - dstY0 &&
> - srcX0 < srcX1 &&
> - srcY0 < srcY1 &&
> - dstX0 < dstX1 &&
> - dstY0 < dstY1) {
> - /* no stretching or flipping.
> - * filter doesn't matter.
> - */
> + is_one_to_one = (srcX1 - srcX0 == dstX1 - dstX0 &&
> + srcY1 - srcY0 == dstY1 - dstY0 &&
> + srcX0 < srcX1 &&
> + srcY0 < srcY1 &&
> + dstX0 < dstX1 &&
> + dstY0 < dstY1);
> +
> + if (filter == GL_NEAREST) {
> for (i = 0; i < 3; i++) {
> - if (mask & buffers[i]) {
> - simple_blit(ctx, srcX0, srcY0, srcX1, srcY1,
> - dstX0, dstY0, dstX1, dstY1, buffers[i]);
> - }
> + if (mask & buffers[i]) {
> + if (is_one_to_one && !simple_blit(ctx,
> + srcX0, srcY0, srcX1, srcY1,
> + dstX0, dstY0, dstX1, dstY1,
> + buffers[i])) {
> + swrast_render_start(ctx);
> + blit_nearest(ctx, srcX0, srcY0, srcX1, srcY1,
> + dstX0, dstY0, dstX1, dstY1, buffers[i]);
> + swrast_render_finish(ctx);
> + }
> + }
> }
> }
> else {
> - if (filter == GL_NEAREST) {
> - for (i = 0; i < 3; i++) {
> - if (mask & buffers[i]) {
> - blit_nearest(ctx, srcX0, srcY0, srcX1, srcY1,
> - dstX0, dstY0, dstX1, dstY1, buffers[i]);
> - }
> - }
> - }
> - else {
> - ASSERT(filter == GL_LINEAR);
> - if (mask & GL_COLOR_BUFFER_BIT) { /* depth/stencil not allowed */
> - blit_linear(ctx, srcX0, srcY0, srcX1, srcY1,
> - dstX0, dstY0, dstX1, dstY1);
> - }
> + ASSERT(filter == GL_LINEAR);
> + if (mask & GL_COLOR_BUFFER_BIT) { /* depth/stencil not allowed */
> + swrast_render_start(ctx);
> + blit_linear(ctx, srcX0, srcY0, srcX1, srcY1,
> + dstX0, dstY0, dstX1, dstY1);
> + swrast_render_finish(ctx);
> }
> }
> -
> - swrast_render_finish(ctx);
> }
Looks good, but I think guts of simple_blit() are an awful lot like
fast_copy_pixels() in s_copypix.c. I think we could refactor a little
bit and have one function for both cases.
For the srcRb==dstRb case of glCopyPixels I just map the whole rb
READ/WRITE and do memcpy() without a temporary image.
-Brian
More information about the mesa-dev
mailing list