From aa0e1be0e8034511ba38edf9789cbbaa52888ae0 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 11 Feb 2021 08:26:38 +0000 Subject: [PATCH] Reland "Vulkan: Fallback to XCB for Xlib surfaces" This is a reland of fb0bf70459c44b31419400598b592eaaae85f932 Reland after making libx11-xcb dynamically loaded since it isn't present on all Linux deployment targets of Chromium. Also includes a couple of additional cosmetic changes to d3d12/PlatformFunctions noticed while looking at it for inspiration. Original change's description: > Vulkan: Fallback to XCB for Xlib surfaces > > Chromium builds the Vulkan loader without support Xlib (because it > prefers XCB) which caused Xlib wgpu::SwapChain creation to fail on the > Vulkan backend. > > This CL adds a fallback to use VK_KHR_xcb_surface if VK_KHR_xlib_surface > isn't present. > > Bug: dawn:662 > Change-Id: I0e0128ee6b5c75da03998dbae231d17e48bacc81 > Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/41180 > Reviewed-by: Austin Eng > Commit-Queue: Austin Eng > Auto-Submit: Corentin Wallez Bug: dawn:662 Change-Id: I617fcd1059dddfa05c29ac20d77f891ca6962342 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/41380 Reviewed-by: Austin Eng Reviewed-by: Jiawei Shao Commit-Queue: Corentin Wallez Auto-Submit: Corentin Wallez --- src/common/vulkan_platform.h | 1 + src/common/xlib_with_undefs.h | 4 ++ src/dawn_native/BUILD.gn | 4 ++ src/dawn_native/CMakeLists.txt | 1 + src/dawn_native/Instance.cpp | 15 +++++++ src/dawn_native/Instance.h | 8 ++++ src/dawn_native/XlibXcbFunctions.cpp | 31 ++++++++++++++ src/dawn_native/XlibXcbFunctions.h | 46 +++++++++++++++++++++ src/dawn_native/d3d12/PlatformFunctions.cpp | 6 +-- src/dawn_native/d3d12/PlatformFunctions.h | 4 +- src/dawn_native/vulkan/SwapChainVk.cpp | 32 +++++++++++++- src/dawn_native/vulkan/VulkanFunctions.cpp | 4 ++ src/dawn_native/vulkan/VulkanFunctions.h | 5 +++ 13 files changed, 153 insertions(+), 8 deletions(-) create mode 100644 src/dawn_native/XlibXcbFunctions.cpp create mode 100644 src/dawn_native/XlibXcbFunctions.h diff --git a/src/common/vulkan_platform.h b/src/common/vulkan_platform.h index 236c68236c..3c78243009 100644 --- a/src/common/vulkan_platform.h +++ b/src/common/vulkan_platform.h @@ -162,6 +162,7 @@ namespace dawn_native { namespace vulkan { #if defined(DAWN_USE_X11) # define VK_USE_PLATFORM_XLIB_KHR +# define VK_USE_PLATFORM_XCB_KHR # include "common/xlib_with_undefs.h" #endif // defined(DAWN_USE_X11) diff --git a/src/common/xlib_with_undefs.h b/src/common/xlib_with_undefs.h index f82a19aa2d..260018b77e 100644 --- a/src/common/xlib_with_undefs.h +++ b/src/common/xlib_with_undefs.h @@ -26,6 +26,10 @@ // interface. #include +// Xlib-xcb.h technically includes Xlib.h but we separate the includes to make it more clear what +// the problem is if one of these two includes fail. +#include + #undef Success #undef None #undef Always diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index b98708c69d..6f750ca8c5 100644 --- a/src/dawn_native/BUILD.gn +++ b/src/dawn_native/BUILD.gn @@ -296,6 +296,10 @@ source_set("dawn_native_sources") { if (dawn_use_x11) { libs += [ "X11" ] + sources += [ + "XlibXcbFunctions.cpp", + "XlibXcbFunctions.h", + ] } if (is_win) { diff --git a/src/dawn_native/CMakeLists.txt b/src/dawn_native/CMakeLists.txt index 8767a70b6c..6263a9dcb9 100644 --- a/src/dawn_native/CMakeLists.txt +++ b/src/dawn_native/CMakeLists.txt @@ -181,6 +181,7 @@ target_link_libraries(dawn_native if (DAWN_USE_X11) find_package(X11 REQUIRED) target_link_libraries(dawn_native PRIVATE ${X11_LIBRARIES}) + target_include_directories(dawn_native PRIVATE ${X11_INCLUDE_DIR}) endif() if (WIN32) diff --git a/src/dawn_native/Instance.cpp b/src/dawn_native/Instance.cpp index 2e3e897e11..f7fe907af8 100644 --- a/src/dawn_native/Instance.cpp +++ b/src/dawn_native/Instance.cpp @@ -19,6 +19,10 @@ #include "dawn_native/ErrorData.h" #include "dawn_native/Surface.h" +#if defined(DAWN_USE_X11) +# include "dawn_native/XlibXcbFunctions.h" +#endif // defined(DAWN_USE_X11) + namespace dawn_native { // Forward definitions of each backend's "Connect" function that creates new BackendConnection. @@ -223,6 +227,17 @@ namespace dawn_native { return mPlatform; } + const XlibXcbFunctions* InstanceBase::GetOrCreateXlibXcbFunctions() { +#if defined(DAWN_USE_X11) + if (mXlibXcbFunctions == nullptr) { + mXlibXcbFunctions = std::make_unique(); + } + return mXlibXcbFunctions.get(); +#else + UNREACHABLE(); +#endif // defined(DAWN_USE_X11) + } + Surface* InstanceBase::CreateSurface(const SurfaceDescriptor* descriptor) { if (ConsumedError(ValidateSurfaceDescriptor(this, descriptor))) { return nullptr; diff --git a/src/dawn_native/Instance.h b/src/dawn_native/Instance.h index bf67a946df..21f320411b 100644 --- a/src/dawn_native/Instance.h +++ b/src/dawn_native/Instance.h @@ -30,6 +30,7 @@ namespace dawn_native { class Surface; + class XlibXcbFunctions; // This is called InstanceBase for consistency across the frontend, even if the backends don't // specialize this class. @@ -67,6 +68,9 @@ namespace dawn_native { void SetPlatform(dawn_platform::Platform* platform); dawn_platform::Platform* GetPlatform() const; + // Get backend-independent libraries that need to be loaded dynamically. + const XlibXcbFunctions* GetOrCreateXlibXcbFunctions(); + // Dawn API Surface* CreateSurface(const SurfaceDescriptor* descriptor); @@ -97,6 +101,10 @@ namespace dawn_native { ExtensionsInfo mExtensionsInfo; TogglesInfo mTogglesInfo; + +#if defined(DAWN_USE_X11) + std::unique_ptr mXlibXcbFunctions; +#endif // defined(DAWN_USE_X11) }; } // namespace dawn_native diff --git a/src/dawn_native/XlibXcbFunctions.cpp b/src/dawn_native/XlibXcbFunctions.cpp new file mode 100644 index 0000000000..605d250824 --- /dev/null +++ b/src/dawn_native/XlibXcbFunctions.cpp @@ -0,0 +1,31 @@ +// Copyright 2021 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "dawn_native/XlibXcbFunctions.h" + +namespace dawn_native { + + XlibXcbFunctions::XlibXcbFunctions() { + if (!mLib.Open("libX11-xcb.so.1") || + !mLib.GetProc(&xGetXCBConnection, "XGetXCBConnection")) { + mLib.Close(); + } + } + XlibXcbFunctions::~XlibXcbFunctions() = default; + + bool XlibXcbFunctions::IsLoaded() const { + return xGetXCBConnection != nullptr; + } + +} // namespace dawn_native diff --git a/src/dawn_native/XlibXcbFunctions.h b/src/dawn_native/XlibXcbFunctions.h new file mode 100644 index 0000000000..f295c2aebe --- /dev/null +++ b/src/dawn_native/XlibXcbFunctions.h @@ -0,0 +1,46 @@ +// Copyright 2021 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef DAWNNATIVE_XLIBXCBFUNCTIONS_H_ +#define DAWNNATIVE_XLIBXCBFUNCTIONS_H_ + +#include "common/DynamicLib.h" +#include "dawn_native/Error.h" + +#include "common/xlib_with_undefs.h" + +class DynamicLib; + +namespace dawn_native { + + // A helper class that dynamically loads the x11-xcb library that contains XGetXCBConnection + // (and nothing else). This has to be dynamic because this libraries isn't present on all Linux + // deployment platforms that Chromium targets. + class XlibXcbFunctions { + public: + XlibXcbFunctions(); + ~XlibXcbFunctions(); + + bool IsLoaded() const; + + // Functions from x11-xcb + decltype(&::XGetXCBConnection) xGetXCBConnection = nullptr; + + private: + DynamicLib mLib; + }; + +} // namespace dawn_native + +#endif // DAWNNATIVE_XLIBXCBFUNCTIONS_H_ diff --git a/src/dawn_native/d3d12/PlatformFunctions.cpp b/src/dawn_native/d3d12/PlatformFunctions.cpp index 960d172e5c..112250c990 100644 --- a/src/dawn_native/d3d12/PlatformFunctions.cpp +++ b/src/dawn_native/d3d12/PlatformFunctions.cpp @@ -100,10 +100,8 @@ namespace dawn_native { namespace d3d12 { } } // anonymous namespace - PlatformFunctions::PlatformFunctions() { - } - PlatformFunctions::~PlatformFunctions() { - } + PlatformFunctions::PlatformFunctions() = default; + PlatformFunctions::~PlatformFunctions() = default; MaybeError PlatformFunctions::LoadFunctions() { DAWN_TRY(LoadD3D12()); diff --git a/src/dawn_native/d3d12/PlatformFunctions.h b/src/dawn_native/d3d12/PlatformFunctions.h index 123519e38c..a6146aebc4 100644 --- a/src/dawn_native/d3d12/PlatformFunctions.h +++ b/src/dawn_native/d3d12/PlatformFunctions.h @@ -22,8 +22,6 @@ #include -class DynamicLib; - namespace dawn_native { namespace d3d12 { // Loads the functions required from the platform dynamically so that we don't need to rely on @@ -108,4 +106,4 @@ namespace dawn_native { namespace d3d12 { }} // namespace dawn_native::d3d12 -#endif // DAWNNATIVE_VULKAN_VULKANFUNCTIONS_H_ +#endif // DAWNNATIVE_D3D12_PLATFORMFUNCTIONS_H_ diff --git a/src/dawn_native/vulkan/SwapChainVk.cpp b/src/dawn_native/vulkan/SwapChainVk.cpp index 0398f1e758..d9f75d15da 100644 --- a/src/dawn_native/vulkan/SwapChainVk.cpp +++ b/src/dawn_native/vulkan/SwapChainVk.cpp @@ -15,6 +15,7 @@ #include "dawn_native/vulkan/SwapChainVk.h" #include "common/Compiler.h" +#include "dawn_native/Instance.h" #include "dawn_native/Surface.h" #include "dawn_native/vulkan/AdapterVk.h" #include "dawn_native/vulkan/BackendVk.h" @@ -25,6 +26,10 @@ #include +#if defined(DAWN_USE_X11) +# include "dawn_native/XlibXcbFunctions.h" +#endif // defined(DAWN_USE_X11) + namespace dawn_native { namespace vulkan { // OldSwapChain @@ -130,7 +135,7 @@ namespace dawn_native { namespace vulkan { #endif // defined(DAWN_PLATFORM_WINDOWS) #if defined(DAWN_USE_X11) - case Surface::Type::Xlib: + case Surface::Type::Xlib: { if (info.HasExt(InstanceExt::XlibSurface)) { VkXlibSurfaceCreateInfoKHR createInfo; createInfo.sType = VK_STRUCTURE_TYPE_XLIB_SURFACE_CREATE_INFO_KHR; @@ -145,7 +150,32 @@ namespace dawn_native { namespace vulkan { "CreateXlibSurface")); return vkSurface; } + + // Fall back to using XCB surfaces if the Xlib extension isn't available. + // See https://xcb.freedesktop.org/MixingCalls/ for more information about + // interoperability between Xlib and XCB + const XlibXcbFunctions* xlibXcb = + backend->GetInstance()->GetOrCreateXlibXcbFunctions(); + ASSERT(xlibXcb != nullptr); + + if (info.HasExt(InstanceExt::XcbSurface) && xlibXcb->IsLoaded()) { + VkXcbSurfaceCreateInfoKHR createInfo; + createInfo.sType = VK_STRUCTURE_TYPE_XCB_SURFACE_CREATE_INFO_KHR; + createInfo.pNext = nullptr; + createInfo.flags = 0; + // The XCB connection lives as long as the X11 display. + createInfo.connection = xlibXcb->xGetXCBConnection( + static_cast(surface->GetXDisplay())); + createInfo.window = surface->GetXWindow(); + + VkSurfaceKHR vkSurface = VK_NULL_HANDLE; + DAWN_TRY(CheckVkSuccess( + fn.CreateXcbSurfaceKHR(instance, &createInfo, nullptr, &*vkSurface), + "CreateXcbSurfaceKHR")); + return vkSurface; + } break; + } #endif // defined(DAWN_USE_X11) default: diff --git a/src/dawn_native/vulkan/VulkanFunctions.cpp b/src/dawn_native/vulkan/VulkanFunctions.cpp index 9584475a4b..261aabe7a8 100644 --- a/src/dawn_native/vulkan/VulkanFunctions.cpp +++ b/src/dawn_native/vulkan/VulkanFunctions.cpp @@ -152,6 +152,10 @@ namespace dawn_native { namespace vulkan { GET_INSTANCE_PROC(CreateXlibSurfaceKHR); GET_INSTANCE_PROC(GetPhysicalDeviceXlibPresentationSupportKHR); } + if (globalInfo.HasExt(InstanceExt::XcbSurface)) { + GET_INSTANCE_PROC(CreateXcbSurfaceKHR); + GET_INSTANCE_PROC(GetPhysicalDeviceXcbPresentationSupportKHR); + } #endif // defined(DAWN_USE_X11) return {}; } diff --git a/src/dawn_native/vulkan/VulkanFunctions.h b/src/dawn_native/vulkan/VulkanFunctions.h index bb98b6d222..2e6218ee27 100644 --- a/src/dawn_native/vulkan/VulkanFunctions.h +++ b/src/dawn_native/vulkan/VulkanFunctions.h @@ -137,6 +137,11 @@ namespace dawn_native { namespace vulkan { PFN_vkCreateXlibSurfaceKHR CreateXlibSurfaceKHR = nullptr; PFN_vkGetPhysicalDeviceXlibPresentationSupportKHR GetPhysicalDeviceXlibPresentationSupportKHR = nullptr; + + // KHR_xcb_surface + PFN_vkCreateXcbSurfaceKHR CreateXcbSurfaceKHR = nullptr; + PFN_vkGetPhysicalDeviceXcbPresentationSupportKHR + GetPhysicalDeviceXcbPresentationSupportKHR = nullptr; #endif // defined(DAWN_USE_X11) // ---------- Device procs