fix mesa's handling of fbo's / window fb (again)
authorRoland Scheidegger <sroland@tungstengraphics.com>
Wed, 18 Jul 2007 18:17:14 +0000 (20:17 +0200)
committerRoland Scheidegger <sroland@tungstengraphics.com>
Thu, 19 Jul 2007 15:59:59 +0000 (17:59 +0200)
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.

src/mesa/main/attrib.c
src/mesa/main/buffers.c
src/mesa/main/buffers.h
src/mesa/main/context.c
src/mesa/main/fbobject.c
src/mesa/main/framebuffer.c

index 1aa0a02fc789d894368d27d9cabcc1c291a5d653..b422198f929c1b71e4c958043f64f81e01947b73 100644 (file)
@@ -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;
index 68a0575d93de28bb905ad83087472db30f51ad35..7764a5d3b2eb2ae0115e8eb9f27be57dbea5d98a 100644 (file)
@@ -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;
 
    /*
index fcc21523421ec5ac8abed55bdd8513bb3da35578..208e7af2b933a9bf9e127c1c6bbbc1e8d092f71f 100644 (file)
@@ -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 );
 
index 4e6732dc7acc33cd28edb619da6945e7144a97ea..00e4c8328e559edc2047de55d091f5a9ca1332f0 100644 (file)
@@ -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;
index f300e481ce089a789d43f091294cd928929d1a21..6f7effcce70b96086523f538a3bfd06e40ecf298 100644 (file)
@@ -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);
       }
index 2ff067164bfaea221918124eadcd5792c1ab2520..c9b30d3252829993b796366dcf096f7928f441c3 100644 (file)
@@ -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);
 }