From fa589229e51cb8ba12890e0961405583cdc7c41a Mon Sep 17 00:00:00 2001 From: DRC Date: Fri, 13 May 2022 18:59:59 -0500 Subject: [PATCH] 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 | 6 ++++++ server/FakePbuffer.cpp | 17 ++++++++++------- server/FakePbuffer.h | 5 +++-- server/backend.cpp | 15 ++++++++++++--- server/fakerut.cpp | 38 ++++++++++++++++++++++++++++++++------ 5 files changed, 63 insertions(+), 18 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 112fa96b..b275b97e 100644 --- a/ChangeLog.md +++ b/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 === diff --git a/server/FakePbuffer.cpp b/server/FakePbuffer.cpp index 7d0e7828..bfa55279 100644 --- a/server/FakePbuffer.cpp +++ b/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) { diff --git a/server/FakePbuffer.h b/server/FakePbuffer.h index 270c60a6..d0b9cba5 100644 --- a/server/FakePbuffer.h +++ b/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; } diff --git a/server/backend.cpp b/server/backend.cpp index 9f13ec23..3c4393ec 100644 --- a/server/backend.cpp +++ b/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; } } diff --git a/server/fakerut.cpp b/server/fakerut.cpp index 0fca16b3..f51d7c36 100644 --- a/server/fakerut.cpp +++ b/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))) \