Implement EGL_ANGLE_x11_visual

Reland with a fixed tests on AMD.

This extension will be used by Chrome to advertise the visual it will
use for its windows. Having ANGLE use the same visual will bring
several benefits:
 - A blit will be avoided, and the content of the window might be
   rendered on the system framebuffer directly.
 - There will be less latency when resizing windows which will make it
   much less likely to see a black border when resizing.

BUG=522149

Change-Id: I66004b6ac45453330af7c4c810ddf2c26941be42
Reviewed-on: https://chromium-review.googlesource.com/314661
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Tested-by: Corentin Wallez <cwallez@chromium.org>

extensions/EGL_ANGLE_x11_visual.txt

@@ -0,0 +1,106 @@
+Name
+
+    ANGLE_x11_visual
+
+Name Strings
+
+    EGL_ANGLE_x11_visual
+
+Contributors
+
+    Corentin Wallez, Google
+    Shannon Woods, Google
+    Jamie Madill, Google
+    Geoff Lang, Google
+
+Contacts
+
+    Corentin Wallez, Google (cwallez 'at' chromium 'dot' org)
+
+Status
+
+    Draft
+
+Version
+
+    Version 1, 2015-11-13
+
+Number
+
+    EGL Extension XXX
+
+Extension Type
+
+    EGL client extension
+
+Dependencies
+
+    Requires EGL_EXT_client_extensions to query its existence without
+    a display.
+
+    Requires EGL_EXT_platform_base.
+
+    This extension is written against the wording of version 9 of the
+    EGL_EXT_platform_base specification.
+
+    Written based on the wording of the EGL 1.5 Specification
+    (August 7 2014).
+
+Overview
+
+    This extension allows passing the X11 visual ID used by the native
+    EGL surface types at display creation time. This will restrict
+    EGLSurfaces to be created from native types with this visual ID,
+    which may allow the created display to be more compatible and
+    performant.
+
+New Types
+
+    None
+
+New Procedures and Functions
+
+    None
+
+New Tokens
+
+    Accepted as an attribute name in the <attrib_list> argument of
+    eglGetPlatformDisplayEXT:
+
+        EGL_X11_VISUAL_ID_ANGLE 0x33A3
+
+Additions to the EGL Specification
+
+    Modify section 3.5.1 (Creating On-Screen Rendering Surfaces), p. 34
+
+    Append the following to the errors of CreateWindowSurface:
+
+    "If an X11 visual was specified at display creation time using
+    EGL_ANGLE_X11_VISUAL_ID that is not equal to the ID of the
+    native_window's visual, an EGL_BAD_MATCH error is generated and
+    EGL_NO_SURFACE is returned."
+
+New Behavior
+
+    To request a display created with a X11 visual ID, the value of
+    EGL_ANGLE_X11_VISUAL_ID should be set to a valid X11 visual ID. If
+    present, this ID will be used during display creation to make a
+    display that is more compatible and potentially more performant when
+    used with EGLsurfaces created from native types with this ID. If the
+    visual ID passed isn't a valid visual ID, eglGetPlatformDisplay will
+    return EGL_NO_DISPLAY and generate an EGL_NOT_INITIALIZED error.
+
+Issues
+
+    1) When the hint is present, should EGLsurface creation functions
+       only accept native types with the hint's visual ID?
+
+       RESOLVED: Yes, generate an error when the visual of the native
+       surface doesn't match. This will avoid having hidden performance
+       or compatibility losses when using this extension.
+
+Revision History
+
+    Version 1, 2015-11-13 (Corentin Wallez)
+      - Initial draft
+

include/EGL/eglext.h

@@ -499,6 +499,11 @@ EGLAPI EGLBoolean EGLAPIENTRY eglQuerySurfacePointerANGLE (EGLDisplay dpy, EGLSu
 #define EGL_FIXED_SIZE_ANGLE              0x3201
 #endif /* EGL_ANGLE_window_fixed_size */
 
+#ifndef EGL_ANGLE_x11_visual
+#define EGL_ANGLE_x11_visual
+#define EGL_X11_VISUAL_ID_ANGLE 0x33A3
+#endif /* EGL_ANGLE_x11_visual */
+
 #ifndef EGL_ARM_pixmap_multisample_discard
 #define EGL_ARM_pixmap_multisample_discard 1
 #define EGL_DISCARD_SAMPLES_ARM           0x3286

src/libANGLE/Caps.cpp

@@ -653,6 +653,7 @@ ClientExtensions::ClientExtensions()
       platformANGLEOpenGL(false),
       deviceCreation(false),
       deviceCreationD3D11(false),
+      x11Visual(false),
       clientGetAllProcAddresses(false)
 {
 }
@@ -670,6 +671,7 @@ std::vector<std::string> ClientExtensions::getStrings() const
     InsertExtensionString("EGL_ANGLE_platform_angle_opengl",       platformANGLEOpenGL,       &extensionStrings);
     InsertExtensionString("EGL_ANGLE_device_creation",             deviceCreation,            &extensionStrings);
     InsertExtensionString("EGL_ANGLE_device_creation_d3d11",       deviceCreationD3D11,       &extensionStrings);
+    InsertExtensionString("EGL_ANGLE_x11_visual",                  x11Visual,                 &extensionStrings);
     InsertExtensionString("EGL_KHR_client_get_all_proc_addresses", clientGetAllProcAddresses, &extensionStrings);
     // clang-format on
 

src/libANGLE/Caps.h

@@ -486,6 +486,9 @@ struct ClientExtensions
     // EGL_ANGLE_device_creation_d3d11
     bool deviceCreationD3D11;
 
+    // EGL_ANGLE_x11_visual
+    bool x11Visual;
+
     // EGL_KHR_client_get_all_proc_addresses
     bool clientGetAllProcAddresses;
 };

src/libANGLE/Display.cpp

@@ -723,6 +723,10 @@ static ClientExtensions GenerateClientExtensions()
     extensions.deviceCreationD3D11 = true;
 #endif
 
+#if defined(ANGLE_USE_X11)
+    extensions.x11Visual = true;
+#endif
+
     extensions.clientGetAllProcAddresses = true;
 
     return extensions;

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

@@ -57,6 +57,7 @@ class FunctionsGLGLX : public FunctionsGL
 DisplayGLX::DisplayGLX()
     : DisplayGL(),
       mFunctionsGL(nullptr),
+      mRequestedVisual(-1),
       mContextConfig(nullptr),
       mContext(nullptr),
       mDummyPbuffer(0),
@@ -80,6 +81,7 @@ egl::Error DisplayGLX::initialize(egl::Display *display)
 {
     mEGLDisplay = display;
     Display *xDisplay = display->getNativeDisplayId();
+    const auto &attribMap = display->getAttributeMap();
 
     // ANGLE_platform_angle allows the creation of a default display
     // using EGL_DEFAULT_DISPLAY (= nullptr). In this case just open
@@ -137,6 +139,35 @@ egl::Error DisplayGLX::initialize(egl::Display *display)
         mMinSwapInterval = 1;
     }
 
+    if (attribMap.contains(EGL_X11_VISUAL_ID_ANGLE))
+    {
+        mRequestedVisual = 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;
+        int attribList[] = {
+            None,
+        };
+        glx::FBConfig *allConfigs = mGLX.chooseFBConfig(attribList, &nConfigs);
+
+        for (int i = 0; i < nConfigs; ++i)
+        {
+            if (getGLXFBConfigAttrib(allConfigs[i], GLX_VISUAL_ID) == mRequestedVisual)
+            {
+                mContextConfig = allConfigs[i];
+                break;
+            }
+        }
+        XFree(allConfigs);
+
+        if (mContextConfig == nullptr)
+        {
+            return egl::Error(EGL_NOT_INITIALIZED, "Invalid visual ID requested.");
+        }
+    }
+    else
+    {
         // When glXMakeCurrent is called, the context and the surface must be
         // compatible which in glX-speak means that their config have the same
         // color buffer type, are both RGBA or ColorIndex, and their buffers have
@@ -144,7 +175,6 @@ egl::Error DisplayGLX::initialize(egl::Display *display)
         // Since our whole EGL implementation is backed by only one GL context, this
         // context must be compatible with all the GLXFBConfig corresponding to the
         // EGLconfigs that we will be exposing.
-    {
         int nConfigs;
         int attribList[] =
         {
@@ -435,6 +465,14 @@ egl::ConfigSet DisplayGLX::generateConfigs() const
         config.nativeVisualType = getGLXFBConfigAttrib(glxConfig, GLX_X_VISUAL_TYPE);
         config.nativeRenderable = EGL_TRUE;
 
+        // When a visual ID has been specified with EGL_ANGLE_x11_visual we should
+        // only return configs with this visual: it will maximize performance by avoid
+        // blits in the driver when showing the window on the screen.
+        if (mRequestedVisual != -1 && config.nativeVisualID != mRequestedVisual)
+        {
+            continue;
+        }
+
         // Buffer sizes
         config.redSize = getGLXFBConfigAttrib(glxConfig, GLX_RED_SIZE);
         config.greenSize = getGLXFBConfigAttrib(glxConfig, GLX_GREEN_SIZE);
@@ -652,6 +690,11 @@ void DisplayGLX::setSwapInterval(glx::Drawable drawable, SwapControlData *data)
     }
 }
 
+bool DisplayGLX::isValidWindowVisualId(int visualId) const
+{
+    return mRequestedVisual == -1 || mRequestedVisual == visualId;
+}
+
 const FunctionsGL *DisplayGLX::getFunctionsGL() const
 {
     return mFunctionsGL;

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

@@ -79,6 +79,8 @@ class DisplayGLX : public DisplayGL
     // acts as expected.
     void setSwapInterval(glx::Drawable drawable, SwapControlData *data);
 
+    bool isValidWindowVisualId(int visualId) const;
+
   private:
     const FunctionsGL *getFunctionsGL() const override;
 
@@ -95,6 +97,7 @@ class DisplayGLX : public DisplayGL
     //TODO(cwallez) yuck, change generateConfigs to be non-const or add a userdata member to egl::Config?
     mutable std::map<int, glx::FBConfig> configIdToGLXConfig;
 
+    EGLint mRequestedVisual;
     glx::FBConfig mContextConfig;
     glx::Context mContext;
     // A pbuffer the context is current on during ANGLE initialization

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

@@ -52,12 +52,26 @@ WindowSurfaceGLX::~WindowSurfaceGLX()
 
 egl::Error WindowSurfaceGLX::initialize()
 {
+    // Check that the window's visual ID is valid, as part of the AMGLE_x11_visual
+    // extension.
+    {
+        XWindowAttributes windowAttributes;
+        XGetWindowAttributes(mDisplay, mParent, &windowAttributes);
+        int visualId = windowAttributes.visual->visualid;
+
+        if (!mGLXDisplay->isValidWindowVisualId(visualId))
+        {
+            return egl::Error(EGL_BAD_MATCH,
+                              "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)
     {

src/tests/angle_end2end_tests.gypi

@@ -81,6 +81,10 @@
             # TODO(cwallez) for Linux, requires a portable implementation of threads
             '<(angle_path)/src/tests/egl_tests/EGLThreadTest.cpp',
         ],
+        'angle_end2end_tests_x11_sources':
+        [
+            '<(angle_path)/src/tests/egl_tests/EGLX11VisualTest.cpp',
+        ],
     },
     'dependencies':
     [
@@ -108,5 +112,12 @@
                 '<@(angle_end2end_tests_win_sources)',
             ],
         }],
+        ['use_x11==1',
+        {
+            'sources':
+            [
+                '<@(angle_end2end_tests_x11_sources)',
+            ],
+        }],
     ]
 }

src/tests/egl_tests/EGLX11VisualTest.cpp

@@ -0,0 +1,213 @@
+//
+// Copyright (c) 2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+
+// EGLX11VisualTest.cpp: tests for EGL_ANGLE_x11_visual extension
+
+#include <gtest/gtest.h>
+
+#include <EGL/egl.h>
+#include <EGL/eglext.h>
+#include <X11/Xlib.h>
+
+#include "OSWindow.h"
+#include "test_utils/ANGLETest.h"
+#include "x11/X11Window.h"
+
+using namespace angle;
+
+namespace
+{
+
+const EGLint contextAttribs[] = {EGL_CONTEXT_CLIENT_VERSION, 2, EGL_NONE};
+}
+
+class EGLX11VisualHintTest : public ::testing::TestWithParam<angle::PlatformParameters>
+{
+  public:
+    void SetUp() override
+    {
+        mEglGetPlatformDisplayEXT = reinterpret_cast<PFNEGLGETPLATFORMDISPLAYEXTPROC>(
+            eglGetProcAddress("eglGetPlatformDisplayEXT"));
+
+        mDisplay = XOpenDisplay(NULL);
+    }
+
+    std::vector<EGLint> getDisplayAttributes(int visualId) const
+    {
+        std::vector<EGLint> attribs;
+
+        attribs.push_back(EGL_PLATFORM_ANGLE_TYPE_ANGLE);
+        attribs.push_back(GetParam().getRenderer());
+        attribs.push_back(EGL_X11_VISUAL_ID_ANGLE);
+        attribs.push_back(visualId);
+        attribs.push_back(EGL_NONE);
+
+        return attribs;
+    }
+
+    unsigned int chooseDifferentVisual(unsigned int visualId)
+    {
+        int numVisuals;
+        XVisualInfo visualTemplate;
+        visualTemplate.screen = DefaultScreen(mDisplay);
+
+        XVisualInfo *visuals =
+            XGetVisualInfo(mDisplay, VisualScreenMask, &visualTemplate, &numVisuals);
+        EXPECT_TRUE(numVisuals >= 2);
+
+        for (int i = 0; i < numVisuals; ++i)
+        {
+            if (visuals[i].visualid != visualId)
+            {
+                int result = visuals[i].visualid;
+                XFree(visuals);
+                return result;
+            }
+        }
+
+        UNREACHABLE();
+        return -1;
+    }
+
+  protected:
+    PFNEGLGETPLATFORMDISPLAYEXTPROC mEglGetPlatformDisplayEXT;
+    Display *mDisplay;
+};
+
+// Test that display creation fails if the visual ID passed in invalid.
+TEST_P(EGLX11VisualHintTest, InvalidVisualID)
+{
+    static const int gInvalidVisualId = -1;
+    auto attributes                   = getDisplayAttributes(gInvalidVisualId);
+
+    EGLDisplay display =
+        mEglGetPlatformDisplayEXT(EGL_PLATFORM_ANGLE_ANGLE, EGL_DEFAULT_DISPLAY, attributes.data());
+    ASSERT_TRUE(display != EGL_NO_DISPLAY);
+
+    ASSERT_TRUE(EGL_FALSE == eglInitialize(display, nullptr, nullptr));
+    ASSERT_EGL_ERROR(EGL_NOT_INITIALIZED);
+}
+
+// Test that context creation with a visual ID succeeds, that the context exposes
+// only one config, and that a clear on a surface with this config works.
+TEST_P(EGLX11VisualHintTest, ValidVisualIDAndClear)
+{
+    // We'll test the extension with one visual ID but we don't care which one. This means we
+    // can use OSWindow to create a window and just grab its visual.
+    OSWindow *osWindow = CreateOSWindow();
+    osWindow->initialize("EGLX11VisualHintTest", 500, 500);
+    osWindow->setVisible(true);
+
+    Window xWindow = osWindow->getNativeWindow();
+
+    XWindowAttributes windowAttributes;
+    ASSERT_NE(0, XGetWindowAttributes(mDisplay, xWindow, &windowAttributes));
+    int visualId = windowAttributes.visual->visualid;
+
+    auto attributes = getDisplayAttributes(visualId);
+    EGLDisplay display =
+        mEglGetPlatformDisplayEXT(EGL_PLATFORM_ANGLE_ANGLE, EGL_DEFAULT_DISPLAY, attributes.data());
+    ASSERT_NE(EGL_NO_DISPLAY, display);
+
+    ASSERT_TRUE(EGL_TRUE == eglInitialize(display, nullptr, nullptr));
+
+    // While this is not required by the extension, test that our implementation returns only one
+    // config, with the same native visual Id that we provided.
+    int nConfigs = 0;
+    ASSERT_TRUE(EGL_TRUE == eglGetConfigs(display, nullptr, 0, &nConfigs));
+    ASSERT_EQ(1, nConfigs);
+
+    int nReturnedConfigs = 0;
+    EGLConfig config;
+    ASSERT_TRUE(EGL_TRUE == eglGetConfigs(display, &config, 1, &nReturnedConfigs));
+    ASSERT_EQ(nConfigs, nReturnedConfigs);
+
+    EGLint eglNativeId;
+    ASSERT_TRUE(EGL_TRUE == eglGetConfigAttrib(display, config, EGL_NATIVE_VISUAL_ID, &eglNativeId));
+    ASSERT_EQ(visualId, eglNativeId);
+
+    // Finally, try to do a clear on the window.
+    EGLContext context = eglCreateContext(display, config, EGL_NO_CONTEXT, contextAttribs);
+    ASSERT_NE(EGL_NO_CONTEXT, context);
+
+    EGLSurface window = eglCreateWindowSurface(display, config, xWindow, nullptr);
+    ASSERT_EGL_SUCCESS();
+
+    eglMakeCurrent(display, window, window, context);
+    ASSERT_EGL_SUCCESS();
+
+    glViewport(0, 0, 500, 500);
+    glClearColor(0.0f, 0.0f, 1.0f, 1.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+    ASSERT_GL_NO_ERROR();
+    EXPECT_PIXEL_EQ(250, 250, 0, 0, 255, 255);
+
+    // Teardown
+    eglDestroySurface(display, window);
+    ASSERT_EGL_SUCCESS();
+
+    eglDestroyContext(display, context);
+    ASSERT_EGL_SUCCESS();
+
+    SafeDelete(osWindow);
+
+    eglMakeCurrent(display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
+    eglTerminate(display);
+}
+
+// Test that EGL_BAD_MATCH is generated when trying to create an EGL window from
+// an X11 window whose visual ID doesn't match the visual ID passed at display creation.
+TEST_P(EGLX11VisualHintTest, InvalidWindowVisualID)
+{
+    // Get the default visual ID, as a good guess of a visual id for which display
+    // creation will succeed.
+    int visualId;
+    {
+        OSWindow *osWindow = CreateOSWindow();
+        osWindow->initialize("EGLX11VisualHintTest", 500, 500);
+        osWindow->setVisible(true);
+
+        Window xWindow = osWindow->getNativeWindow();
+
+        XWindowAttributes windowAttributes;
+        ASSERT_NE(0, XGetWindowAttributes(mDisplay, xWindow, &windowAttributes));
+        visualId = windowAttributes.visual->visualid;
+
+        SafeDelete(osWindow);
+    }
+
+    auto attributes = getDisplayAttributes(visualId);
+    EGLDisplay display =
+        mEglGetPlatformDisplayEXT(EGL_PLATFORM_ANGLE_ANGLE, EGL_DEFAULT_DISPLAY, attributes.data());
+    ASSERT_NE(EGL_NO_DISPLAY, display);
+
+    ASSERT_TRUE(EGL_TRUE == eglInitialize(display, nullptr, nullptr));
+
+
+    // Initialize the window with a visual id different from the display's visual id
+    int otherVisualId = chooseDifferentVisual(visualId);
+    ASSERT_NE(visualId, otherVisualId);
+
+    OSWindow *osWindow = new X11Window(otherVisualId);
+    osWindow->initialize("EGLX11VisualHintTest", 500, 500);
+    osWindow->setVisible(true);
+
+    Window xWindow = osWindow->getNativeWindow();
+
+    // Creating the EGL window should fail with EGL_BAD_MATCH
+    int nReturnedConfigs = 0;
+    EGLConfig config;
+    ASSERT_TRUE(EGL_TRUE == eglGetConfigs(display, &config, 1, &nReturnedConfigs));
+    ASSERT_EQ(1, nReturnedConfigs);
+
+    EGLSurface window = eglCreateWindowSurface(display, config, xWindow, nullptr);
+    ASSERT_EQ(EGL_NO_SURFACE, window);
+    ASSERT_EGL_ERROR(EGL_BAD_MATCH);
+
+    SafeDelete(osWindow);
+}
+
+ANGLE_INSTANTIATE_TEST(EGLX11VisualHintTest, ES2_OPENGL());

util/x11/X11Window.cpp

@@ -8,6 +8,7 @@
 
 #include "x11/X11Window.h"
 
+#include "common/debug.h"
 #include "system_utils.h"
 #include "Timer.h"
 
@@ -152,8 +153,21 @@ static void AddX11KeyStateToEvent(Event *event, unsigned int state)
 
 X11Window::X11Window()
     : WM_DELETE_WINDOW(None),
+      WM_PROTOCOLS(None),
+      TEST_EVENT(None),
       mDisplay(nullptr),
-      mWindow(0)
+      mWindow(0),
+      mRequestedVisualId(-1)
+{
+}
+
+X11Window::X11Window(int visualId)
+    : WM_DELETE_WINDOW(None),
+      WM_PROTOCOLS(None),
+      TEST_EVENT(None),
+      mDisplay(nullptr),
+      mWindow(0),
+      mRequestedVisualId(visualId)
 {
 }
 
@@ -176,7 +190,28 @@ bool X11Window::initialize(const std::string &name, size_t width, size_t height)
         int screen = DefaultScreen(mDisplay);
         Window root = RootWindow(mDisplay, screen);
 
-        Visual *visual = DefaultVisual(mDisplay, screen);
+        Visual *visual;
+        if (mRequestedVisualId == -1)
+        {
+            visual = DefaultVisual(mDisplay, screen);
+        }
+        else
+        {
+            XVisualInfo visualTemplate;
+            visualTemplate.visualid = mRequestedVisualId;
+
+            int numVisuals       = 0;
+            XVisualInfo *visuals = XGetVisualInfo(mDisplay, VisualIDMask, &visualTemplate, &numVisuals);
+            if (numVisuals <= 0)
+            {
+                return false;
+            }
+            ASSERT(numVisuals == 1);
+
+            visual = visuals[0].visual;
+            XFree(visuals);
+        }
+
         int depth = DefaultDepth(mDisplay, screen);
         Colormap colormap = XCreateColormap(mDisplay, root, visual, AllocNone);
 

util/x11/X11Window.h

@@ -20,6 +20,7 @@ class X11Window : public OSWindow
 {
   public:
     X11Window();
+    X11Window(int visualId);
     ~X11Window();
 
     bool initialize(const std::string &name, size_t width, size_t height) override;
@@ -46,6 +47,7 @@ class X11Window : public OSWindow
 
     Display *mDisplay;
     Window mWindow;
+    int mRequestedVisualId;
 };
 
 #endif // UTIL_X11_WINDOW_H