Rework the GL_READ_BUFFER, GL_DRAW_BUFFER state repairs that Roland previously did.

Basically, in update_framebuffer() (which should be called after an FBO is bound
with MakeCurrent or BindFramebuffer) we check if the FBO is a window-system FBO.
If it is, update the FBO's GL_READ/DRAW_BUFFER state according to the context state.
Old code still in place but disabled with #if 0 / #endif.

5 files changed, 131 insertions, 18 deletions

diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
index 82dd059e4b..94b4be54d5 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -336,6 +336,20 @@ read_buffer_enum_to_index(GLenum buffer)
  * \sa _mesa_DrawBuffersARB
  *
  * \param buffer  buffer token such as GL_LEFT or GL_FRONT_AND_BACK, etc.
+ *
+ * Note that the behaviour of this function depends on whether the
+ * current ctx->DrawBuffer is a window-system framebuffer (Name=0) or
+ * a user-created framebuffer object (Name!=0).
+ *   In the former case, we update the per-context ctx->Color.DrawBuffer
+ *   state var _and_ the FB's ColorDrawBuffer state.
+ *   In the later case, we update the FB's ColorDrawBuffer state only.
+ *
+ * Furthermore, upon a MakeCurrent() or BindFramebuffer() call, if the
+ * new FB is a window system FB, we need to re-update the FB's
+ * ColorDrawBuffer state to match the context.  This is handled in
+ * _mesa_update_framebuffer().
+ *
+ * See the GL_EXT_framebuffer_object spec for more info.
  */
 void GLAPIENTRY
 _mesa_DrawBuffer(GLenum buffer)
@@ -489,10 +503,12 @@ set_color_output(GLcontext *ctx, GLuint output, GLenum buffer,
 
 
 /**
- * Helper routine used by _mesa_DrawBuffer, _mesa_DrawBuffersARB and
- * other places (window fbo fixup) to set fbo (and the old ctx) fields.
+ * Helper function to set the GL_DRAW_BUFFER state in the context and
+ * current FBO.
+ *
  * All error checking will have been done prior to calling this function
  * so nothing should go wrong at this point.
+ *
  * \param ctx  current context
  * \param n    number of color outputs to set
  * \param buffers  array[n] of colorbuffer names, like GL_LEFT.
@@ -532,6 +548,7 @@ _mesa_drawbuffers(GLcontext *ctx, GLuint n, const GLenum *buffers,
 }
 
 
+#if 0
 GLboolean
 _mesa_readbuffer_update_fields(GLcontext *ctx, GLenum buffer)
 {
@@ -563,13 +580,45 @@ _mesa_readbuffer_update_fields(GLcontext *ctx, GLenum buffer)
    }
 
    if (fb->Name == 0) {
+      /* Only update the per-context GL_READ_BUFFER state if we're bound to
+       * a window-system framebuffer.
+       */
       ctx->Pixel.ReadBuffer = buffer;
    }
+
+   /* Set the FBO's GL_READ_BUFFER state */
    fb->ColorReadBuffer = buffer;
    fb->_ColorReadBufferIndex = srcBuffer;
 
    return GL_TRUE;
 }
+#endif
+
+
+/**
+ * Like \sa _mesa_drawbuffers(), this is a helper function for setting
+ * GL_READ_BUFFER state in the context and current FBO.
+ * \param ctx  the rendering context
+ * \param buffer  GL_FRONT, GL_BACK, GL_COLOR_ATTACHMENT0, etc.
+ * \param bufferIndex  the numerical index corresponding to 'buffer'
+ */
+void
+_mesa_readbuffer(GLcontext *ctx, GLenum buffer, GLint bufferIndex)
+{
+   struct gl_framebuffer *fb = ctx->ReadBuffer;
+
+   if (fb->Name == 0) {
+      /* Only update the per-context READ_BUFFER state if we're bound to
+       * a window-system framebuffer.
+       */
+      ctx->Pixel.ReadBuffer = buffer;
+   }
+
+   fb->ColorReadBuffer = buffer;
+   fb->_ColorReadBufferIndex = bufferIndex;
+
+   ctx->NewState |= _NEW_PIXEL;
+}
 
 
 
@@ -580,16 +629,43 @@ _mesa_readbuffer_update_fields(GLcontext *ctx, GLenum buffer)
 void GLAPIENTRY
 _mesa_ReadBuffer(GLenum buffer)
 {
+   struct gl_framebuffer *fb;
+   GLbitfield supportedMask;
+   GLint srcBuffer;
    GET_CURRENT_CONTEXT(ctx);
    ASSERT_OUTSIDE_BEGIN_END_AND_FLUSH(ctx);
 
    if (MESA_VERBOSE & VERBOSE_API)
       _mesa_debug(ctx, "glReadBuffer %s\n", _mesa_lookup_enum_by_nr(buffer));
 
-   if (!_mesa_readbuffer_update_fields(ctx, buffer))
-      return;
+   fb = ctx->ReadBuffer;
 
-   ctx->NewState |= _NEW_PIXEL;
+   if (MESA_VERBOSE & VERBOSE_API)
+      _mesa_debug(ctx, "glReadBuffer %s\n", _mesa_lookup_enum_by_nr(buffer));
+
+   if (fb->Name > 0 && buffer == GL_NONE) {
+      /* This is legal for user-created framebuffer objects */
+      srcBuffer = -1;
+   }
+   else {
+      /* general case / window-system framebuffer */
+      srcBuffer = read_buffer_enum_to_index(buffer);
+      if (srcBuffer == -1) {
+         _mesa_error(ctx, GL_INVALID_ENUM,
+                     "glReadBuffer(buffer=0x%x)", buffer);
+         return;
+      }
+      supportedMask = supported_buffer_bitmask(ctx, fb);
+      if (((1 << srcBuffer) & supportedMask) == 0) {
+         _mesa_error(ctx, GL_INVALID_OPERATION,
+                     "glReadBuffer(buffer=0x%x)", buffer);
+         return;
+      }
+   }
+
+   /* OK, all error checking has been completed now */
+
+   _mesa_readbuffer(ctx, buffer, srcBuffer);
 
    /*
     * Call device driver function.
diff --git a/src/mesa/main/buffers.h b/src/mesa/main/buffers.h
index 208e7af2b9..e7228a6da6 100644
--- a/src/mesa/main/buffers.h
+++ b/src/mesa/main/buffers.h
@@ -5,9 +5,9 @@
 
 /*
  * Mesa 3-D graphics library
- * Version:  6.5
+ * Version:  7.1
  *
- * Copyright (C) 1999-2005  Brian Paul   All Rights Reserved.
+ * Copyright (C) 1999-2007  Brian Paul   All Rights Reserved.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -56,8 +56,13 @@ extern void
 _mesa_drawbuffers(GLcontext *ctx, GLuint n, const GLenum *buffers,
                   const GLbitfield *destMask);
 
+extern void
+_mesa_readbuffer(GLcontext *ctx, GLenum buffer, GLint bufferIndex);
+
+#if 0
 extern GLboolean
 _mesa_readbuffer_update_fields(GLcontext *ctx, GLenum buffer);
+#endif
 
 extern void GLAPIENTRY
 _mesa_ReadBuffer( GLenum mode );
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index f3141fe63e..9ee7b4bec7 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1530,6 +1530,7 @@ _mesa_make_current( GLcontext *newCtx, GLframebuffer *drawBuffer,
           */
          if (!newCtx->DrawBuffer || newCtx->DrawBuffer->Name == 0) {
             _mesa_reference_framebuffer(&newCtx->DrawBuffer, drawBuffer);
+#if 000
          /* fix up the fb fields - these will end up wrong otherwise
             if the DRIdrawable changes, and everything relies on them.
             This is a bit messy (same as needed in _mesa_BindFramebufferEXT) */
@@ -1539,12 +1540,18 @@ _mesa_make_current( GLcontext *newCtx, GLframebuffer *drawBuffer,
                buffers[i] = newCtx->Color.DrawBuffer[i];
             }
             _mesa_drawbuffers(newCtx, newCtx->Const.MaxDrawBuffers, buffers, NULL);
+#endif
          }
          if (!newCtx->ReadBuffer || newCtx->ReadBuffer->Name == 0) {
             _mesa_reference_framebuffer(&newCtx->ReadBuffer, readBuffer);
+#if 00
             _mesa_readbuffer_update_fields(newCtx, newCtx->Pixel.ReadBuffer);
+#endif
          }
 
+         /* XXX only set this flag if we're really changing the draw/read
+          * framebuffer bindings.
+          */
 	 newCtx->NewState |= _NEW_BUFFERS;
 
 #if 1
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index fcb620b39d..5cd18d0287 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -962,9 +962,11 @@ _mesa_BindFramebufferEXT(GLenum target, GLuint framebuffer)
    }
 
    FLUSH_VERTICES(ctx, _NEW_BUFFERS);
+
    if (ctx->Driver.Flush) {  
       ctx->Driver.Flush(ctx);
    }
+
    if (framebuffer) {
       /* Binding a user-created framebuffer object */
       newFb = _mesa_lookup_framebuffer(ctx, framebuffer);
@@ -998,34 +1000,43 @@ _mesa_BindFramebufferEXT(GLenum target, GLuint framebuffer)
     * XXX check if re-binding same buffer and skip some of this code.
     */
 
+#if 000
    /* for window-framebuffers, re-initialize the fbo values, as they
       could be wrong (makecurrent with a new drawable while still a fbo
       was bound will lead to default init fbo values).
       note that therefore the context ReadBuffer/DrawBuffer values are not
       valid while fbo's are bound!!! */
+#endif
    if (bindReadBuf) {
       _mesa_reference_framebuffer(&ctx->ReadBuffer, newFbread);
+#if 000
       if (!newFbread->Name) {
          _mesa_readbuffer_update_fields(ctx, ctx->Pixel.ReadBuffer);
       }
+#endif
    }
 
    if (bindDrawBuf) {
       /* check if old FB had any texture attachments */
       check_end_texture_render(ctx, ctx->DrawBuffer);
+
       /* check if time to delete this framebuffer */
       _mesa_reference_framebuffer(&ctx->DrawBuffer, newFb);
-      if (!newFb->Name) {
+
+      if (newFb->Name != 0) {
+         /* check if newly bound framebuffer has any texture attachments */
+         check_begin_texture_render(ctx, newFb);
+      }
+      else {
+         /* XXX try to remove this: */
+#if 000
          GLuint i;
          GLenum buffers[MAX_DRAW_BUFFERS];
          for(i = 0; i < ctx->Const.MaxDrawBuffers; i++) {
             buffers[i] = ctx->Color.DrawBuffer[i];
          }
          _mesa_drawbuffers(ctx, ctx->Const.MaxDrawBuffers, buffers, NULL);
-      }
-      else {
-         /* check if newly bound framebuffer has any texture attachments */
-         check_begin_texture_render(ctx, newFb);
+#endif
       }
    }
 
diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
index 72d85f6f89..3e36197d88 100644
--- a/src/mesa/main/framebuffer.c
+++ b/src/mesa/main/framebuffer.c
@@ -32,6 +32,7 @@
 
 #include "glheader.h"
 #include "imports.h"
+#include "buffers.h"
 #include "context.h"
 #include "depthstencil.h"
 #include "mtypes.h"
@@ -583,7 +584,7 @@ _mesa_update_stencil_buffer(GLcontext *ctx,
 
 
 /**
- * Update the list of color drawing renderbuffer pointers.
+ * Update the (derived) list of color drawing renderbuffer pointers.
  * Later, when we're rendering we'll loop from 0 to _NumColorDrawBuffers
  * writing colors.
  */
@@ -627,7 +628,7 @@ update_color_draw_buffers(GLcontext *ctx, struct gl_framebuffer *fb)
 
 
 /**
- * Update the color read renderbuffer pointer.
+ * Update the (derived) color read renderbuffer pointer.
  * Unlike the DrawBuffer, we can only read from one (or zero) color buffers.
  */
 static void
@@ -668,10 +669,23 @@ update_color_read_buffer(GLcontext *ctx, struct gl_framebuffer *fb)
 static void
 update_framebuffer(GLcontext *ctx, struct gl_framebuffer *fb)
 {
-   /* Completeness only matters for user-created framebuffers */
-   if (fb->Name != 0) {
-      /* XXX: EXT_framebuffer_blit:
-         framebuffer must still be complete wrt read/draw? */
+   if (fb->Name == 0) {
+      /* This is a window-system framebuffer */
+      /* Need to update the FB's GL_DRAW_BUFFER state to match the
+       * context state (GL_READ_BUFFER too).
+       */
+      if (fb->ColorDrawBuffer[0] != ctx->Color.DrawBuffer[0]) {
+         _mesa_drawbuffers(ctx, ctx->Const.MaxDrawBuffers,
+                           ctx->Color.DrawBuffer, NULL);
+      }
+      if (fb->ColorReadBuffer != ctx->Pixel.ReadBuffer) {
+         
+      }
+   }
+   else {
+      /* This is a user-created framebuffer.
+       * Completeness only matters for user-created framebuffers.
+       */
       _mesa_test_framebuffer_completeness(ctx, fb);
       _mesa_update_framebuffer_visual(fb);
    }