fix mesa's handling of fbo's / window fb (again)

Make sure the relevant fields in window fbs get updated at appropriate time
(those are NOT the same as fbos!!!), and fix up related code accordingly.
This is a bit ugly, but there's a reason the issues section in EXT_fbo is
a couple hundred pages long...
Hopefully correct now.

6 files changed, 77 insertions, 41 deletions

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 1aa0a02fc78..b422198f929 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -98,9 +98,13 @@ _mesa_PushAttrib(GLbitfield mask)
    }
 
    if (mask & GL_COLOR_BUFFER_BIT) {
+      GLuint i;
       struct gl_colorbuffer_attrib *attr;
       attr = MALLOC_STRUCT( gl_colorbuffer_attrib );
       MEMCPY( attr, &ctx->Color, sizeof(struct gl_colorbuffer_attrib) );
+      /* push the Draw FBO's DrawBuffer[] state, not ctx->Color.DrawBuffer[] */
+      for (i = 0; i < ctx->Const.MaxDrawBuffers; i ++)
+         attr->DrawBuffer[i] = ctx->DrawBuffer->ColorDrawBuffer[i];
       newnode = new_attrib_node( GL_COLOR_BUFFER_BIT );
       newnode->data = attr;
       newnode->next = head;
diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
index 68a0575d93d..7764a5d3b2e 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -370,6 +370,14 @@ _mesa_DrawBuffer(GLenum buffer)
 
    /* if we get here, there's no error so set new state */
    _mesa_drawbuffers(ctx, 1, &buffer, &destMask);
+
+   /*
+    * Call device driver function.
+    */
+   if (ctx->Driver.DrawBuffers)
+      ctx->Driver.DrawBuffers(ctx, 1, &buffer);
+   else if (ctx->Driver.DrawBuffer)
+      ctx->Driver.DrawBuffer(ctx, buffer);
 }
 
 
@@ -435,6 +443,14 @@ _mesa_DrawBuffersARB(GLsizei n, const GLenum *buffers)
 
    /* OK, if we get here, there were no errors so set the new state */
    _mesa_drawbuffers(ctx, n, buffers, destMask);
+
+   /*
+    * Call device driver function.
+    */
+   if (ctx->Driver.DrawBuffers)
+      ctx->Driver.DrawBuffers(ctx, n, buffers);
+   else if (ctx->Driver.DrawBuffer)
+      ctx->Driver.DrawBuffer(ctx, buffers[0]);
 }
 
 
@@ -463,14 +479,15 @@ set_color_output(GLcontext *ctx, GLuint output, GLenum buffer,
    /* not really needed, will be set later */
    fb->_NumColorDrawBuffers[output] = 0;
 
+   if (fb->Name == 0)
    /* Set traditional state var */
-   ctx->Color.DrawBuffer[output] = buffer;
+      ctx->Color.DrawBuffer[output] = buffer;
 }
 
 
 /**
  * Helper routine used by _mesa_DrawBuffer, _mesa_DrawBuffersARB and
- * _mesa_PopAttrib to set drawbuffer state.
+ * other places (window fbo fixup) to set fbo (and the old ctx) fields.
  * All error checking will have been done prior to calling this function
  * so nothing should go wrong at this point.
  * \param ctx  current context
@@ -479,6 +496,7 @@ set_color_output(GLcontext *ctx, GLuint output, GLenum buffer,
  * \param destMask  array[n] of BUFFER_* bitmasks which correspond to the
  *                  colorbuffer names.  (i.e. GL_FRONT_AND_BACK =>
  *                  BUFFER_BIT_FRONT_LEFT | BUFFER_BIT_BACK_LEFT).
+ * \param callDriver call driver or not (bad idea sometimes this is called)
  */
 void
 _mesa_drawbuffers(GLcontext *ctx, GLuint n, const GLenum *buffers,
@@ -509,30 +527,15 @@ _mesa_drawbuffers(GLcontext *ctx, GLuint n, const GLenum *buffers,
    }
 
    ctx->NewState |= _NEW_COLOR;
-
-   /*
-    * Call device driver function.
-    */
-   if (ctx->Driver.DrawBuffers)
-      ctx->Driver.DrawBuffers(ctx, n, buffers);
-   else if (ctx->Driver.DrawBuffer)
-      ctx->Driver.DrawBuffer(ctx, buffers[0]);
 }
 
 
-
-/**
- * Called by glReadBuffer to set the source renderbuffer for reading pixels.
- * \param mode color buffer such as GL_FRONT, GL_BACK, etc.
- */
-void GLAPIENTRY
-_mesa_ReadBuffer(GLenum buffer)
+GLboolean
+_mesa_readbuffer_update_fields(GLcontext *ctx, GLenum buffer)
 {
    struct gl_framebuffer *fb;
    GLbitfield supportedMask;
    GLint srcBuffer;
-   GET_CURRENT_CONTEXT(ctx);
-   ASSERT_OUTSIDE_BEGIN_END_AND_FLUSH(ctx);
 
    fb = ctx->ReadBuffer;
 
@@ -548,20 +551,43 @@ _mesa_ReadBuffer(GLenum buffer)
       srcBuffer = read_buffer_enum_to_index(buffer);
       if (srcBuffer == -1) {
          _mesa_error(ctx, GL_INVALID_ENUM, "glReadBuffer(buffer=0x%x)", buffer);
-         return;
+         return GL_FALSE;
       }
       supportedMask = supported_buffer_bitmask(ctx, fb);
       if (((1 << srcBuffer) & supportedMask) == 0) {
          _mesa_error(ctx, GL_INVALID_OPERATION, "glReadBuffer(buffer=0x%x)", buffer);
-         return;
+         return GL_FALSE;
       }
    }
 
-   ctx->Pixel.ReadBuffer = buffer;
-
+   if (fb->Name == 0) {
+      ctx->Pixel.ReadBuffer = buffer;
+   }
    fb->ColorReadBuffer = buffer;
    fb->_ColorReadBufferIndex = srcBuffer;
 
+   return GL_TRUE;
+}
+
+
+
+/**
+ * Called by glReadBuffer to set the source renderbuffer for reading pixels.
+ * \param mode color buffer such as GL_FRONT, GL_BACK, etc.
+ * \param callDriver call driver or not (bad idea sometimes this is called)
+ */
+void GLAPIENTRY
+_mesa_ReadBuffer(GLenum buffer)
+{
+   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;
+
    ctx->NewState |= _NEW_PIXEL;
 
    /*
diff --git a/src/mesa/main/buffers.h b/src/mesa/main/buffers.h
index fcc21523421..208e7af2b93 100644
--- a/src/mesa/main/buffers.h
+++ b/src/mesa/main/buffers.h
@@ -56,6 +56,9 @@ extern void
 _mesa_drawbuffers(GLcontext *ctx, GLuint n, const GLenum *buffers,
                   const GLbitfield *destMask);
 
+extern GLboolean
+_mesa_readbuffer_update_fields(GLcontext *ctx, GLenum buffer);
+
 extern void GLAPIENTRY
 _mesa_ReadBuffer( GLenum mode );
 
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 4e6732dc7ac..00e4c8328e5 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1496,14 +1496,8 @@ _mesa_make_current( GLcontext *newCtx, GLframebuffer *drawBuffer,
          if (!newCtx->DrawBuffer || newCtx->DrawBuffer->Name == 0) {
             _mesa_reference_framebuffer(&newCtx->DrawBuffer, drawBuffer);
          /* fix up the fb fields - these will end up wrong otherwise
-            if the DRIdrawable changes, and someone may rely on them.
-          */
-            /* What a mess!?! */
-            /* XXX this is still not quite correct. Imagine a user-created fbo
-               bound on a context. Now rebind with a completely new drawable.
-               Upon rebinding to the window-framebuffer, we have no idea what
-               the read and write buffers should be (front, back, ...) - that
-               information was only available in the previously used drawable... */
+            if the DRIdrawable changes, and everything relies on them.
+            This is a bit messy (same as needed in _mesa_BindFramebufferEXT) */
             int i;
             GLenum buffers[MAX_DRAW_BUFFERS];
             for(i = 0; i < newCtx->Const.MaxDrawBuffers; i++) {
@@ -1513,7 +1507,7 @@ _mesa_make_current( GLcontext *newCtx, GLframebuffer *drawBuffer,
          }
          if (!newCtx->ReadBuffer || newCtx->ReadBuffer->Name == 0) {
             _mesa_reference_framebuffer(&newCtx->ReadBuffer, readBuffer);
-            _mesa_ReadBuffer(newCtx->Pixel.ReadBuffer);
+            _mesa_readbuffer_update_fields(newCtx, newCtx->Pixel.ReadBuffer);
          }
 
 	 newCtx->NewState |= _NEW_BUFFERS;
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index f300e481ce0..6f7effcce70 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -29,6 +29,7 @@
  */
 
 
+#include "buffers.h"
 #include "context.h"
 #include "fbobject.h"
 #include "framebuffer.h"
@@ -1001,23 +1002,32 @@ _mesa_BindFramebufferEXT(GLenum target, GLuint framebuffer)
     * XXX check if re-binding same buffer and skip some of this code.
     */
 
+   /* 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!!! */
    if (bindReadBuf) {
       _mesa_reference_framebuffer(&ctx->ReadBuffer, newFbread);
-      /* set context value */
-      ctx->Pixel.ReadBuffer = newFbread->ColorReadBuffer;
+      if (!newFbread->Name) {
+         _mesa_readbuffer_update_fields(ctx, ctx->Pixel.ReadBuffer);
+      }
    }
 
    if (bindDrawBuf) {
-      GLuint i;
       /* 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);
-      /* set context value */
-      for (i = 0; i < ctx->Const.MaxDrawBuffers; i++) {
-	 ctx->Color.DrawBuffer[i] = newFb->ColorDrawBuffer[i];
+      if (!newFb->Name) {
+         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);
       }
-      if (newFb->Name != 0) {
+      else {
          /* check if newly bound framebuffer has any texture attachments */
          check_begin_texture_render(ctx, newFb);
       }
diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
index 2ff067164bf..c9b30d32528 100644
--- a/src/mesa/main/framebuffer.c
+++ b/src/mesa/main/framebuffer.c
@@ -695,8 +695,7 @@ _mesa_update_framebuffer(GLcontext *ctx)
    struct gl_framebuffer *fbread = ctx->ReadBuffer;
 
    update_framebuffer(ctx, fb);
-   if (fbread != fb && fbread != NULL /* can happen at make_current -
-      core/driver circular dependencies, should be fixed up */)
+   if (fbread != fb)
       update_framebuffer(ctx, fbread);
 }