Fixed GLX context creation and reorganized code to be more understandable.

src/SFML/Window/Unix/GlxContext.cpp

@@ -45,20 +45,30 @@ m_ownsWindow(true)
     // Open a connection with the X server
     m_display = OpenDisplay();
     m_connection = XGetXCBConnection(m_display);
+    xcb_screen_t* screen = XCBScreenOfDisplay(m_connection, DefaultScreen(m_display));
+
+    // Choose the visual according to the context settings
+    XVisualInfo visualInfo = selectBestVisual(m_display, VideoMode::getDesktopMode().bitsPerPixel, ContextSettings());
+
+    // Define the window attributes
+    xcb_colormap_t colormap = xcb_generate_id(m_connection);
+    xcb_create_colormap(m_connection, XCB_COLORMAP_ALLOC_NONE, colormap, screen->root, visualInfo.visualid);
+    const uint32_t value_list[] = {colormap};
 
     // Create a dummy window (disabled and hidden)
-    xcb_screen_t* screen = XCBScreenOfDisplay(m_connection, DefaultScreen(m_display));
     m_window = xcb_generate_id(m_connection);
     xcb_create_window(
         m_connection,
-        screen->root_depth,
+        static_cast<uint8_t>(visualInfo.depth),
-        m_window, screen->root,
+        m_window,
+        screen->root,
         0, 0,
         1, 1,
         0,
         XCB_WINDOW_CLASS_INPUT_OUTPUT,
-        screen->root_visual,
+        visualInfo.visualid,
-        0, NULL
+        XCB_CW_COLORMAP,
+        value_list
     );
 
     // Create the context
@@ -95,20 +105,30 @@ m_ownsWindow(true)
     // Open a connection with the X server
     m_display = OpenDisplay();
     m_connection = XGetXCBConnection(m_display);
+    xcb_screen_t* screen = XCBScreenOfDisplay(m_connection, DefaultScreen(m_display));
+
+    // Choose the visual according to the context settings
+    XVisualInfo visualInfo = selectBestVisual(m_display, VideoMode::getDesktopMode().bitsPerPixel, settings);
+
+    // Define the window attributes
+    xcb_colormap_t colormap = xcb_generate_id(m_connection);
+    xcb_create_colormap(m_connection, XCB_COLORMAP_ALLOC_NONE, colormap, screen->root, visualInfo.visualid);
+    const uint32_t value_list[] = {colormap};
 
     // Create the hidden window
-    xcb_screen_t* screen = XCBScreenOfDisplay(m_connection, DefaultScreen(m_display));
     m_window = xcb_generate_id(m_connection);
     xcb_create_window(
         m_connection,
-        screen->root_depth,
+        static_cast<uint8_t>(visualInfo.depth),
-        m_window, screen->root,
+        m_window,
+        screen->root,
         0, 0,
         width, height,
         0,
         XCB_WINDOW_CLASS_INPUT_OUTPUT,
-        screen->root_visual,
+        visualInfo.visualid,
-        0, NULL
+        XCB_CW_COLORMAP,
+        value_list
     );
 
     // Create the context
@@ -167,6 +187,80 @@ void GlxContext::setVerticalSyncEnabled(bool enabled)
 ////////////////////////////////////////////////////////////
 XVisualInfo GlxContext::selectBestVisual(::Display* display, unsigned int bitsPerPixel, const ContextSettings& settings)
 {
+    // First try to get a visual via an associated GLXFBConfig (requires GLX 1.3 or greater)
+    // There are no GLX versions prior to 1.0
+    int major, minor;
+    if (glXQueryVersion(display, &major, &minor) && ((major > 1) || (minor >= 3)))
+    {
+        // Select a GLXFB config that matches the requested context settings
+        int nbConfigs = 0;
+        int fbAttributes[] =
+        {
+            GLX_DEPTH_SIZE, static_cast<int>(settings.depthBits),
+            GLX_STENCIL_SIZE, static_cast<int>(settings.stencilBits),
+            GLX_SAMPLE_BUFFERS, settings.antialiasingLevel > 0,
+            GLX_SAMPLES, static_cast<int>(settings.antialiasingLevel),
+            GLX_RED_SIZE, 8,
+            GLX_GREEN_SIZE, 8,
+            GLX_BLUE_SIZE, 8,
+            GLX_ALPHA_SIZE, bitsPerPixel == 32 ? 8 : 0,
+            GLX_DOUBLEBUFFER, True,
+            GLX_X_RENDERABLE, True,
+            GLX_DRAWABLE_TYPE, GLX_WINDOW_BIT,
+            GLX_RENDER_TYPE, GLX_RGBA_BIT,
+            GLX_CONFIG_CAVEAT, GLX_NONE,
+            None
+        };
+        GLXFBConfig* configs = glXChooseFBConfig(display, DefaultScreen(display), fbAttributes, &nbConfigs);
+
+        int bestScore = 0xFFFF;
+        XVisualInfo bestVisual;
+
+        for (int i = 0; configs && (i < nbConfigs); ++i)
+        {
+            XVisualInfo* visual = glXGetVisualFromFBConfig(display, configs[i]);
+            
+            // We only want FBConfigs with associated X visuals (most have one)
+            if (!visual)
+                continue;
+
+            // Check mandatory attributes
+            int doubleBuffer;
+            glXGetFBConfigAttrib(display, configs[i], GLX_DOUBLEBUFFER, &doubleBuffer);
+            if (!doubleBuffer)
+                continue;
+
+            // Extract the components of the current visual
+            int red, green, blue, alpha, depth, stencil, multiSampling, samples;
+            glXGetFBConfigAttrib(display, configs[i], GLX_RED_SIZE,           &red);
+            glXGetFBConfigAttrib(display, configs[i], GLX_GREEN_SIZE,         &green);
+            glXGetFBConfigAttrib(display, configs[i], GLX_BLUE_SIZE,          &blue);
+            glXGetFBConfigAttrib(display, configs[i], GLX_ALPHA_SIZE,         &alpha);
+            glXGetFBConfigAttrib(display, configs[i], GLX_DEPTH_SIZE,         &depth);
+            glXGetFBConfigAttrib(display, configs[i], GLX_STENCIL_SIZE,       &stencil);
+            glXGetFBConfigAttrib(display, configs[i], GLX_SAMPLE_BUFFERS_ARB, &multiSampling);
+            glXGetFBConfigAttrib(display, configs[i], GLX_SAMPLES_ARB,        &samples);
+
+            // Evaluate the visual
+            int color = red + green + blue + alpha;
+            int score = evaluateFormat(bitsPerPixel, settings, color, depth, stencil, multiSampling ? samples : 0);
+
+            // If it's better than the current best, make it the new best
+            if (score < bestScore)
+            {
+                bestScore = score;
+                bestVisual = *visual;
+            }
+            
+            XFree(visual);
+        }
+        
+        XFree(configs);
+
+        if (bestScore < 0xFFFF)
+            return bestVisual;
+    }
+
     // Retrieve all the visuals
     int count;
     XVisualInfo* visuals = XGetVisualInfo(display, 0, NULL, &count);
@@ -226,82 +320,24 @@ void GlxContext::createContext(GlxContext* shared, unsigned int bitsPerPixel, co
     // Save the creation settings
     m_settings = settings;
 
-    // Get the attributes of the target window
-    XWindowAttributes windowAttributes;
-    if (XGetWindowAttributes(m_display, m_window, &windowAttributes) == 0)
-    {
-        err() << "Failed to get the window attributes" << std::endl;
-        return;
-    }
-
-    // Setup the visual infos to match
-    XVisualInfo tpl;
-    tpl.depth    = windowAttributes.depth;
-    tpl.visualid = XVisualIDFromVisual(windowAttributes.visual);
-    tpl.screen   = DefaultScreen(m_display);
-
-    // Get all the visuals matching the template
-    int nbVisuals = 0;
-    XVisualInfo* visuals = XGetVisualInfo(m_display, VisualDepthMask | VisualIDMask | VisualScreenMask, &tpl, &nbVisuals);
-    if (!visuals || (nbVisuals == 0))
-    {
-        if (visuals)
-            XFree(visuals);
-        err() << "There is no valid visual for the selected screen" << std::endl;
-        return;
-    }
-
-    // Find the best visual
-    int          bestScore  = 0xFFFF;
-    XVisualInfo* bestVisual = NULL;
-    for (int i = 0; i < nbVisuals; ++i)
-    {
-        // Get the current visual attributes
-        int RGBA, doubleBuffer, red, green, blue, alpha, depth, stencil, multiSampling, samples;
-        glXGetConfig(m_display, &visuals[i], GLX_RGBA,               &RGBA);
-        glXGetConfig(m_display, &visuals[i], GLX_DOUBLEBUFFER,       &doubleBuffer); 
-        glXGetConfig(m_display, &visuals[i], GLX_RED_SIZE,           &red);
-        glXGetConfig(m_display, &visuals[i], GLX_GREEN_SIZE,         &green); 
-        glXGetConfig(m_display, &visuals[i], GLX_BLUE_SIZE,          &blue); 
-        glXGetConfig(m_display, &visuals[i], GLX_ALPHA_SIZE,         &alpha); 
-        glXGetConfig(m_display, &visuals[i], GLX_DEPTH_SIZE,         &depth);        
-        glXGetConfig(m_display, &visuals[i], GLX_STENCIL_SIZE,       &stencil);
-        glXGetConfig(m_display, &visuals[i], GLX_SAMPLE_BUFFERS_ARB, &multiSampling);        
-        glXGetConfig(m_display, &visuals[i], GLX_SAMPLES_ARB,        &samples);
-
-        // First check the mandatory parameters
-        if ((RGBA == 0) || (doubleBuffer == 0))
-            continue;
-
-        // Evaluate the current configuration
-        int color = red + green + blue + alpha;
-        int score = evaluateFormat(bitsPerPixel, m_settings, color, depth, stencil, multiSampling ? samples : 0);
-
-        // Keep it if it's better than the current best
-        if (score < bestScore)
-        {
-            bestScore  = score;
-            bestVisual = &visuals[i];
-        }
-    }
-
-    // Make sure that we have found a visual
-    if (!bestVisual)
-    {
-        err() << "Failed to find a suitable pixel format for the window -- cannot create OpenGL context" << std::endl;
-        return;
-    }
-
     // Get the context to share display lists with
     GLXContext toShare = shared ? shared->m_context : NULL;
 
-    // Create the OpenGL context -- first try context versions >= 3.0 if it is requested (they require special code)
+    // There are no GLX versions prior to 1.0
-    while (!m_context && (m_settings.majorVersion >= 3))
+    int major = 0;
+    int minor = 0;
+    
+    if (!glXQueryVersion(m_display, &major, &minor))
+        err() << "Failed to query GLX version, limited to legacy context creation" << std::endl;
+
+    // Create the OpenGL context -- first try context versions >= 3.0 if it is requested (they require special code and GLX 1.3 or greater)
+    if ((m_settings.majorVersion >= 3) && ((major > 1) || (minor >= 3)))
     {
         const GLubyte* name = reinterpret_cast<const GLubyte*>("glXCreateContextAttribsARB");
         PFNGLXCREATECONTEXTATTRIBSARBPROC glXCreateContextAttribsARB = reinterpret_cast<PFNGLXCREATECONTEXTATTRIBSARBPROC>(glXGetProcAddress(name));
         if (glXCreateContextAttribsARB)
         {
+            // Select a GLXFB config that matches the requested context settings
             int nbConfigs = 0;
             int fbAttributes[] =
             {
@@ -322,39 +358,98 @@ void GlxContext::createContext(GlxContext* shared, unsigned int bitsPerPixel, co
             };
             GLXFBConfig* configs = glXChooseFBConfig(m_display, DefaultScreen(m_display), fbAttributes, &nbConfigs);
 
-            if (configs && nbConfigs)
+            int bestScore = 0xFFFF;
+            GLXFBConfig* bestConfig;
+
+            for (int i = 0; configs && (i < nbConfigs); ++i)
             {
-                // Create the context
+                XVisualInfo* visual = glXGetVisualFromFBConfig(m_display, configs[i]);
-                int attributes[] =
+                
+                // We only want FBConfigs with associated X visuals (most have one)
+                // We do this to match what was used during window creation
+                if (!visual)
+                    continue;
+                
+                // We won't need the visual for glXCreateContextAttribsARB, so get rid of it
+                XFree(visual);
+
+                // Check mandatory attributes
+                int doubleBuffer;
+                glXGetFBConfigAttrib(m_display, configs[i], GLX_DOUBLEBUFFER, &doubleBuffer);
+                if (!doubleBuffer)
+                    continue;
+
+                // Extract the components of the current visual
+                int red, green, blue, alpha, depth, stencil, multiSampling, samples;
+                glXGetFBConfigAttrib(m_display, configs[i], GLX_RED_SIZE,           &red);
+                glXGetFBConfigAttrib(m_display, configs[i], GLX_GREEN_SIZE,         &green);
+                glXGetFBConfigAttrib(m_display, configs[i], GLX_BLUE_SIZE,          &blue);
+                glXGetFBConfigAttrib(m_display, configs[i], GLX_ALPHA_SIZE,         &alpha);
+                glXGetFBConfigAttrib(m_display, configs[i], GLX_DEPTH_SIZE,         &depth);
+                glXGetFBConfigAttrib(m_display, configs[i], GLX_STENCIL_SIZE,       &stencil);
+                glXGetFBConfigAttrib(m_display, configs[i], GLX_SAMPLE_BUFFERS_ARB, &multiSampling);
+                glXGetFBConfigAttrib(m_display, configs[i], GLX_SAMPLES_ARB,        &samples);
+
+                // Evaluate the visual
+                int color = red + green + blue + alpha;
+                int score = evaluateFormat(bitsPerPixel, settings, color, depth, stencil, multiSampling ? samples : 0);
+
+                // If it's better than the current best, make it the new best
+                if (score < bestScore)
                 {
-                    GLX_CONTEXT_MAJOR_VERSION_ARB, static_cast<int>(m_settings.majorVersion),
+                    bestScore = score;
-                    GLX_CONTEXT_MINOR_VERSION_ARB, static_cast<int>(m_settings.minorVersion),
+                    bestConfig = &configs[i];
-                    GLX_CONTEXT_PROFILE_MASK_ARB, GLX_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB,
+                }
-                    0, 0
+            }
-                };
+
-                m_context = glXCreateContextAttribsARB(m_display, configs[0], toShare, true, attributes);
+            if (bestScore < 0xFFFF)
+            {
+                while (!m_context && (m_settings.majorVersion >= 3))
+                {
+                    // Create the context
+                    int attributes[] =
+                    {
+                        GLX_CONTEXT_MAJOR_VERSION_ARB, static_cast<int>(m_settings.majorVersion),
+                        GLX_CONTEXT_MINOR_VERSION_ARB, static_cast<int>(m_settings.minorVersion),
+                        GLX_CONTEXT_PROFILE_MASK_ARB, GLX_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB,
+                        0, 0
+                    };
+                    m_context = glXCreateContextAttribsARB(m_display, *bestConfig, toShare, true, attributes);
+
+                    if (m_context)
+                    {
+                        // Update the creation settings from the chosen format
+                        int depth, stencil, multiSampling, samples;
+                        glXGetFBConfigAttrib(m_display, *bestConfig, GLX_DEPTH_SIZE,         &depth);
+                        glXGetFBConfigAttrib(m_display, *bestConfig, GLX_STENCIL_SIZE,       &stencil);
+                        glXGetFBConfigAttrib(m_display, *bestConfig, GLX_SAMPLE_BUFFERS_ARB, &multiSampling);
+                        glXGetFBConfigAttrib(m_display, *bestConfig, GLX_SAMPLES_ARB,        &samples);
+                        m_settings.depthBits         = static_cast<unsigned int>(depth);
+                        m_settings.stencilBits       = static_cast<unsigned int>(stencil);
+                        m_settings.antialiasingLevel = multiSampling ? samples : 0;
+                    }
+                    else
+                    {
+                        // If we couldn't create the context, lower the version number and try again -- stop at 3.0
+                        // Invalid version numbers will be generated by this algorithm (like 3.9), but we really don't care
+                        if (m_settings.minorVersion > 0)
+                        {
+                            // If the minor version is not 0, we decrease it and try again
+                            m_settings.minorVersion--;
+                        }
+                        else
+                        {
+                            // If the minor version is 0, we decrease the major version
+                            m_settings.majorVersion--;
+                            m_settings.minorVersion = 9;
+                        }
+                    }
+                }
             }
 
             if (configs)
                 XFree(configs);
         }
-
-        // If we couldn't create the context, lower the version number and try again -- stop at 3.0
-        // Invalid version numbers will be generated by this algorithm (like 3.9), but we really don't care
-        if (!m_context)
-        {
-            if (m_settings.minorVersion > 0)
-            {
-                // If the minor version is not 0, we decrease it and try again
-                m_settings.minorVersion--;
-            }
-            else
-            {
-                // If the minor version is 0, we decrease the major version
-                m_settings.majorVersion--;
-                m_settings.minorVersion = 9;
-            }
-        }
     }
 
     // If the OpenGL >= 3.0 context failed or if we don't want one, create a regular OpenGL 1.x/2.x context
@@ -364,31 +459,42 @@ void GlxContext::createContext(GlxContext* shared, unsigned int bitsPerPixel, co
         m_settings.majorVersion = 2;
         m_settings.minorVersion = 0;
 
-        m_context = glXCreateContext(m_display, bestVisual, toShare, true);
+        // Retrieve the attributes of the target window
+        XWindowAttributes windowAttributes;
+        if (XGetWindowAttributes(m_display, m_window, &windowAttributes) == 0)
+        {
+            err() << "Failed to get the window attributes" << std::endl;
+            return;
+        }
+
+        // Get its visuals
+        XVisualInfo tpl;
+        tpl.screen   = DefaultScreen(m_display);
+        tpl.visualid = XVisualIDFromVisual(windowAttributes.visual);
+        int nbVisuals = 0;
+        XVisualInfo* visualInfo = XGetVisualInfo(m_display, VisualIDMask | VisualScreenMask, &tpl, &nbVisuals);
+
+        // Create the context, using the target window's visual
+        m_context = glXCreateContext(m_display, visualInfo, toShare, true);
         if (!m_context)
         {
             err() << "Failed to create an OpenGL context for this window" << std::endl;
             return;
         }
+
+        // Update the creation settings from the chosen format
+        int depth, stencil, multiSampling, samples;
+        glXGetConfig(m_display, visualInfo, GLX_DEPTH_SIZE,         &depth);
+        glXGetConfig(m_display, visualInfo, GLX_STENCIL_SIZE,       &stencil);
+        glXGetConfig(m_display, visualInfo, GLX_SAMPLE_BUFFERS_ARB, &multiSampling);
+        glXGetConfig(m_display, visualInfo, GLX_SAMPLES_ARB,        &samples);
+        m_settings.depthBits         = static_cast<unsigned int>(depth);
+        m_settings.stencilBits       = static_cast<unsigned int>(stencil);
+        m_settings.antialiasingLevel = multiSampling ? samples : 0;
+
+        // Free the visual info
+        XFree(visualInfo);
     }
-
-    // Update the creation settings from the chosen format
-    int depth, stencil, multiSampling, samples;
-    glXGetConfig(m_display, bestVisual, GLX_DEPTH_SIZE,         &depth);
-    glXGetConfig(m_display, bestVisual, GLX_STENCIL_SIZE,       &stencil);
-    glXGetConfig(m_display, bestVisual, GLX_SAMPLE_BUFFERS_ARB, &multiSampling);        
-    glXGetConfig(m_display, bestVisual, GLX_SAMPLES_ARB,        &samples);
-    m_settings.depthBits         = static_cast<unsigned int>(depth);
-    m_settings.stencilBits       = static_cast<unsigned int>(stencil);
-    m_settings.antialiasingLevel = multiSampling ? samples : 0;
-
-    // Change the target window's colormap so that it matches the context's one
-    ::Window root = RootWindow(m_display, DefaultScreen(m_display));
-    Colormap colorMap = XCreateColormap(m_display, root, bestVisual->visual, AllocNone);
-    XSetWindowColormap(m_display, m_window, colorMap);
-
-    // Free the temporary visuals array
-    XFree(visuals);
 }
 
 } // namespace priv

src/SFML/Window/Unix/WindowImplX11.cpp

@@ -140,7 +140,7 @@ m_useSizeHints(false)
 
 
 ////////////////////////////////////////////////////////////
-WindowImplX11::WindowImplX11(VideoMode mode, const String& title, unsigned long style, const ContextSettings& /*settings*/) :
+WindowImplX11::WindowImplX11(VideoMode mode, const String& title, unsigned long style, const ContextSettings& settings) :
 m_window      (0),
 m_inputMethod (NULL),
 m_inputContext(NULL),
@@ -176,8 +176,13 @@ m_useSizeHints(false)
     if (fullscreen)
         switchToFullscreen(mode);
 
+    // Choose the visual according to the context settings
+    XVisualInfo visualInfo = ContextType::selectBestVisual(m_display, mode.bitsPerPixel, settings);
+
     // Define the window attributes
-    const uint32_t value_list[] = {fullscreen, static_cast<uint32_t>(eventMask)};
+    xcb_colormap_t colormap = xcb_generate_id(m_connection);
+    xcb_create_colormap(m_connection, XCB_COLORMAP_ALLOC_NONE, colormap, m_screen->root, visualInfo.visualid);
+    const uint32_t value_list[] = {fullscreen, static_cast<uint32_t>(eventMask), colormap};
 
     // Create the window
     m_window = xcb_generate_id(m_connection);
@@ -186,15 +191,15 @@ m_useSizeHints(false)
         m_connection,
         xcb_create_window_checked(
             m_connection,
-            XCB_COPY_FROM_PARENT,
+            static_cast<uint8_t>(visualInfo.depth),
             m_window,
             m_screen->root,
             left, top,
             width, height,
             0,
             XCB_WINDOW_CLASS_INPUT_OUTPUT,
-            XCB_COPY_FROM_PARENT,
+            visualInfo.visualid,
-            XCB_CW_EVENT_MASK | XCB_CW_OVERRIDE_REDIRECT,
+            XCB_CW_EVENT_MASK | XCB_CW_OVERRIDE_REDIRECT | XCB_CW_COLORMAP,
             value_list
         )
     ));