From 1d5df140ceea7ae46541eb96e65875a0fc44edf2 Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Wed, 14 Oct 2020 16:44:15 +1300 Subject: [PATCH 1/2] Explicitly make the X connection for EGL. EGL can mistakenly choose the GBM backend when using EGL_DEFAULT_DISPLAY. Fixes https://github.com/flutter/flutter/issues/60429 --- shell/platform/linux/BUILD.gn | 2 ++ shell/platform/linux/config/BUILD.gn | 6 ++--- shell/platform/linux/fl_renderer_x11.cc | 33 +++++++++++++++++++++---- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/shell/platform/linux/BUILD.gn b/shell/platform/linux/BUILD.gn index 523ea1194899c..0512376aefe7c 100644 --- a/shell/platform/linux/BUILD.gn +++ b/shell/platform/linux/BUILD.gn @@ -129,6 +129,7 @@ source_set("flutter_linux") { "//flutter/shell/platform/linux/config:gtk", "//flutter/shell/platform/linux/config:egl", "//flutter/shell/platform/linux/config:wayland-egl", + "//flutter/shell/platform/linux/config:x11", "//third_party/khronos:khronos_headers", ] @@ -172,6 +173,7 @@ executable("flutter_linux_unittests") { configs += [ "//flutter/shell/platform/linux/config:gtk", "//flutter/shell/platform/linux/config:wayland-egl", + "//flutter/shell/platform/linux/config:x11", ] # Set flag to allow public headers to be directly included (library users should not do this) diff --git a/shell/platform/linux/config/BUILD.gn b/shell/platform/linux/config/BUILD.gn index 77402c5315b78..e8fd120e25d3b 100644 --- a/shell/platform/linux/config/BUILD.gn +++ b/shell/platform/linux/config/BUILD.gn @@ -5,10 +5,8 @@ import("//build/config/linux/pkg_config.gni") import("//flutter/shell/platform/glfw/config.gni") -if (build_glfw_shell) { - pkg_config("x11") { - packages = [ "x11" ] - } +pkg_config("x11") { + packages = [ "x11" ] } pkg_config("gtk") { diff --git a/shell/platform/linux/fl_renderer_x11.cc b/shell/platform/linux/fl_renderer_x11.cc index 2c646b16ca223..6746f3bfabe1c 100644 --- a/shell/platform/linux/fl_renderer_x11.cc +++ b/shell/platform/linux/fl_renderer_x11.cc @@ -5,14 +5,30 @@ #include "fl_renderer_x11.h" #ifdef GDK_WINDOWING_X11 +#include + #include "flutter/shell/platform/linux/egl_utils.h" struct _FlRendererX11 { FlRenderer parent_instance; + + /// Connection to the X server. + Display* display; }; G_DEFINE_TYPE(FlRendererX11, fl_renderer_x11, fl_renderer_get_type()) +static void fl_renderer_x11_dispose(GObject* object) { + FlRendererX11* self = FL_RENDERER_X11(object); + + if (self->display != nullptr) { + XCloseDisplay(self->display); + self->display = nullptr; + } + + G_OBJECT_CLASS(fl_renderer_x11_parent_class)->dispose(object); +} + // Implements FlRenderer::setup_window_attr. static gboolean fl_renderer_x11_setup_window_attr( FlRenderer* renderer, @@ -50,11 +66,17 @@ static gboolean fl_renderer_x11_setup_window_attr( // Implements FlRenderer::create_display. static EGLDisplay fl_renderer_x11_create_display(FlRenderer* renderer) { - // Note the use of EGL_DEFAULT_DISPLAY rather than sharing the existing - // display connection from GTK. This is because this EGL display is going to - // be accessed by a thread from Flutter. The GTK/X11 display connection is not - // thread safe and would cause a crash. - return eglGetDisplay(EGL_DEFAULT_DISPLAY); + FlRendererX11* self = FL_RENDERER_X11(renderer); + + // We make our own connection to the X server because the EGL calls are made + // from Flutter on a different thread to GTK. If we re-used the existing GTK + // connection then this would crash as Xlib is not thread safe. + if (self->display == nullptr) { + Display* display = gdk_x11_get_default_xdisplay(); + self->display = XOpenDisplay(DisplayString(display)); + } + + return eglGetDisplay(self->display); } // Implements FlRenderer::create_surfaces. @@ -99,6 +121,7 @@ static gboolean fl_renderer_x11_create_surfaces(FlRenderer* renderer, } static void fl_renderer_x11_class_init(FlRendererX11Class* klass) { + G_OBJECT_CLASS(klass)->dispose = fl_renderer_x11_dispose; FL_RENDERER_CLASS(klass)->setup_window_attr = fl_renderer_x11_setup_window_attr; FL_RENDERER_CLASS(klass)->create_display = fl_renderer_x11_create_display; From 58df918698c53e00b4b42b9a59cfd4b18d870a6a Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Thu, 15 Oct 2020 09:12:42 +1300 Subject: [PATCH 2/2] Update comments from review --- shell/platform/linux/fl_renderer_x11.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/platform/linux/fl_renderer_x11.cc b/shell/platform/linux/fl_renderer_x11.cc index 6746f3bfabe1c..2e9d7c147996a 100644 --- a/shell/platform/linux/fl_renderer_x11.cc +++ b/shell/platform/linux/fl_renderer_x11.cc @@ -12,7 +12,7 @@ struct _FlRendererX11 { FlRenderer parent_instance; - /// Connection to the X server. + // Connection to the X server. Display* display; }; @@ -68,9 +68,9 @@ static gboolean fl_renderer_x11_setup_window_attr( static EGLDisplay fl_renderer_x11_create_display(FlRenderer* renderer) { FlRendererX11* self = FL_RENDERER_X11(renderer); - // We make our own connection to the X server because the EGL calls are made - // from Flutter on a different thread to GTK. If we re-used the existing GTK - // connection then this would crash as Xlib is not thread safe. + // Create a dedicated connection to the X server because the EGL calls are + // made from Flutter on a different thread to GTK. Re-using the existing + // GTK X11 connection would crash as Xlib is not thread safe. if (self->display == nullptr) { Display* display = gdk_x11_get_default_xdisplay(); self->display = XOpenDisplay(DisplayString(display));