Skip to content

Use Wayland subsurfaces for rendering when available.#188767

Closed
robert-ancell wants to merge 20 commits into
flutter:masterfrom
robert-ancell:split-fl-view-renderer-backends
Closed

Use Wayland subsurfaces for rendering when available.#188767
robert-ancell wants to merge 20 commits into
flutter:masterfrom
robert-ancell:split-fl-view-renderer-backends

Conversation

@robert-ancell

Copy link
Copy Markdown
Contributor

No description provided.

Split FlViewRenderer into a minimal abstract base class with two
self-contained backend subclasses, FlViewRendererOpenGL and
FlViewRendererSoftware. Each subclass owns its own state and compositor
and implements rendering for its backend. fl_view.cc selects which
subclass to create based on the engine's renderer type.
Make FlCompositorOpenGL and FlCompositorSoftware derive directly from
GObject and expose their present_layers, get_frame_size and render
functions as public concrete functions. The view renderers and
compositor tests now call these directly, removing the need for the
FlCompositor base class, which is deleted.
Make FlCompositorSoftware pure frame storage/combining by moving the
frame_mutex and task_runner into FlViewRendererSoftware. The renderer now
owns the synchronization: it locks around compositor access, signals the
task runner after presenting, and runs the resize wait loop in draw.
Make FlCompositorOpenGL pure frame storage/combining by moving the
frame_mutex and task_runner into FlViewRendererOpenGL. The renderer now
owns the synchronization: it locks around compositor access, signals the
task runner after presenting, and runs the resize wait loop in draw.
The pixels buffer is only populated when shareable is FALSE, not TRUE.
Introduce an empty FlViewRenderer subclass for rendering OpenGL into a Wayland
subsurface, selected when the OpenGL backend is used on a Wayland display. The
renderer implementation will be added in a later stage.
Render Flutter OpenGL frames into a Wayland subsurface by compositing
layers into the engine's shareable framebuffer, then blitting the
composited frame to a wl_egl_window EGL surface and swapping. Adds
wayland-client/wayland-egl configs and an accessor to retrieve the
compositor framebuffer.
Match the FlViewRendererOpenGL and FlViewRendererSoftware convention of
clearing the frame and resize mutexes in dispose() while leaving the
compositor release in finalize().
Switching the EGL context on the engine's raster thread inside
present_layers corrupted the Impeller GLES backend's cached context
state, causing a fatal GL_INVALID_FRAMEBUFFER_OPERATION on glClear.
Composite on the raster thread as before, but perform the EGL blit and
swap to the subsurface on the GTK thread in draw(), where it never
touches the engine's rendering context. Also stop terminating the EGL
display in unrealize, since it is shared with the engine.
Move the Wayland registry binding and wl_compositor/wl_subcompositor/
wl_surface/wl_subsurface management out of FlViewRendererSubsurface and
into a dedicated FlSubsurface module, mirroring flutter_subsurface.c in
the prototype. The renderer now creates an FlSubsurface and uses it to
obtain the wl_surface for its EGL window and to reposition the
subsurface.
Destroying an FlFramebuffer makes OpenGL calls (glDeleteFramebuffers
etc.) that epoxy resolves lazily and which require a current context.
The sibling was held in a g_autoptr and therefore destroyed at scope
exit, after the EGL context had already been cleared, tripping epoxy's
"Couldn't find current GLX or EGL context" assertion. Release it
explicitly while the context is still current.
The resize barrier (ported from the prototype's flutter_view_resize)
blocked the GTK main thread in size_allocate on a plain GCond until the
raster thread presented a frame of the new size. Unlike the OpenGL
renderer's fl_task_runner_wait, this wait did not pump the platform task
runner, so it could deadlock frame production and lock up resizing.

A Wayland subsurface commits its own buffer independently of the parent
surface, so no such synchronization is needed: drop the barrier and the
now-unused task runner, and just resize the wl_egl_window.
A wl_surface has an infinite input region by default, so the subsurface
placed over the view captured pointer, touch and keyboard events instead
of letting them reach the parent GTK surface, breaking all input. Set an
empty input region on the subsurface so events pass through to GTK, which
handles input for the view.
Add a method on the FlViewRenderer base class to emit the "first-frame"
signal, so subclasses no longer emit it by name directly. Update the
OpenGL, software and subsurface renderers to use it.
The subsurface renderer created its own EGL context unshared with the
engine and relied on EGLImage (via fl_framebuffer_create_sibling) to
access the engine's composited frame. This required EGLImage support.

Create the subsurface's EGL context sharing the engine's render context
so the engine frame texture can be read directly. The compositor's frame
sharing strategy is now an explicit enum (EGL image, shared context, or
CPU copy); the shared-context mode creates a framebuffer with no EGLImage
and skips the CPU readback.
The EGL config is only used while setting up the EGL context and window
surface, so it does not need to be stored on the renderer.
The EGL display is owned by the engine's FlOpenGLManager, not the
renderer. Rather than caching a copy, fetch it through a get_egl_display
helper where needed. This removes duplicated, borrowed state and makes
it clear the renderer does not own the display.
The background color storage, default, set_background_color and the
paint-background logic were duplicated in each renderer backend. Move
them into the FlViewRenderer base class: it now stores the color in
private instance data, exposes fl_view_renderer_paint_background, and
implements set_background_color directly (the per-subclass vfunc is
removed). Subclasses call fl_view_renderer_paint_background in their
draw implementation.
@robert-ancell
robert-ancell requested a review from a team as a code owner June 30, 2026 04:44
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 30, 2026
@robert-ancell
robert-ancell requested review from knopp and removed request for a team June 30, 2026 04:44
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-linux Building on or for Linux specifically a: desktop Running on desktop team-linux Owned by the Linux platform team labels Jun 30, 2026
@robert-ancell
robert-ancell requested a review from mattkae June 30, 2026 04:44
@robert-ancell
robert-ancell marked this pull request as draft June 30, 2026 04:44
@robert-ancell

Copy link
Copy Markdown
Contributor Author

Still a work in progress but would appreciate some early reviews!

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Flutter Linux platform embedding by removing the abstract FlCompositor class and introducing dedicated subclasses of FlViewRenderer (FlViewRendererOpenGL, FlViewRendererSoftware, and FlViewRendererSubsurface) along with a new FlSubsurface helper to manage Wayland subsurfaces. Feedback on these changes highlights a compilation error from calling wl_compositor_destroy, potential null pointer dereferences during Wayland and GTK initialization, an OpenGL ES version mismatch when using glBlitFramebuffer, and resource leaks on EGL setup failure paths. Additionally, suggestions were made to add defensive checks for coordinate translation, window dimensions, and EGL calls, as well as to optimize performance by reusing framebuffer objects instead of recreating them on every frame.

g_clear_pointer(&self->subsurface, wl_subsurface_destroy);
g_clear_pointer(&self->surface, wl_surface_destroy);
g_clear_pointer(&self->subcompositor, wl_subcompositor_destroy);
g_clear_pointer(&self->compositor, wl_compositor_destroy);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

wl_compositor does not have a destructor request in the Wayland protocol, so wl_compositor_destroy is not generated in wayland-client-protocol.h. Calling it will cause a compilation error. Use wl_proxy_destroy instead to release the client-side proxy.

  if (self->compositor != nullptr) {
    wl_proxy_destroy(reinterpret_cast<struct wl_proxy*>(self->compositor));
    self->compositor = nullptr;
  }

Comment on lines +90 to +97
GtkWidget* toplevel = gtk_widget_get_toplevel(widget);
GdkWindow* gdk_window = gtk_widget_get_window(toplevel);
struct wl_surface* parent_surface =
gdk_wayland_window_get_wl_surface(gdk_window);
self->surface = wl_compositor_create_surface(self->compositor);
self->subsurface = wl_subcompositor_get_subsurface(
self->subcompositor, self->surface, parent_surface);
wl_subsurface_set_sync(self->subsurface);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Defensive programming: gtk_widget_get_window can return nullptr if the toplevel window is not realized, and gdk_wayland_window_get_wl_surface can return nullptr if the window is not fully initialized. Additionally, check if wl_compositor_create_surface and wl_subcompositor_get_subsurface return nullptr to prevent potential crashes.

  GtkWidget* toplevel = gtk_widget_get_toplevel(widget);
  GdkWindow* gdk_window = gtk_widget_get_window(toplevel);
  if (gdk_window == nullptr) {
    g_warning("Toplevel window is not realized");
    g_object_unref(self);
    return nullptr;
  }
  struct wl_surface* parent_surface = 
      gdk_wayland_window_get_wl_surface(gdk_window);
  if (parent_surface == nullptr) {
    g_warning("Failed to get Wayland surface for toplevel window");
    g_object_unref(self);
    return nullptr;
  }
  self->surface = wl_compositor_create_surface(self->compositor);
  if (self->surface == nullptr) {
    g_warning("Failed to create Wayland surface");
    g_object_unref(self);
    return nullptr;
  }
  self->subsurface = wl_subcompositor_get_subsurface(
      self->subcompositor, self->surface, parent_surface);
  if (self->subsurface == nullptr) {
    g_warning("Failed to create Wayland subsurface");
    g_object_unref(self);
    return nullptr;
  }
  wl_subsurface_set_sync(self->subsurface);

Comment on lines +111 to +112
static const EGLint context_attributes[] = {EGL_CONTEXT_CLIENT_VERSION, 2,
EGL_NONE};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

glBlitFramebuffer is an OpenGL ES 3.0 feature. Requesting an OpenGL ES 2.0 context (EGL_CONTEXT_CLIENT_VERSION, 2) will cause a failure or crash on systems that strictly enforce ES 2.0. Request client version 3 instead.

  static const EGLint context_attributes[] = {EGL_CONTEXT_CLIENT_VERSION, 3,
                                              EGL_NONE};

Comment on lines +115 to +136
self->egl_context = eglCreateContext(egl_display, egl_config, share_context,
context_attributes);
if (self->egl_context == EGL_NO_CONTEXT) {
g_warning("Failed to create EGL context for subsurface");
return FALSE;
}

self->egl_window =
wl_egl_window_create(fl_subsurface_get_surface(self->subsurface),
width * scale, height * scale);
if (self->egl_window == nullptr) {
g_warning("Failed to create wl_egl_window for subsurface");
return FALSE;
}

self->egl_surface = eglCreateWindowSurface(
egl_display, egl_config,
reinterpret_cast<EGLNativeWindowType>(self->egl_window), nullptr);
if (self->egl_surface == EGL_NO_SURFACE) {
g_warning("Failed to create EGL window surface for subsurface");
return FALSE;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Resource leak on error paths: if wl_egl_window_create or eglCreateWindowSurface fails, previously created EGL resources (like self->egl_context or self->egl_window) are leaked. Clean them up before returning FALSE.

  self->egl_context = eglCreateContext(egl_display, egl_config, share_context,
                                       context_attributes);
  if (self->egl_context == EGL_NO_CONTEXT) {
    g_warning("Failed to create EGL context for subsurface");
    return FALSE;
  }

  self->egl_window = 
      wl_egl_window_create(fl_subsurface_get_surface(self->subsurface),
                           width * scale, height * scale);
  if (self->egl_window == nullptr) {
    g_warning("Failed to create wl_egl_window for subsurface");
    eglDestroyContext(egl_display, self->egl_context);
    self->egl_context = EGL_NO_CONTEXT;
    return FALSE;
  }

  self->egl_surface = eglCreateWindowSurface(
      egl_display, egl_config,
      reinterpret_cast<EGLNativeWindowType>(self->egl_window), nullptr);
  if (self->egl_surface == EGL_NO_SURFACE) {
    g_warning("Failed to create EGL window surface for subsurface");
    wl_egl_window_destroy(self->egl_window);
    self->egl_window = nullptr;
    eglDestroyContext(egl_display, self->egl_context);
    self->egl_context = EGL_NO_CONTEXT;
    return FALSE;
  }

Comment on lines +69 to +71
gint x, y;
gtk_widget_translate_coordinates(widget, toplevel, 0, 0, &x, &y);
fl_subsurface_set_position(self->subsurface, x, y);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Defensive programming: gtk_widget_translate_coordinates can fail if the widget is not realized or not in the same screen. Check its return value before using x and y.

  gint x = 0, y = 0;
  if (gtk_widget_translate_coordinates(widget, toplevel, 0, 0, &x, &y)) {
    fl_subsurface_set_position(self->subsurface, x, y);
  }

Comment on lines +122 to +124
self->egl_window =
wl_egl_window_create(fl_subsurface_get_surface(self->subsurface),
width * scale, height * scale);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Guard against 0 or negative dimensions when creating the wl_egl_window to prevent Wayland protocol errors or EGL failures.

  int egl_width = MAX(static_cast<int>(width) * scale, 1);
  int egl_height = MAX(static_cast<int>(height) * scale, 1);
  self->egl_window =
      wl_egl_window_create(fl_subsurface_get_surface(self->subsurface),
                           egl_width, egl_height);

Comment on lines +231 to +239
EGLDisplay egl_display = get_egl_display(self);
if (self->egl_surface != EGL_NO_SURFACE) {
eglDestroySurface(egl_display, self->egl_surface);
self->egl_surface = EGL_NO_SURFACE;
}
if (self->egl_context != EGL_NO_CONTEXT) {
eglDestroyContext(egl_display, self->egl_context);
self->egl_context = EGL_NO_CONTEXT;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Defensive programming: Guard eglDestroySurface and eglDestroyContext against EGL_NO_DISPLAY to prevent potential crashes or errors on buggy drivers.

  EGLDisplay egl_display = get_egl_display(self);
  if (egl_display != EGL_NO_DISPLAY) {
    if (self->egl_surface != EGL_NO_SURFACE) {
      eglDestroySurface(egl_display, self->egl_surface);
      self->egl_surface = EGL_NO_SURFACE;
    }
    if (self->egl_context != EGL_NO_CONTEXT) {
      eglDestroyContext(egl_display, self->egl_context);
      self->egl_context = EGL_NO_CONTEXT;
    }
  }

Comment on lines +278 to +280
EGLDisplay egl_display = get_egl_display(self);
eglMakeCurrent(egl_display, self->egl_surface, self->egl_surface,
self->egl_context);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Defensive programming: eglMakeCurrent can fail. Check its return value before executing OpenGL commands.

  EGLDisplay egl_display = get_egl_display(self);
  if (!eglMakeCurrent(egl_display, self->egl_surface, self->egl_surface,
                      self->egl_context)) {
    g_warning("Failed to make EGL context current for subsurface");
    return TRUE;
  }

Comment on lines +294 to +304
GLuint read_framebuffer;
glGenFramebuffers(1, &read_framebuffer);
glBindFramebuffer(GL_READ_FRAMEBUFFER, read_framebuffer);
glFramebufferTexture2D(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
GL_TEXTURE_2D,
fl_framebuffer_get_texture_id(framebuffer), 0);
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0);
glBlitFramebuffer(0, 0, width, height, 0, 0, width, height,
GL_COLOR_BUFFER_BIT, GL_NEAREST);
eglSwapBuffers(egl_display, self->egl_surface);
glDeleteFramebuffers(1, &read_framebuffer);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Creating and deleting framebuffer objects (FBOs) on every frame is inefficient and can cause performance stuttering. Consider generating a persistent FBO during initialization (e.g., in setup_egl or lazily on the first draw) and deleting it in unrealize.

@robert-ancell

Copy link
Copy Markdown
Contributor Author

Rebuilding this branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop CICD Run CI/CD engine flutter/engine related. See also e: labels. platform-linux Building on or for Linux specifically team-linux Owned by the Linux platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant