EGLBE: Restore buf state when binding default FB

This is very subtle, but glBindFramebuffer(..., 0) should restore the
current context's previous draw/read buffer state for the default
framebuffer, as glXMake*Current() does.  Swapping the buffers of a GLX
drawable causes the EGL back end to create a new FBO to serve as the
drawable's default framebuffer, with the new FBO containing the previous
RBOs bound in reverse order.  The default draw/read buffer for a new FBO
is GL_COLOR_ATTACHMENT0, which the EGL back end uses as the front left
buffer.  Thus, if the default framebuffer for a drawable isn't bound
when the drawable's buffers are swapped, then the FBO's initial
draw/read buffer remains unchanged.  If GL_BACK was active the last time
the default framebuffer was bound, then calling
glBindFrameBuffer(..., 0) after a buffer swap put VirtualGL into a weird
state whereby it reported that GL_BACK was active, but the RBO
corresponding to GL_FRONT was actually active.  This issue caused Webots
to flicker when rendering a camera view, and it may have affected other
applications as well.

Fixes #172

Author: dcommander

ChangeLog.md

@@ -15,6 +15,12 @@ of the form `egl{n}`, where `{n}` is a zero-based index, or by a DRI device
 path.  A list of valid EGL device IDs and their associated DRI device paths can
 be obtained by running `/opt/VirtualGL/bin/eglinfo -e`.
 
+4. Fixed an issue in the EGL back end whereby `glBindFramebuffer()`, when used
+to bind the default framebuffer, did not restore the previous draw/read buffer
+state for the default framebuffer.  This issue was known to cause flickering in
+Webots when rendering a camera view, and it may have affected other
+applications as well.
+
 
 3.0.1
 =====

server/backend.cpp

@@ -73,25 +73,32 @@ static FakePbuffer *getCurrentFakePbuffer(EGLint readdraw)
 void bindFramebuffer(GLenum target, GLuint framebuffer, bool ext)
 {
 	#ifdef EGLBACKEND
+	const GLenum *oldDrawBufs = NULL;  GLsizei nDrawBufs = 0;
+	GLenum oldReadBuf = GL_NONE;
+	FakePbuffer *drawpb = NULL, *readpb = NULL;
+
 	if(fconfig.egl)
 	{
 		if(framebuffer == 0)
 		{
 			if(target == GL_DRAW_FRAMEBUFFER || target == GL_FRAMEBUFFER)
 			{
-				FakePbuffer *pb = pbhashegl.find(getCurrentDrawableEGL());
-				if(pb)
+				drawpb = pbhashegl.find(getCurrentDrawableEGL());
+				if(drawpb)
 				{
-					framebuffer = pb->getFBO();
+					oldDrawBufs =
+						ctxhashegl.getDrawBuffers(_eglGetCurrentContext(), nDrawBufs);
+					framebuffer = drawpb->getFBO();
 					ctxhashegl.setDrawFBO(_eglGetCurrentContext(), 0);
 				}
 			}
 			if(target == GL_READ_FRAMEBUFFER || target == GL_FRAMEBUFFER)
 			{
-				FakePbuffer *pb = pbhashegl.find(getCurrentReadDrawableEGL());
-				if(pb)
+				readpb = pbhashegl.find(getCurrentReadDrawableEGL());
+				if(readpb)
 				{
-					framebuffer = pb->getFBO();
+					oldReadBuf = ctxhashegl.getReadBuffer(_eglGetCurrentContext());
+					framebuffer = readpb->getFBO();
 					ctxhashegl.setReadFBO(_eglGetCurrentContext(), 0);
 				}
 			}
@@ -107,6 +114,20 @@ void bindFramebuffer(GLenum target, GLuint framebuffer, bool ext)
 	#endif
 	if(ext) _glBindFramebufferEXT(target, framebuffer);
 	else _glBindFramebuffer(target, framebuffer);
+	#ifdef EGLBACKEND
+	if(fconfig.egl)
+	{
+		if(oldDrawBufs)
+		{
+			if(nDrawBufs == 1)
+				drawpb->setDrawBuffer(oldDrawBufs[0], false);
+			else if(nDrawBufs > 0)
+				drawpb->setDrawBuffers(nDrawBufs, oldDrawBufs, false);
+			delete [] oldDrawBufs;
+		}
+		if(oldReadBuf) readpb->setReadBuffer(oldReadBuf, false);
+	}
+	#endif
 }
 
 

server/fakerut.cpp

@@ -994,10 +994,10 @@ int readbackTestMS(void)
 
 		XMapWindow(dpy, win);
 
-		clr.clear(GL_BACK);
-		VERIFY_BUF_COLOR(GL_BACK, clr.bits(-1), "GL_BACK");
 		clr.clear(GL_FRONT);
 		VERIFY_BUF_COLOR(GL_FRONT, clr.bits(-1), "GL_FRONT");
+		clr.clear(GL_BACK);
+		VERIFY_BUF_COLOR(GL_BACK, clr.bits(-1), "GL_BACK");
 		glGenFramebuffers(1, &fbo);
 		glGenRenderbuffers(1, &rbo0);
 		glGenRenderbuffers(1, &rbo1);
@@ -1017,9 +1017,26 @@ int readbackTestMS(void)
 		VERIFY_FBO(fbo, GL_COLOR_ATTACHMENT0, GL_COLOR_ATTACHMENT1, fbo,
 			GL_COLOR_ATTACHMENT1);
 		glBindFramebuffer(GL_FRAMEBUFFER, 0);
+		VERIFY_FBO(0, GL_BACK, GL_NONE, 0, GL_BACK);
+		checkBufferState(GL_BACK, GL_BACK, dpy, win, win, ctx);
+		clr.clear(0);
+		VERIFY_BUF_COLOR(0, clr.bits(-1), "GL_BACK");
+		// When using the EGL back end in VirtualGL 3.0 beta1 through 3.0.1, the
+		// following command will fail, because glBindFramebuffer(..., 0) didn't
+		// restore the previous draw/read buffer state for the default framebuffer.
+		// Swapping the buffers of a GLX drawable causes the EGL back end to
+		// create a new FBO to serve as the drawable's default framebuffer, with
+		// the new FBO containing the previous RBOs bound in reverse order.  The
+		// default draw/read buffer for a new FBO is GL_COLOR_ATTACHMENT0, which
+		// the EGL back end uses as the front left buffer.  Since the default
+		// framebuffer isn't bound when the buffers are swapped in the code above,
+		// the FBO's initial draw/read buffer remains unchanged.  Thus, at this
+		// point in the code, VirtualGL reported that GL_BACK was active, but the
+		// RBO corresponding to GL_FRONT was actually active.  As a result, the
+		// previous two commands affected the front buffer rather than the back
+		// buffer.
+		VERIFY_BUF_COLOR(GL_BACK, clr.bits(-1), "GL_BACK");
 		VERIFY_BUF_COLOR(GL_FRONT, clr.bits(-2), "GL_FRONT");
-		VERIFY_FBO(0, GL_FRONT, GL_NONE, 0, GL_FRONT);
-		checkBufferState(GL_FRONT, GL_FRONT, dpy, win, win, ctx);
 		checkFrame(dpy, win, 1, lastFrame);
 		checkWindowColor(dpy, win, clr.bits(-2));
 
@@ -2044,6 +2061,13 @@ int offScreenTest(bool dbPixmap, bool doSelectEvent)
 			printf("FBO->Window:                    ");
 			if(!(glXMakeContextCurrent(dpy, glxwin, glxwin, ctx)))
 				THROWNL("Could not make context current");
+			// The following command will fail unless the implementation of
+			// glXMake*Current() in the EGL back end restores the context's previous
+			// read buffer state for the default framebuffer.  The multithreaded
+			// rendering test will fail unless the implementation of
+			// glXMake*Current() in the EGL back end restores the context's previous
+			// draw buffer state for the default framebuffer.
+			VERIFY_BUF_COLOR(0, clr.bits(-2), "Win");
 			CHECK_GL_ERROR();
 			checkCurrent(dpy, glxwin, glxwin, ctx, dpyw / 2, dpyh / 2);
 			clr.clear(GL_BACK);