Improve new wxGLCanvas compatibility with old hardware and code

Combined patch with the following changes:
- Don't add wxGLAttributes::Defaults() when the attributes-list is NULL.
- Add display default attributes used in wx versions before 3.1 when the
  attributes-list is NULL. These attributes are different for each platform.
- Fix wxMSW PixelFormatDescriptor initialization.
- Don't set color buffers when RGBA() is used.
- Fix setting colour sizes in OS X and a few other fixes.
- Make documentation more clear about these subjects.

Closes #17425.

include/wx/glcanvas.h

@@ -161,6 +161,11 @@ public:
     wxGLAttributes& Samplers(int val);
     wxGLAttributes& FrameBuffersRGB();
     void EndList(); // No more values can be chained
+
+    // This function is undocumented and can not be chained on purpose!
+    // To keep backwards compatibility with versions before wx3.1 we add here
+    // the default values used in those versions for the case of NULL list.
+    void AddDefaultsForWXBefore31();
 };
 
 // ----------------------------------------------------------------------------

interface/wx/glcanvas.h

@@ -130,6 +130,7 @@ public:
     @note Not all of platform-dependant available attributes are implemented in
           wxWidgets. You can set other attributes by using AddAttribute() and
           AddAttribBits() functions inherited from the base wxGLAttribsBase class.
+          While WGL_/GLX_/NS attributes can be added, PFD_ (for MSW) can not.
 
     @since 3.1.0
 
@@ -142,13 +143,14 @@ class wxGLAttributes : public wxGLAttribsBase
 {
 public:
     /**
-        Use true color (8 bits for each color plus 8 bits for alpha channel).
+        Use true colour instead of colour index rendering for each pixel.
+        It makes no effect for OS X.
     */
     wxGLAttributes& RGBA();
 
     /**
-        Specifies the number of bits for buffer when it isn't a RGBA buffer.
+        Specifies the number of bits for colour buffer. For RGBA it's
-        It makes no effect for OS X.
+        normally the sum of the bits per each component.
 
         @param val
         The number of bits.
@@ -164,12 +166,12 @@ public:
     wxGLAttributes& Level(int val);
 
     /**
-        Requests using double buffering if present.
+        Requests using double buffering.
     */
     wxGLAttributes& DoubleBuffer();
 
     /**
-        Use stereoscopic display if present.
+        Use stereoscopic display.
     */
     wxGLAttributes& Stereo();
 
@@ -182,7 +184,8 @@ public:
     wxGLAttributes& AuxBuffers(int val);
 
     /**
-        Specifies the minimal number of bits for colour buffers.
+        Specifies the minimal number of bits for each colour and alpha.
+        On MSW and OSX this function also sets the size of the colour buffer.
 
         @param mRed
         The minimal number of bits for colour red.
@@ -212,7 +215,8 @@ public:
     wxGLAttributes& Stencil(int val);
 
     /**
-        Specifies the minimal number of bits for the accumulation buffer.
+        Specifies the minimal number of bits for each accumulator channel.
+        On MSW and OSX this function also sets the size of the accumulation buffer.
 
         @param mRed
         The minimal number of bits for red accumulator.
@@ -552,7 +556,7 @@ enum
     /// do not use a palette.
     WX_GL_RGBA = 1,
 
-    /// (F) Specifies the number of bits for buffer if not WX_GL_RGBA.
+    /// (F) Specifies the number of bits for colour buffer.
     WX_GL_BUFFER_SIZE,
 
     /// (F) 0 for main buffer, >0 for overlay, <0 for underlay.
@@ -816,7 +820,8 @@ public:
         This constructor is still available only for compatibility reasons.
         Please use the constructor with wxGLAttributes instead.
 
-        If @a attribList is not specified, wxGLAttributes::Defaults() is used.
+        If @a attribList is not specified, wxGLAttributes::PlatformDefaults()
+        is used, plus some other attributes (see below).
 
         @param attribList
             Array of integers. With this parameter you can set the device
@@ -871,10 +876,10 @@ public:
     static bool IsDisplaySupported(const int* attribList);
 
     /**
-        Returns true if the extension with given name is supported
+        Returns true if the extension with given name is supported.
 
         Notice that while this function is implemented for all of GLX, WGL and
-        AGL the extensions names are usually not the same for different
+        NSOpenGL the extensions names are usually not the same for different
         platforms and so the code using it still usually uses conditional
         compilation.
     */

src/common/glcmn.cpp

@@ -187,8 +187,8 @@ bool wxGLCanvasBase::ParseAttribList(const int *attribList,
 
     if ( !attribList )
     {
-        // Set default attributes
+        // Default visual attributes used in wx versions before wx3.1
-        dispAttrs.Defaults();
+        dispAttrs.AddDefaultsForWXBefore31();
         dispAttrs.EndList();
         if ( ctxAttrs )
             ctxAttrs->EndList();
@@ -196,8 +196,8 @@ bool wxGLCanvasBase::ParseAttribList(const int *attribList,
     }
 
     int src = 0;
-    int minColo[4] = {0, 0, 0, 0};
+    int minColo[4] = {-1, -1, -1, -1};
-    int minAcum[4] = {0, 0, 0, 0};
+    int minAcum[4] = {-1, -1, -1, -1};
     int num = 0;
     while ( attribList[src] )
     {
@@ -363,9 +363,9 @@ bool wxGLCanvasBase::ParseAttribList(const int *attribList,
     }
 
     // Set color and accumulation
-    if ( minColo[0] || minColo[1] || minColo[2] || minColo[3] )
+    if ( minColo[0] >= 0 || minColo[1] >= 0 || minColo[2] >= 0 || minColo[3] >= 0 )
         dispAttrs.MinRGBA(minColo[0], minColo[1], minColo[2], minColo[3]);
-    if ( minAcum[0] || minAcum[1] || minAcum[2] || minAcum[3] )
+    if ( minAcum[0] >= 0 || minAcum[1] >= 0 || minAcum[2] >= 0 || minAcum[3] >= 0 )
         dispAttrs.MinAcumRGBA(minAcum[0], minAcum[1], minAcum[2], minAcum[3]);
 
     // The attributes lists must be zero-terminated

src/msw/glcanvas.cpp

@@ -340,10 +340,6 @@ wxGLAttributes& wxGLAttributes::RGBA()
 {
     AddAttribute(WGL_PIXEL_TYPE_ARB);
     AddAttribute(WGL_TYPE_RGBA_ARB);
-    AddAttribute(WGL_COLOR_BITS_ARB);
-    AddAttribute(24);
-    AddAttribute(WGL_ALPHA_BITS_ARB);
-    AddAttribute(8);
     return *this;
 }
 
@@ -543,6 +539,12 @@ wxGLAttributes& wxGLAttributes::Defaults()
     return *this;
 }
 
+void wxGLAttributes::AddDefaultsForWXBefore31()
+{
+    // ParseAttribList() will add EndList(), don't do it now
+    RGBA().DoubleBuffer().Depth(16);
+}
+
 // ----------------------------------------------------------------------------
 // wxGLContext
 // ----------------------------------------------------------------------------
@@ -597,7 +599,11 @@ wxGLContext::wxGLContext(wxGLCanvas *win,
                                                 other ? other->m_glContext : 0,
                                                 contextAttribs);
     }
-    else
+
+
+    // Some old hardware may accept the use of this ARB, but may fail.
+    // In case of NULL attributes we'll try creating the context old-way.
+    if ( !m_glContext && (!contextAttribs || !needsARB) )
     {
         // Create legacy context
         m_glContext = wglCreateContext(win->GetHDC());
@@ -735,7 +741,7 @@ bool wxGLCanvas::Create(wxWindow *parent,
                         const int *attribList,
                         const wxPalette& palette)
 {
-    // Separate 'pixel format' attributes.
+    // Separate 'pixel format' attributes and add platform-defaults.
     // Also store context attributes for wxGLContext ctor
     wxGLAttributes dispAttrs;
     if ( ! ParseAttribList(attribList, dispAttrs, &m_GLCTXAttrs) )
@@ -753,8 +759,6 @@ bool wxGLCanvas::Create(wxWindow *parent,
                         const wxString& name,
                         const wxPalette& palette)
 {
-    // Create the window first: we will either use it as is or use it to query
-    // for multisampling support and recreate it later with another pixel format
     if ( !CreateWindow(parent, id, pos, size, style, name) )
         return false;
 
@@ -876,18 +880,29 @@ static void SetPFDForAttributes(PIXELFORMATDESCRIPTOR& pfd, const int* attrsList
     // Some defaults
     pfd.nSize =  sizeof(PIXELFORMATDESCRIPTOR);
     pfd.nVersion = 1;
-    pfd.iPixelType = PFD_TYPE_RGBA;
+    pfd.iPixelType = PFD_TYPE_COLORINDEX; // If no RGBA is specified
     pfd.iLayerType = PFD_MAIN_PLANE; // For very early MSW OpenGL
+    // Initialize rest of fields
+    pfd.dwFlags = 0;
+    pfd.cColorBits = 0;
+    pfd.cRedBits = pfd.cRedShift = pfd.cGreenBits = pfd.cGreenShift = pfd.cBlueBits = pfd.cBlueShift = 0;
+    pfd.cAlphaBits = pfd.cAlphaShift = 0;
+    pfd.cAccumBits = 0;
+    pfd.cAccumRedBits = pfd.cAccumGreenBits = pfd.cAccumBlueBits = pfd.cAccumAlphaBits = 0;
+    pfd.cDepthBits = pfd.cStencilBits = pfd.cAuxBuffers = 0;
+    pfd.bReserved = 0;
+    pfd.dwLayerMask = pfd.dwVisibleMask = pfd.dwDamageMask = 0;
 
     // We can meet some WGL_XX values not managed by wx. But the user
     // may require them. Allow here those that are also used for pfd.
+    // Notice that the user can't require PFD values, only WGL ones.
     // Color shift and transparency are not handled.
     for ( int arg = 0; attrsListWGL[arg]; )
     {
         switch ( attrsListWGL[arg++] )
         {
             case WGL_DRAW_TO_WINDOW_ARB:
-                if ( attrsListWGL[arg++] )
+                if ( attrsListWGL[arg++] ) //arg++ is for skipping 'true' attribute
                     pfd.dwFlags |= PFD_DRAW_TO_WINDOW;
                 break;
 
@@ -924,12 +939,12 @@ static void SetPFDForAttributes(PIXELFORMATDESCRIPTOR& pfd, const int* attrsList
                 break;
 
             case WGL_NUMBER_OVERLAYS_ARB:
-                pfd.bReserved &= 240;
+                // Bits 0-3
                 pfd.bReserved |= attrsListWGL[arg++] & 15;
                 break;
 
             case WGL_NUMBER_UNDERLAYS_ARB:
-                pfd.bReserved &= 15;
+                // Bits 4-7
                 pfd.bReserved |= attrsListWGL[arg++] & 240;
                 break;
 
@@ -1168,7 +1183,7 @@ bool wxGLCanvasBase::IsDisplaySupported(const int *attribList)
 {
     // We need a device context to test the pixel format, so get one
     // for the root window.
-    // Not true anymore. Keep it just in case some body uses this undocumented function
+    // Not true anymore. Keep it just in case somebody uses this undocumented function
     return wxGLCanvas::ChooseMatchingPixelFormat(ScreenHDC(), attribList) > 0;
 }
 

src/osx/glcanvas_osx.cpp

@@ -180,17 +180,17 @@ void wxGLContextAttrs::EndList()
 
 wxGLAttributes& wxGLAttributes::RGBA()
 {
-    AddAttribute(WXOpenGLPFAColorSize);
+    // No effect
-    AddAttribute(24);
-    AddAttribute(WXOpenGLPFAAlphaSize);
-    AddAttribute(8);
     return *this;
 }
 
 wxGLAttributes& wxGLAttributes::BufferSize(int val)
 {
-    // No effect
+    if ( val >= 0 )
-    wxUnusedVar(val);
+    {
+        AddAttribute(WXOpenGLPFAColorSize);
+        AddAttribute(val);
+    }
     return *this;
 }
 
@@ -225,6 +225,10 @@ wxGLAttributes& wxGLAttributes::AuxBuffers(int val)
 
 wxGLAttributes& wxGLAttributes::MinRGBA(int mRed, int mGreen, int mBlue, int mAlpha)
 {
+    // NOTE: 'NSOpenGLPixelFormat' doesn't offer a value for each of the RGBA
+    //   components, but just one for the size of the colour buffer. In order
+    //   to make wx more effective, this function _does_ set the colour buffer.
+
     int minColorBits = 0;
     if ( mRed > minColorBits )
         minColorBits = mRed;
@@ -232,6 +236,9 @@ wxGLAttributes& wxGLAttributes::MinRGBA(int mRed, int mGreen, int mBlue, int mAl
         minColorBits = mGreen;
     if ( mBlue > minColorBits )
         minColorBits = mBlue;
+    // Now that we have the R/G/B maximum, obtain a minimun for the buffer.
+    minColorBits *= 4; // 'alpha' included
+
     if ( minColorBits > 0 )
     {
         AddAttribute(WXOpenGLPFAColorSize);
@@ -268,6 +275,7 @@ wxGLAttributes& wxGLAttributes::Stencil(int val)
 
 wxGLAttributes& wxGLAttributes::MinAcumRGBA(int mRed, int mGreen, int mBlue, int mAlpha)
 {
+    // See MinRGBA() for some explanation.
     int minAcumBits = 0;
     if ( mRed > minAcumBits )
         minAcumBits = mRed;
@@ -275,15 +283,14 @@ wxGLAttributes& wxGLAttributes::MinAcumRGBA(int mRed, int mGreen, int mBlue, int
         minAcumBits = mGreen;
     if ( mBlue > minAcumBits )
         minAcumBits = mBlue;
-    if ( minAcumBits > 0 )
+    if ( mAlpha > minAcumBits )
+        minAcumBits = mAlpha;
+    minAcumBits *= 4;
+    if ( minAcumBits >= 0 )
     {
         AddAttribute(WXOpenGLPFAAccumSize);
         AddAttribute(minAcumBits);
     }
-
-    // No effect for Alpha in accumulation buffer
-    wxUnusedVar(mAlpha);
-
     return *this;
 }
 
@@ -352,6 +359,15 @@ wxGLAttributes& wxGLAttributes::Defaults()
     return *this;
 }
 
+void wxGLAttributes::AddDefaultsForWXBefore31()
+{
+    // ParseAttribList() will add EndList(), don't do it now
+    DoubleBuffer();
+    // Negative value will keep its buffer untouched
+    BufferSize(8).Depth(8).MinRGBA(-1, -1, -1, 0);
+}
+
+
 // ----------------------------------------------------------------------------
 // wxGLContext
 // ----------------------------------------------------------------------------

src/unix/glx11.cpp

@@ -428,14 +428,19 @@ wxGLAttributes& wxGLAttributes::PlatformDefaults()
 
 wxGLAttributes& wxGLAttributes::Defaults()
 {
-    RGBA().DoubleBuffer();
+    RGBA().DoubleBuffer().Depth(16).SampleBuffers(1).Samplers(4);
-    if ( wxGLCanvasX11::GetGLXVersion() < 13 )
-        Depth(1).MinRGBA(1, 1, 1, 0);
-    else
-        Depth(16).SampleBuffers(1).Samplers(4);
     return *this;
 }
 
+void wxGLAttributes::AddDefaultsForWXBefore31()
+{
+    // ParseAttribList() will add EndList(), don't do it now
+    DoubleBuffer();
+    if ( wxGLCanvasX11::GetGLXVersion() < 13 )
+        RGBA().Depth(1).MinRGBA(1, 1, 1, 0);
+    // For GLX >= 1.3 its defaults (GLX_RGBA_BIT and GLX_WINDOW_BIT) are OK
+}
+
 
 // ============================================================================
 // wxGLContext implementation
@@ -486,18 +491,17 @@ wxGLContext::wxGLContext(wxGLCanvas *win,
 
     // We need to create a temporary context to get the
     // glXCreateContextAttribsARB function
-    GLXContext tempContext = glXCreateContext(dpy, vi, NULL,
+    GLXContext tempContext = glXCreateContext(dpy, vi, NULL, x11Direct);
-                                              win->GetGLCTXAttrs().x11Direct );
     wxCHECK_RET(tempContext, "glXCreateContext failed" );
 
-    PFNGLXCREATECONTEXTATTRIBSARBPROC glXCreateContextAttribsARB
+    PFNGLXCREATECONTEXTATTRIBSARBPROC wx_glXCreateContextAttribsARB
         = (PFNGLXCREATECONTEXTATTRIBSARBPROC)
         glXGetProcAddress((GLubyte *)"glXCreateContextAttribsARB");
 
     glXDestroyContext( dpy, tempContext );
 
     // The preferred way is using glXCreateContextAttribsARB, even for old context
-    if ( !glXCreateContextAttribsARB && needsARB ) // OpenGL 3 context creation
+    if ( !wx_glXCreateContextAttribsARB && needsARB ) // OpenGL 3 context creation
     {
         wxLogMessage(_("OpenGL 3.0 or later is not supported by the OpenGL driver."));
         return;
@@ -508,29 +512,43 @@ wxGLContext::wxGLContext(wxGLCanvas *win,
     g_ctxErrorOccurred = false;
     int (*oldHandler)(Display*, XErrorEvent*) = XSetErrorHandler(&CTXErrorHandler);
 
-    if ( glXCreateContextAttribsARB )
+    if ( wx_glXCreateContextAttribsARB )
     {
         GLXFBConfig *fbc = win->GetGLXFBConfig();
         wxCHECK_RET( fbc, "Invalid GLXFBConfig for OpenGL" );
 
-        m_glContext = glXCreateContextAttribsARB( dpy, fbc[0],
+        m_glContext = wx_glXCreateContextAttribsARB( dpy, fbc[0],
                                 other ? other->m_glContext : None,
                                 x11Direct, contextAttribs );
-    }
-    else if ( wxGLCanvas::GetGLXVersion() >= 13 )
-    {
-        GLXFBConfig *fbc = win->GetGLXFBConfig();
-        wxCHECK_RET( fbc, "Invalid GLXFBConfig for OpenGL" );
 
-        m_glContext = glXCreateNewContext( dpy, fbc[0], renderType,
+        // Some old hardware may accept the use of this ARB, but may fail.
-                                           other ? other->m_glContext : None,
+        // In case of NULL attributes we'll try creating the context old-way.
-                                           x11Direct );
+        XSync( dpy, False );
+        if ( g_ctxErrorOccurred && (!contextAttribs || !needsARB) )
+        {
+            g_ctxErrorOccurred = false; //Reset
+            m_glContext = NULL;
+        }
     }
-    else // GLX <= 1.2
+
+    if ( !g_ctxErrorOccurred && !m_glContext )
     {
-        m_glContext = glXCreateContext( dpy, vi,
+        // Old-way, without context atributes. Up to GL 2.1
-                                        other ? other->m_glContext : None,
+        if ( wxGLCanvas::GetGLXVersion() >= 13 )
-                                        x11Direct );
+        {
+            GLXFBConfig *fbc = win->GetGLXFBConfig();
+            wxCHECK_RET( fbc, "Invalid GLXFBConfig for OpenGL" );
+
+            m_glContext = glXCreateNewContext( dpy, fbc[0], renderType,
+                                               other ? other->m_glContext : None,
+                                               x11Direct );
+        }
+        else // GLX <= 1.2
+        {
+            m_glContext = glXCreateContext( dpy, vi,
+                                            other ? other->m_glContext : None,
+                                            x11Direct );
+        }
     }
 
     // Sync to ensure any errors generated are processed.