Fixed requesting an unsupported OpenGL context version causing X to close the application, fixed GlContext initialization not updating settings properly, added error checks to GLLoader.cpp and fixed GL errors occurring when using a >= 3.0 OpenGL context.

src/SFML/Graphics/GLLoader.cpp

@@ -26,11 +26,24 @@
 // Headers
 ////////////////////////////////////////////////////////////
 #include <SFML/Graphics/GLLoader.hpp>
+#include <SFML/Graphics/GLCheck.hpp>
 #include <SFML/Window/Context.hpp>
 #include <cstdlib>
 #include <cstring>
 #include <cstddef>
 
+#if !defined(GL_MAJOR_VERSION)
+    #define GL_MAJOR_VERSION 0x821B
+#endif
+
+#if !defined(GL_MINOR_VERSION)
+    #define GL_MINOR_VERSION 0x821C
+#endif
+
+#if !defined(GL_NUM_EXTENSIONS)
+    #define GL_NUM_EXTENSIONS 0x821D
+#endif
+
 static sf::GlFunctionPointer IntGetProcAddress(const char* name)
 {
     return sf::Context::getFunction(name);
@@ -395,6 +408,9 @@ static void LoadExtByName(const char *extensionName)
 
 static void ProcExtsFromExtString(const char *strExtList)
 {
+    if (!strExtList)
+        strExtList = "";
+
     size_t iExtListLen = strlen(strExtList);
     const char *strExtListEnd = strExtList + iExtListLen;
     const char *strCurrPos = strExtList;
@@ -432,7 +448,38 @@ int sfogl_LoadFunctions()
     int numFailed = 0;
     ClearExtensionVars();
 
-    ProcExtsFromExtString((const char *)glGetString(GL_EXTENSIONS));
+    const char* extensionString = NULL;
+
+    if(sfogl_GetMajorVersion() < 3)
+    {
+        // Try to load the < 3.0 way
+        glCheck(extensionString = (const char *)glGetString(GL_EXTENSIONS));
+
+        ProcExtsFromExtString(extensionString);
+    }
+    else
+    {
+        // Try to load the >= 3.0 way
+        const GLubyte* (CODEGEN_FUNCPTR *glGetStringiFunc)(GLenum, GLuint) = NULL;
+        glGetStringiFunc = (const GLubyte* (CODEGEN_FUNCPTR *)(GLenum, GLuint))IntGetProcAddress("glGetStringi");
+
+        if (glGetStringiFunc)
+        {
+            int numExtensions = 0;
+            glCheck(glGetIntegerv(GL_NUM_EXTENSIONS, &numExtensions));
+
+            if (numExtensions)
+            {
+                for (unsigned int i = 0; i < static_cast<unsigned int>(numExtensions); ++i)
+                {
+                    glCheck(extensionString = (const char *)glGetStringiFunc(GL_EXTENSIONS, i));
+
+                    ProcExtsFromExtString(extensionString);
+                }
+            }
+        }
+    }
+
     numFailed = Load_Version_1_2();
 
     if(numFailed == 0)
@@ -481,7 +528,16 @@ static void ParseVersionFromString(int *pOutMajor, int *pOutMinor, const char *s
 
 static void GetGLVersion()
 {
-    ParseVersionFromString(&g_major_version, &g_minor_version, (const char*)glGetString(GL_VERSION));
+    glGetIntegerv(GL_MAJOR_VERSION, &g_major_version);
+    glGetIntegerv(GL_MINOR_VERSION, &g_minor_version);
+
+    // Check if we have to retrieve the context version using the legacy method
+    if (glGetError() == GL_INVALID_ENUM)
+    {
+        const char* versionString = NULL;
+        glCheck(versionString = (const char*)glGetString(GL_VERSION));
+        ParseVersionFromString(&g_major_version, &g_minor_version, versionString);
+    }
 }
 
 int sfogl_GetMajorVersion()

src/SFML/Window/GlContext.cpp

@@ -73,6 +73,14 @@
     #define GL_MULTISAMPLE 0x809D
 #endif
 
+#if !defined(GL_MAJOR_VERSION)
+    #define GL_MAJOR_VERSION 0x821B
+#endif
+
+#if !defined(GL_MINOR_VERSION)
+    #define GL_MINOR_VERSION 0x821C
+#endif
+
 #if !defined(GL_CONTEXT_FLAGS)
     #define GL_CONTEXT_FLAGS 0x821E
 #endif
@@ -333,6 +341,21 @@ void GlContext::initialize()
     setActive(true);
 
     // Retrieve the context version number
+    int majorVersion = 0;
+    int minorVersion = 0;
+
+    // Try the new way first
+    glGetIntegerv(GL_MAJOR_VERSION, &majorVersion);
+    glGetIntegerv(GL_MINOR_VERSION, &minorVersion);
+
+    if (glGetError() != GL_INVALID_ENUM)
+    {
+        m_settings.majorVersion = static_cast<unsigned int>(majorVersion);
+        m_settings.minorVersion = static_cast<unsigned int>(minorVersion);
+    }
+    else
+    {
+        // Try the old way
         const GLubyte* version = glGetString(GL_VERSION);
         if (version)
         {
@@ -346,29 +369,27 @@ void GlContext::initialize()
             m_settings.majorVersion = 2;
             m_settings.minorVersion = 1;
         }
+    }
 
-    // Verify non-default settings, just to be extra sure
+    m_settings.attributeFlags = ContextSettings::Default;
-    if (m_settings.attributeFlags & ContextSettings::Debug)
+
+    if (m_settings.majorVersion >= 3)
     {
         // Retrieve the context flags
         int flags = 0;
         glGetIntegerv(GL_CONTEXT_FLAGS, &flags);
 
-        if (!(flags & GL_CONTEXT_FLAG_DEBUG_BIT))
+        if (flags & GL_CONTEXT_FLAG_DEBUG_BIT)
-        {
+            m_settings.attributeFlags |= ContextSettings::Debug;
-            m_settings.attributeFlags ^= ContextSettings::Debug;
-        }
-    }
 
-    if (m_settings.attributeFlags & ContextSettings::Core)
+        if ((m_settings.majorVersion > 3) || (m_settings.minorVersion >= 2))
         {
             // Retrieve the context profile
             int profile = 0;
             glGetIntegerv(GL_CONTEXT_PROFILE_MASK, &profile);
 
-        if (!(profile & GL_CONTEXT_CORE_PROFILE_BIT))
+            if (profile & GL_CONTEXT_CORE_PROFILE_BIT)
-        {
+                m_settings.attributeFlags |= ContextSettings::Core;
-            m_settings.attributeFlags ^= ContextSettings::Core;
         }
     }
 

src/SFML/Window/Unix/GlxContext.cpp

@@ -28,8 +28,51 @@
 #include <SFML/Window/Unix/GlxContext.hpp>
 #include <SFML/Window/Unix/WindowImplX11.hpp>
 #include <SFML/Window/Unix/Display.hpp>
+#include <SFML/System/Mutex.hpp>
+#include <SFML/System/Lock.hpp>
 #include <SFML/System/Err.hpp>
 
+#if !defined(GLX_DEBUGGING) && defined(SFML_DEBUG)
+    // Enable this to print messages to err() everytime GLX produces errors
+    //#define GLX_DEBUGGING
+#endif
+
+namespace
+{
+    sf::Mutex glxErrorMutex;
+    bool glxErrorOccurred = false;
+
+    int HandleXError(::Display*, XErrorEvent*)
+    {
+        glxErrorOccurred = true;
+        return 0;
+    }
+
+    class GlxErrorHandler
+    {
+    public:
+
+        GlxErrorHandler(::Display* display) :
+        m_display(display),
+        m_lock   (glxErrorMutex)
+        {
+            glxErrorOccurred = false;
+            m_previousHandler = XSetErrorHandler(HandleXError);
+        }
+
+        ~GlxErrorHandler()
+        {
+            XSync(m_display, False);
+            XSetErrorHandler(m_previousHandler);
+        }
+
+    private:
+        sf::Lock   m_lock;
+        ::Display* m_display;
+        int      (*m_previousHandler)(::Display*, XErrorEvent*);
+    };
+}
+
 
 namespace sf
 {
@@ -160,9 +203,18 @@ GlxContext::~GlxContext()
     // Destroy the context
     if (m_context)
     {
+#if defined(GLX_DEBUGGING)
+        GlxErrorHandler handler(m_display);
+#endif
+
         if (glXGetCurrentContext() == m_context)
             glXMakeCurrent(m_display, None, NULL);
         glXDestroyContext(m_display, m_context);
+
+#if defined(GLX_DEBUGGING)
+        if (glxErrorOccurred)
+            err() << "GLX error in GlxContext::~GlxContext()" << std::endl;
+#endif
     }
 
     // Destroy the window if we own it
@@ -187,15 +239,38 @@ GlFunctionPointer GlxContext::getFunction(const char* name)
 ////////////////////////////////////////////////////////////
 bool GlxContext::makeCurrent()
 {
-    return m_context && glXMakeCurrent(m_display, m_window, m_context);
+    if (!m_context)
+        return false;
+
+#if defined(GLX_DEBUGGING)
+    GlxErrorHandler handler(m_display);
+#endif
+
+    bool result = glXMakeCurrent(m_display, m_window, m_context);
+
+#if defined(GLX_DEBUGGING)
+    if (glxErrorOccurred)
+        err() << "GLX error in GlxContext::makeCurrent()" << std::endl;
+#endif
+
+    return result;
 }
 
 
 ////////////////////////////////////////////////////////////
 void GlxContext::display()
 {
+#if defined(GLX_DEBUGGING)
+    GlxErrorHandler handler(m_display);
+#endif
+
     if (m_window)
         glXSwapBuffers(m_display, m_window);
+
+#if defined(GLX_DEBUGGING)
+    if (glxErrorOccurred)
+        err() << "GLX error in GlxContext::display()" << std::endl;
+#endif
 }
 
 
@@ -307,6 +382,10 @@ XVisualInfo GlxContext::selectBestVisual(::Display* display, unsigned int bitsPe
     XVisualInfo* visuals = XGetVisualInfo(display, 0, NULL, &count);
     if (visuals)
     {
+#if defined(GLX_DEBUGGING)
+        GlxErrorHandler handler(display);
+#endif
+
         // Evaluate all the returned visuals, and pick the best one
         int bestScore = 0xFFFF;
         XVisualInfo bestVisual;
@@ -341,6 +420,11 @@ XVisualInfo GlxContext::selectBestVisual(::Display* display, unsigned int bitsPe
             }
         }
 
+#if defined(GLX_DEBUGGING)
+        if (glxErrorOccurred)
+            err() << "GLX error in GlxContext::selectBestVisual()" << std::endl;
+#endif
+
         // Free the array of visuals
         XFree(visuals);
 
@@ -490,6 +574,11 @@ void GlxContext::createContext(GlxContext* shared, unsigned int bitsPerPixel, co
                         GLX_CONTEXT_FLAGS_ARB,         debug,
                         0,                             0
                     };
+
+                    // RAII GLX error handler (we simply ignore errors here)
+                    // On an error, glXCreateContextAttribsARB will return 0 anyway
+                    GlxErrorHandler handler(m_display);
+
                     m_context = glXCreateContextAttribsARB(m_display, *bestConfig, toShare, true, attributes);
                 }
                 else
@@ -507,11 +596,20 @@ void GlxContext::createContext(GlxContext* shared, unsigned int bitsPerPixel, co
                         GLX_CONTEXT_MINOR_VERSION_ARB, static_cast<int>(m_settings.minorVersion),
                         0,                             0
                     };
+
+                    // RAII GLX error handler (we simply ignore errors here)
+                    // On an error, glXCreateContextAttribsARB will return 0 anyway
+                    GlxErrorHandler handler(m_display);
+
                     m_context = glXCreateContextAttribsARB(m_display, *bestConfig, toShare, true, attributes);
                 }
 
                 if (m_context)
                 {
+#if defined(GLX_DEBUGGING)
+                    GlxErrorHandler handler(m_display);
+#endif
+
                     // Update the creation settings from the chosen format
                     int depth, stencil, multiSampling, samples;
                     glXGetFBConfigAttrib(m_display, *bestConfig, GLX_DEPTH_SIZE,         &depth);
@@ -521,6 +619,11 @@ void GlxContext::createContext(GlxContext* shared, unsigned int bitsPerPixel, co
                     m_settings.depthBits         = static_cast<unsigned int>(depth);
                     m_settings.stencilBits       = static_cast<unsigned int>(stencil);
                     m_settings.antialiasingLevel = multiSampling ? samples : 0;
+
+#if defined(GLX_DEBUGGING)
+                    if (glxErrorOccurred)
+                        err() << "GLX error in GlxContext::createContext()" << std::endl;
+#endif
                 }
                 else
                 {
@@ -554,6 +657,10 @@ void GlxContext::createContext(GlxContext* shared, unsigned int bitsPerPixel, co
         }
     }
 
+#if defined(GLX_DEBUGGING)
+    GlxErrorHandler handler(m_display);
+#endif
+
     // If glXCreateContextAttribsARB failed, use glXCreateContext
     if (!m_context)
     {
@@ -608,6 +715,11 @@ void GlxContext::createContext(GlxContext* shared, unsigned int bitsPerPixel, co
         // Free the visual info
         XFree(visualInfo);
     }
+
+#if defined(GLX_DEBUGGING)
+    if (glxErrorOccurred)
+        err() << "GLX error in GlxContext::createContext()" << std::endl;
+#endif
 }
 
 } // namespace priv