On 12/17/2012 07:35 PM, Anuj Phogat wrote:
This patch fixes a case when blitting to a framebuffer with
renderbuffers/textures attached to GL_COLOR_ATTACHMENT{1, 2, ...}.
Earlier we were incorrectly blitting to GL_COLOR_ATTACHMENT0 by default.
It also fixes a blitting case when drawAttachment->Texture ==
readAttachment->Texture. This was causing an assertion failure in intel's
i965 drivers (intel_miptree_attach_map()) with gles3 conformance test case:
framebuffer_blit_functionality_minifying_blit
V2: Fixed a case when number of draw buffer attachments are zero.
Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
---
src/mesa/main/fbobject.c | 14 +++++++++++++-
src/mesa/swrast/s_blit.c | 33 ++++++++++++++++++++++++++-------
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index d87239e..f4e427b 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -2818,8 +2818,20 @@ _mesa_BlitFramebuffer(GLint srcX0, GLint srcY0, GLint
srcX1, GLint srcY1,
/* get color read/draw renderbuffers */
if (mask & GL_COLOR_BUFFER_BIT) {
+ const GLuint numColorDrawBuffers =
+ ctx->DrawBuffer->_NumColorDrawBuffers;
colorReadRb = readFb->_ColorReadBuffer;
- colorDrawRb = drawFb->_ColorDrawBuffers[0];
+
+ if (numColorDrawBuffers > 0) {
+ for (int i = 0; i < numColorDrawBuffers; i++) {
+ if (ctx->DrawBuffer->_ColorDrawBuffers[i] != NULL) {
+ colorDrawRb = ctx->DrawBuffer->_ColorDrawBuffers[i];
+ break;
+ }
+ }
+ }
+ else
+ colorDrawRb = NULL;
This looks wrong to me. You're looping through all the draw buffers,
advancing the pointer until you find the first drawbuffer, then
stopping...but you're not doing any checks.
I believe you actually want to check the read buffer against *each* of
the draw buffers, i.e.
colorReadRb = readFb->_ColorReadBuffer;
for (int i = 0; i < numColorDrawBuffers; i++) {
if (ctx->DrawBuffer->_ColorDrawBuffers[i] == NULL) {
continue;
/* From the EXT_framebuffer_object spec: ... */
if (colorReadRb == NULL || colorDrawRb == NULL) {
colorReadRb = colorDrawRb = NULL;
mask &= ~GL_COLOR_BUFFER_BIT;
break;
} else if (!compatible_color_datatypes(...)) {
...error...
return;
}
}
}
It looks like the compatible_resolve_formats check (below) should also
be in this loop, so it happens on a per-buffer basis.
I believe my interpretation of checking the read buffer against *all*
color draw buffers is correct. For example, this spec text:
"The read buffer contains signed integer values and **any** draw buffer
does not contain signed integer values."
...really seems to imply that we should be applying these error checks
to each of the draw buffers in turn.
/* From the EXT_framebuffer_object spec:
*
diff --git a/src/mesa/swrast/s_blit.c b/src/mesa/swrast/s_blit.c
index b0c56a4..c579572 100644
--- a/src/mesa/swrast/s_blit.c
+++ b/src/mesa/swrast/s_blit.c
@@ -111,6 +111,9 @@ blit_nearest(struct gl_context *ctx,
GLbitfield buffer)
{
struct gl_renderbuffer *readRb, *drawRb;
+ struct gl_renderbuffer_attachment *readAtt, *drawAtt;
+ struct gl_framebuffer *readFb = ctx->ReadBuffer;
+ struct gl_framebuffer *drawFb = ctx->DrawBuffer;
const GLint srcWidth = ABS(srcX1 - srcX0);
const GLint dstWidth = ABS(dstX1 - dstX0);
@@ -146,8 +149,18 @@ blit_nearest(struct gl_context *ctx,
switch (buffer) {
case GL_COLOR_BUFFER_BIT:
- readRb = ctx->ReadBuffer->_ColorReadBuffer;
- drawRb = ctx->DrawBuffer->_ColorDrawBuffers[0];
+ readAtt = &readFb->Attachment[readFb->_ColorReadBufferIndex];
+ for (int i = 0; i < drawFb->_NumColorDrawBuffers; i++) {
+ int idx = drawFb->_ColorDrawBufferIndexes[i];
+ if (idx != -1) {
+ drawAtt = &drawFb->Attachment[idx];
+ }
+ else {
+ continue;
if (idx == -1)
continue;
drawAtt = &drawFb->Attachment[idx];
but again, I'm skeptical here since this loop finds the last draw
attachment and doesn't do anything with the prior ones...
+ }
+ }
+ readRb = readFb->_ColorReadBuffer;
+ drawRb = drawAtt->Renderbuffer;
if (readRb->Format == drawRb->Format) {
mode = DIRECT;
@@ -159,8 +172,10 @@ blit_nearest(struct gl_context *ctx,
break;
case GL_DEPTH_BUFFER_BIT:
- readRb = ctx->ReadBuffer->Attachment[BUFFER_DEPTH].Renderbuffer;
- drawRb = ctx->DrawBuffer->Attachment[BUFFER_DEPTH].Renderbuffer;
+ readAtt = &readFb->Attachment[BUFFER_DEPTH];
+ drawAtt = &drawFb->Attachment[BUFFER_DEPTH];
+ readRb = readAtt->Renderbuffer;
+ drawRb = drawAtt->Renderbuffer;
/* Note that for depth/stencil, the formats of src/dst must match. By
* using the core helpers for pack/unpack, we avoid needing to handle
@@ -175,8 +190,10 @@ blit_nearest(struct gl_context *ctx,
pixelSize = 4;
break;
case GL_STENCIL_BUFFER_BIT:
- readRb = ctx->ReadBuffer->Attachment[BUFFER_STENCIL].Renderbuffer;
- drawRb = ctx->DrawBuffer->Attachment[BUFFER_STENCIL].Renderbuffer;
+ readAtt = &readFb->Attachment[BUFFER_STENCIL];
+ drawAtt = &drawFb->Attachment[BUFFER_STENCIL];
+ readRb = readAtt->Renderbuffer;
+ drawRb = drawAtt->Renderbuffer;
mode = UNPACK_S;
pixelSize = 1;
break;
@@ -208,7 +225,9 @@ blit_nearest(struct gl_context *ctx,
return;
}
- if (readRb == drawRb) {
+ if ((readRb == drawRb) ||
+ (readAtt->Texture && drawAtt->Texture &&
+ (readAtt->Texture == drawAtt->Texture))) {
/* map whole buffer for read/write */
/* XXX we could be clever and just map the union region of the
* source and dest rects.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
