EGLBE: Fix OGL errs in glXMake*()/glXSwapBuffers()

- When calling FakePbuffer::createBuffer() from backend::makeCurrent(),
  don't try to re-bind the old FBO if the new FBO will immediately be
  bound.  This fixes a GL_INVALID_OPERATION error that could occur due
  to the following sequence of function calls:

    glXMakeCurrent(dpy, window, context);
    glXMakeCurrent(dpy, 0, 0);
    XDestroyWindow(dpy, window);
    glXMakeCurrent(dpy, other_drawable, context);

  XDestroyWindow() destroys the FakePbuffer instance associated with the
  window, which destroys the FBO and RBOs associated with the
  FakePbuffer instance.  Since XDestroyWindow() is a context-less
  function, the FBO destruction occurs in the RBO context.  However, the
  destroyed FBO is still the current FBO in the application's context.
  The FBO associated with other_drawable will be made current once the
  second glXMake*Current() call completes, so we simply need to avoid
  binding the destroyed FBO within the body of that function call.  The
  easiest way to do that is to avoid restoring an FBO binding via the
  BufferState instance created in FakePbuffer::createBuffer() if that
  binding would immediately be overwritten.

  We do likewise within FakePbuffer:swap(), to fix a similar
  GL_INVALID_OPERATION error in glXSwapBuffers() that was observed
  sporadically in the fakerut multithreaded rendering test.  The exact
  mechanism behind this error was not entirely clear.

- In backend::makeCurrent(), call FakePbuffer::setDrawBuffer() rather
  than FakePbuffer::setDrawBuffers() if ContextHashEGL.getDrawBuffers()
  returns only one buffer.  If ContextHashEGL.getDrawBuffers() returns
  only one buffer, then that buffer may be GL_FRONT or another enum that
  would trigger a GL_INVALID_ENUM error if passed to
  FakePbuffer::setDrawBuffers() (since FakePbuffer::setDrawBuffers()
  emulates the behavior of glDrawBuffers().)

Fixes #199

ChangeLog.md

@@ -29,6 +29,12 @@ known to affect Qt5 applications.
 installed in **/etc/sddm**, which is the case when SDDM is installed through
 EPEL.
 
+6. Fixed several issues in the EGL back end that caused OpenGL errors to be
+generated by the interposed `glXMake*Current()` and `glXSwapBuffers()`
+functions.  These OpenGL errors sometimes caused fatal errors when closing Qt
+applications, and they may have affected other types of applications and use
+cases as well.
+
 
 3.0
 ===

server/FakePbuffer.cpp

@@ -72,7 +72,8 @@ FakePbuffer::~FakePbuffer(void)
 }
 
 
-void FakePbuffer::createBuffer(bool useRBOContext, bool ignoreReadDrawBufs)
+void FakePbuffer::createBuffer(bool useRBOContext, bool ignoreReadDrawBufs,
+	bool ignoreDrawFBO, bool ignoreReadFBO)
 {
 	TempContextEGL *tc = NULL;
 	BufferState *bs = NULL;
@@ -85,11 +86,12 @@ void FakePbuffer::createBuffer(bool useRBOContext, bool ignoreReadDrawBufs)
 			tc = new TempContextEGL(getRBOContext(dpy).getContext());
 		else
 		{
-			if(ignoreReadDrawBufs)
-				bs = new BufferState(BS_DRAWFBO | BS_READFBO | BS_RBO);
-			else
-				bs = new BufferState(BS_DRAWFBO | BS_READFBO | BS_RBO | BS_DRAWBUFS |
-					BS_READBUF);
+			int saveMask = BS_RBO;
+			if(!ignoreReadDrawBufs) saveMask |= BS_DRAWBUFS | BS_READBUF;
+			if(!ignoreDrawFBO) saveMask |= BS_DRAWFBO;
+			if(!ignoreReadFBO) saveMask |= BS_READFBO;
+
+			bs = new BufferState(saveMask);
 		}
 
 		TRY_GL();
@@ -223,7 +225,8 @@ void FakePbuffer::swap(void)
 		GLuint oldFBO = fbo;
 
 		if(getCurrentDrawable() == id || getCurrentReadDrawable() == id)
-			createBuffer(false);
+			createBuffer(false, false, drawFBO == (GLint)oldFBO,
+				readFBO == (GLint)oldFBO);
 
 		if(getCurrentDrawable() == id && drawFBO == (GLint)oldFBO)
 		{

server/FakePbuffer.h

@@ -1,4 +1,4 @@
-// Copyright (C)2019-2021 D. R. Commander
+// Copyright (C)2019-2022 D. R. Commander
 //
 // This library is free software and may be redistributed and/or modified under
 // the terms of the wxWindows Library License, Version 3.1 or (at your option)
@@ -28,7 +28,8 @@ namespace backend
 			FakePbuffer(Display *dpy, VGLFBConfig config, const int *glxAttribs);
 			~FakePbuffer(void);
 
-			void createBuffer(bool useRBOContext, bool ignoreReadDrawBufs = false);
+			void createBuffer(bool useRBOContext, bool ignoreReadDrawBufs = false,
+				bool ignoreDrawFBO = false, bool ignoreReadFBO = false);
 			Display *getDisplay(void) { return dpy; }
 			GLXDrawable getID(void) { return id; }
 			VGLFBConfig getFBConfig(void) { return config; }

server/backend.cpp

@@ -807,8 +807,14 @@ Bool makeCurrent(Display *dpy, GLXDrawable draw, GLXDrawable read,
 			_glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &drawFBO);
 			_glGetIntegerv(GL_READ_FRAMEBUFFER_BINDING, &readFBO);
 
-			if(drawpb) drawpb->createBuffer(false, true);
-			if(readpb && readpb != drawpb) readpb->createBuffer(false, true);
+			if(drawpb)
+				drawpb->createBuffer(false, true,
+					(ctxhashegl.getDrawFBO(ctx) == 0 || drawFBO == 0),
+					(ctxhashegl.getReadFBO(ctx) == 0 || readFBO == 0));
+			if(readpb && readpb != drawpb)
+				readpb->createBuffer(false, true,
+					(ctxhashegl.getDrawFBO(ctx) == 0 || drawFBO == 0),
+					(ctxhashegl.getReadFBO(ctx) == 0 || readFBO == 0));
 
 			bool boundNewDrawFBO = false, boundNewReadFBO = false;
 			if(drawpb && (ctxhashegl.getDrawFBO(ctx) == 0 || drawFBO == 0))
@@ -837,7 +843,10 @@ Bool makeCurrent(Display *dpy, GLXDrawable draw, GLXDrawable read,
 					oldDrawBufs = ctxhashegl.getDrawBuffers(ctx, nDrawBufs);
 					if(oldDrawBufs && nDrawBufs > 0)
 					{
-						drawpb->setDrawBuffers(nDrawBufs, oldDrawBufs, false);
+						if(nDrawBufs == 1)
+							drawpb->setDrawBuffer(oldDrawBufs[0], false);
+						else
+							drawpb->setDrawBuffers(nDrawBufs, oldDrawBufs, false);
 						delete [] oldDrawBufs;
 					}
 				}

server/fakerut.cpp

@@ -39,6 +39,14 @@
 using namespace util;
 
 
+#define CHECK_GL_ERROR() \
+{ \
+	int e = glGetError(); \
+	if(e != GL_NO_ERROR) THROW("OpenGL error"); \
+	while(e != GL_NO_ERROR) e = glGetError(); \
+}
+
+
 #define CLEAR_BUFFER(buffer, r, g, b, a) \
 { \
 	if(buffer > 0) \
@@ -1644,6 +1652,7 @@ class TestThread : public Runnable
 				glClear(GL_COLOR_BUFFER_BIT);
 				glReadBuffer(GL_FRONT);
 				glXSwapBuffers(dpy, win);
+				CHECK_GL_ERROR();
 				checkReadbackState(GL_FRONT, dpy, win, win, ctx);
 				checkFrame(dpy, win, 1, lastFrame);
 				checkWindowColor(dpy, win, colors[clr].bits, false);
@@ -1953,6 +1962,7 @@ int offScreenTest(bool dbPixmap, bool doSelectEvent)
 			VERIFY_BUF_COLOR(GL_BACK, clr.bits(-2), "PB");
 			if(!(glXMakeContextCurrent(dpy, glxwin, pb, ctx)))
 				THROWNL("Could not make context current");
+			CHECK_GL_ERROR();
 			checkCurrent(dpy, glxwin, pb, ctx, dpyw / 2, dpyh / 2);
 			glReadBuffer(GL_BACK);  glDrawBuffer(GL_BACK);
 			glCopyPixels(0, 0, dpyw / 2, dpyh / 2, GL_COLOR);
@@ -1974,6 +1984,7 @@ int offScreenTest(bool dbPixmap, bool doSelectEvent)
 			printf("Window->Pbuffer:                ");
 			if(!(glXMakeContextCurrent(dpy, glxwin, glxwin, ctx)))
 				THROWNL("Could not make context current");
+			CHECK_GL_ERROR();
 			fontListBase = glGenLists(maxChar + 1);
 			glXUseXFont(fontInfo->fid, minChar, maxChar - minChar + 1,
 				fontListBase + minChar);
@@ -1983,6 +1994,7 @@ int offScreenTest(bool dbPixmap, bool doSelectEvent)
 			VERIFY_BUF_COLOR(GL_BACK, clr.bits(-2), "Win");
 			if(!(glXMakeContextCurrent(dpy, pb, glxwin, ctx)))
 				THROWNL("Could not make context current");
+			CHECK_GL_ERROR();
 			fontListBase = glGenLists(maxChar + 1);
 			glXUseXFont(fontInfo->fid, minChar, maxChar - minChar + 1,
 				fontListBase + minChar);
@@ -1992,6 +2004,23 @@ int offScreenTest(bool dbPixmap, bool doSelectEvent)
 			glCopyPixels(0, 0, dpyw / 2, dpyh / 2, GL_COLOR);
 			glXSwapBuffers(dpy, pb);
 			VERIFY_BUF_COLOR(GL_BACK, clr.bits(-2), "PB");
+
+			// Verify that the EGL back end manages FBO bindings properly when
+			// attaching a context to an OpenGL window, destroying the window, then
+			// attaching the same context to a different drawable.  This will fail
+			// when using the EGL back end with VirtualGL 3.0.
+			Window win2;
+			if((win2 = XCreateWindow(dpy, root, 0, 0, dpyw / 2, dpyh / 2, 0,
+				vis->depth, InputOutput, vis->visual,
+				CWBorderPixel | CWColormap | CWEventMask, &swa)) == 0)
+				THROW("Could not create window");
+			XMapWindow(dpy, win2);
+			glXMakeCurrent(dpy, win2, ctx);
+			glXMakeCurrent(dpy, 0, 0);
+			XDestroyWindow(dpy, win2);
+			glXMakeContextCurrent(dpy, pb, pb, ctx);
+			CHECK_GL_ERROR();
+
 			printf("SUCCESS\n");
 		}
 		catch(std::exception &e)
@@ -2005,12 +2034,14 @@ int offScreenTest(bool dbPixmap, bool doSelectEvent)
 			printf("FBO->Window:                    ");
 			if(!(glXMakeContextCurrent(dpy, glxwin, glxwin, ctx)))
 				THROWNL("Could not make context current");
+			CHECK_GL_ERROR();
 			checkCurrent(dpy, glxwin, glxwin, ctx, dpyw / 2, dpyh / 2);
 			clr.clear(GL_BACK);
 			clr.clear(GL_FRONT);
 			VERIFY_BUF_COLOR(GL_BACK, clr.bits(-2), "Win");
 			if(!(glXMakeContextCurrent(dpy, glxwin, glxwin, ctx)))
 				THROWNL("Could not make context current");
+			CHECK_GL_ERROR();
 			checkCurrent(dpy, glxwin, glxwin, ctx, dpyw / 2, dpyh / 2);
 			glDrawBuffer(GL_FRONT_AND_BACK);
 			glReadBuffer(GL_FRONT);
@@ -2084,6 +2115,7 @@ int offScreenTest(bool dbPixmap, bool doSelectEvent)
 
 			if(!(glXMakeContextCurrent(dpy, glxpm0, glxpm0, ctx)))
 				THROWNL("Could not make context current");
+			CHECK_GL_ERROR();
 			fontListBase = glGenLists(maxChar + 1);
 			glXUseXFont(fontInfo->fid, minChar, maxChar - minChar + 1,
 				fontListBase + minChar);
@@ -2575,12 +2607,6 @@ static void checkContext(unsigned long mask)
 	CHECK_INT4(GL_VIEWPORT, expectedViewport);
 }
 
-#define CHECK_GL_ERROR() \
-{ \
-	int e = glGetError(); \
-	if(e != GL_NO_ERROR) THROW("OpenGL error"); \
-}
-
 #define CHECK_CONTEXT(m) \
 { \
 	if(!(glXMakeCurrent(dpy, win, ctx1))) \