GLX: Avoid creating child window when X visual ID is specified.

ANGLE's GLX backend creates a child ID because the visual of the X
window must match that of the GLX window, and we can't always be
certain.
EGL_ANGLE_x11_visual allows applications to specify the visual ID
of the window passed to ANGLE via EGL_X11_VISUAL_ID_ANGLE. When this
is the case, we don't need to make a child window.
Since Chrome always passes this information, this may help optimize
ANGLE's GLX usage in Chrome, because we don't have to poll the parent
window to manage the child window's properties.

Bug: chromium:1132827
Change-Id: If8082d2d07469905afffab01dde2ec9fca8d4eb9
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2611556
Commit-Queue: Jonah Ryan-Davis <jonahr@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>

src/libANGLE/renderer/gl/glx/DisplayGLX.cpp

@@ -166,7 +166,6 @@ egl::Error DisplayGLX::initialize(egl::Display *display)
     if (attribMap.contains(EGL_X11_VISUAL_ID_ANGLE))
     {
         mRequestedVisual = static_cast<EGLint>(attribMap.get(EGL_X11_VISUAL_ID_ANGLE, -1));
-
         // There is no direct way to get the GLXFBConfig matching an X11 visual ID
         // so we have to iterate over all the GLXFBConfigs to find the right one.
         int nConfigs;
@@ -882,9 +881,14 @@ void DisplayGLX::setSwapInterval(glx::Drawable drawable, SwapControlData *data)
     }
 }
 
-bool DisplayGLX::isValidWindowVisualId(unsigned long visualId) const
+bool DisplayGLX::isWindowVisualIdSpecified() const
 {
-    return mRequestedVisual == -1 || static_cast<unsigned long>(mRequestedVisual) == visualId;
+    return mRequestedVisual != -1;
+}
+
+bool DisplayGLX::isMatchingWindowVisualId(unsigned long visualId) const
+{
+    return isWindowVisualIdSpecified() && static_cast<unsigned long>(mRequestedVisual) == visualId;
 }
 
 void DisplayGLX::generateExtensions(egl::DisplayExtensions *outExtensions) const

src/libANGLE/renderer/gl/glx/DisplayGLX.h

@@ -89,7 +89,8 @@ class DisplayGLX : public DisplayGL
     // acts as expected.
     void setSwapInterval(glx::Drawable drawable, SwapControlData *data);
 
-    bool isValidWindowVisualId(unsigned long visualId) const;
+    bool isWindowVisualIdSpecified() const;
+    bool isMatchingWindowVisualId(unsigned long visualId) const;
 
     WorkerContext *createWorkerContext(std::string *infoLog);
 

src/libANGLE/renderer/gl/glx/WindowSurfaceGLX.cpp

@@ -31,6 +31,9 @@ WindowSurfaceGLX::WindowSurfaceGLX(const egl::SurfaceState &state,
       mParent(window),
       mWindow(0),
       mDisplay(display),
+      mUseChildWindow(false),
+      mParentWidth(0),
+      mParentHeight(0),
       mGLX(glx),
       mGLXDisplay(glxDisplay),
       mFBConfig(fbConfig),
@@ -44,7 +47,7 @@ WindowSurfaceGLX::~WindowSurfaceGLX()
         mGLX.destroyWindow(mGLXWindow);
     }
 
-    if (mWindow)
+    if (mUseChildWindow && mWindow)
     {
         // When destroying the window, it may happen that the window has already been
         // destroyed by the application (this happens in Chromium). There is no way to
@@ -61,66 +64,82 @@ WindowSurfaceGLX::~WindowSurfaceGLX()
 
 egl::Error WindowSurfaceGLX::initialize(const egl::Display *display)
 {
-    // Check that the window's visual ID is valid, as part of the AMGLE_x11_visual
-    // extension.
+    mUseChildWindow = !mGLXDisplay->isWindowVisualIdSpecified();
+
+    XVisualInfo *visualInfo = nullptr;
+    Colormap colormap       = 0;
+    if (mUseChildWindow)
+    {
+        // The visual of the X window, GLX window and GLX context must match,
+        // however we received a user-created window that can have any visual
+        // and wouldn't work with our GLX context. To work in all cases, we
+        // create a child window with the right visual that covers all of its
+        // parent.
+        visualInfo = mGLX.getVisualFromFBConfig(mFBConfig);
+        if (!visualInfo)
+        {
+            return egl::EglBadNativeWindow()
+                   << "Failed to get the XVisualInfo for the child window.";
+        }
+        Visual *visual = visualInfo->visual;
+
+        if (!getWindowDimensions(mParent, &mParentWidth, &mParentHeight))
+        {
+            return egl::EglBadNativeWindow() << "Failed to get the parent window's dimensions.";
+        }
+
+        // The depth, colormap and visual must match otherwise we get a X error
+        // so we specify the colormap attribute. Also we do not want the window
+        // to be taken into account for input so we specify the event and
+        // do-not-propagate masks to 0 (the defaults). Finally we specify the
+        // border pixel attribute so that we can use a different visual depth
+        // than our parent (seems like X uses that as a condition to render
+        // the subwindow in a different buffer)
+        XSetWindowAttributes attributes;
+        unsigned long attributeMask = CWColormap | CWBorderPixel;
+
+        colormap = XCreateColormap(mDisplay, mParent, visual, AllocNone);
+        if (!colormap)
+        {
+            XFree(visualInfo);
+            return egl::EglBadNativeWindow()
+                   << "Failed to create the Colormap for the child window.";
+        }
+        attributes.colormap     = colormap;
+        attributes.border_pixel = 0;
+
+        // TODO(cwallez) set up our own error handler to see if the call failed
+        mWindow = XCreateWindow(mDisplay, mParent, 0, 0, mParentWidth, mParentHeight, 0,
+                                visualInfo->depth, InputOutput, visual, attributeMask, &attributes);
+    }
+    else
     {
         XWindowAttributes windowAttributes;
         XGetWindowAttributes(mDisplay, mParent, &windowAttributes);
         unsigned long visualId = windowAttributes.visual->visualid;
-
-        if (!mGLXDisplay->isValidWindowVisualId(visualId))
+        // Check that the window's visual ID is valid, as part of the AMGLE_x11_visual
+        // extension.
+        if (!mGLXDisplay->isMatchingWindowVisualId(visualId))
         {
             return egl::EglBadMatch() << "The visual of native_window doesn't match the visual "
                                          "given with ANGLE_X11_VISUAL_ID";
         }
     }
 
-    // The visual of the X window, GLX window and GLX context must match,
-    // however we received a user-created window that can have any visual
-    // and wouldn't work with our GLX context. To work in all cases, we
-    // create a child window with the right visual that covers all of its
-    // parent.
-    XVisualInfo *visualInfo = mGLX.getVisualFromFBConfig(mFBConfig);
-    if (!visualInfo)
-    {
-        return egl::EglBadNativeWindow() << "Failed to get the XVisualInfo for the child window.";
-    }
-    Visual *visual = visualInfo->visual;
+    mGLXWindow = mGLX.createWindow(mFBConfig, (mUseChildWindow ? mWindow : mParent), nullptr);
 
-    if (!getWindowDimensions(mParent, &mParentWidth, &mParentHeight))
+    if (mUseChildWindow)
     {
-        return egl::EglBadNativeWindow() << "Failed to get the parent window's dimensions.";
+        XMapWindow(mDisplay, mWindow);
     }
 
-    // The depth, colormap and visual must match otherwise we get a X error
-    // so we specify the colormap attribute. Also we do not want the window
-    // to be taken into account for input so we specify the event and
-    // do-not-propagate masks to 0 (the defaults). Finally we specify the
-    // border pixel attribute so that we can use a different visual depth
-    // than our parent (seems like X uses that as a condition to render
-    // the subwindow in a different buffer)
-    XSetWindowAttributes attributes;
-    unsigned long attributeMask = CWColormap | CWBorderPixel;
-
-    Colormap colormap = XCreateColormap(mDisplay, mParent, visual, AllocNone);
-    if (!colormap)
-    {
-        XFree(visualInfo);
-        return egl::EglBadNativeWindow() << "Failed to create the Colormap for the child window.";
-    }
-    attributes.colormap     = colormap;
-    attributes.border_pixel = 0;
-
-    // TODO(cwallez) set up our own error handler to see if the call failed
-    mWindow    = XCreateWindow(mDisplay, mParent, 0, 0, mParentWidth, mParentHeight, 0,
-                            visualInfo->depth, InputOutput, visual, attributeMask, &attributes);
-    mGLXWindow = mGLX.createWindow(mFBConfig, mWindow, nullptr);
-
-    XMapWindow(mDisplay, mWindow);
     XFlush(mDisplay);
 
-    XFree(visualInfo);
-    XFreeColormap(mDisplay, colormap);
+    if (mUseChildWindow)
+    {
+        XFree(visualInfo);
+        XFreeColormap(mDisplay, colormap);
+    }
 
     mGLXDisplay->syncXCommands();
 
@@ -139,10 +158,13 @@ egl::Error WindowSurfaceGLX::swap(const gl::Context *context)
     mGLXDisplay->setSwapInterval(mGLXWindow, &mSwapControl);
     mGLX.swapBuffers(mGLXWindow);
 
-    egl::Error error = checkForResize();
-    if (error.isError())
+    if (mUseChildWindow)
     {
-        return error;
+        egl::Error error = checkForResize();
+        if (error.isError())
+        {
+            return error;
+        }
     }
 
     return egl::NoError();
@@ -185,14 +207,40 @@ void WindowSurfaceGLX::setSwapInterval(EGLint interval)
 
 EGLint WindowSurfaceGLX::getWidth() const
 {
-    // The size of the window is always the same as the cached size of its parent.
-    return mParentWidth;
+    if (mUseChildWindow)
+    {
+        // If there's a child window, the size of the window is always the same as the cached
+        // size of its parent.
+        return mParentWidth;
+    }
+    else
+    {
+        unsigned int parentWidth, parentHeight;
+        if (!getWindowDimensions(mParent, &parentWidth, &parentHeight))
+        {
+            return mParentWidth;
+        }
+        return parentWidth;
+    }
 }
 
 EGLint WindowSurfaceGLX::getHeight() const
 {
-    // The size of the window is always the same as the cached size of its parent.
-    return mParentHeight;
+    if (mUseChildWindow)
+    {
+        // If there's a child window, the size of the window is always the same as the cached
+        // size of its parent.
+        return mParentHeight;
+    }
+    else
+    {
+        unsigned int parentWidth, parentHeight;
+        if (!getWindowDimensions(mParent, &parentWidth, &parentHeight))
+        {
+            return mParentHeight;
+        }
+        return parentHeight;
+    }
 }
 
 EGLint WindowSurfaceGLX::isPostSubBufferSupported() const

src/libANGLE/renderer/gl/glx/WindowSurfaceGLX.h

@@ -63,10 +63,13 @@ class WindowSurfaceGLX : public SurfaceGLX
     bool getWindowDimensions(Window window, unsigned int *width, unsigned int *height) const;
 
     Window mParent;
-    unsigned int mParentWidth, mParentHeight;
     Window mWindow;
     Display *mDisplay;
 
+    bool mUseChildWindow;
+    // Only updated when mUseChildWindow is true
+    unsigned int mParentWidth, mParentHeight;
+
     const FunctionsGLX &mGLX;
     DisplayGLX *mGLXDisplay;