Repository navigation
Conversation
…nd subsurface On Wayland, FlView always presents OpenGL frames through FlViewRendererSubsurface. Some setups freeze or render black with it (see flutter#196529, flutter#195822), and there is no way to opt out short of GDK_BACKEND=x11. Add an "opengl-gtk" value for FLUTTER_LINUX_RENDERER that keeps the OpenGL renderer but presents through GTK (FlViewRendererOpenGL), as on X11. The default is unchanged.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a use_subsurface configuration to FlEngine to control whether OpenGL frames are presented via a Wayland subsurface or through GTK (when FLUTTER_LINUX_RENDERER is set to opengl-gtk). The feedback suggests explicitly initializing renderer_type to kOpenGL to avoid relying on zero-initialization, and refactoring the RendererDefault test to isolate it from host environment variables.
|
|
||
| self->project = FL_DART_PROJECT(g_object_ref(project)); | ||
| const gchar* renderer = g_getenv("FLUTTER_LINUX_RENDERER"); | ||
| self->use_subsurface = TRUE; |
There was a problem hiding this comment.
Relying on zero-initialization for the default renderer_type enum value is fragile. If the enum values are reordered or a new default is introduced, this could break silently. Explicitly initializing self->renderer_type to kOpenGL alongside self->use_subsurface improves safety and maintainability.
self->renderer_type = kOpenGL;
self->use_subsurface = TRUE;| TEST_F(FlEngineTest, RendererDefault) { | ||
| EXPECT_EQ(fl_engine_get_renderer_type(engine), kOpenGL); | ||
| EXPECT_TRUE(fl_engine_get_use_subsurface(engine)); | ||
| } |
There was a problem hiding this comment.
The RendererDefault test uses the shared engine instance from the test fixture, which is initialized during SetUp(). If the host environment running the tests has the FLUTTER_LINUX_RENDERER environment variable set (e.g., to software or opengl-gtk), this test will fail. To make the test robust against the host environment, it should use a locally created engine with FLUTTER_LINUX_RENDERER explicitly unset (and restored afterwards).
TEST_F(FlEngineTest, RendererDefault) {
g_autofree gchar* initial_renderer =
g_strdup(g_getenv("FLUTTER_LINUX_RENDERER"));
g_unsetenv("FLUTTER_LINUX_RENDERER");
g_autoptr(FlDartProject) project = fl_dart_project_new();
g_autoptr(FlEngine) default_engine = fl_engine_new(project);
EXPECT_EQ(fl_engine_get_renderer_type(default_engine), kOpenGL);
EXPECT_TRUE(fl_engine_get_use_subsurface(default_engine));
if (initial_renderer != nullptr) {
g_setenv("FLUTTER_LINUX_RENDERER", initial_renderer, TRUE);
} else {
g_unsetenv("FLUTTER_LINUX_RENDERER");
}
}
On Wayland,
FlViewalways presents OpenGL frames throughFlViewRendererSubsurface(added in 3.50). With that renderer, some setups freeze at startup or show a black window (#196529, #195822). The only way to get the GTK path (FlViewRendererOpenGL) back isGDK_BACKEND=x11.This adds an
opengl-gtkvalue to the existingFLUTTER_LINUX_RENDERERenvironment variable. It keeps the OpenGL renderer but presents through GTK, as on X11. Nothing changes when the variable is unset,openglorsoftware.fl_engine.cc/fl_engine_private.h: newfl_engine_get_use_subsurface(),TRUEunlessFLUTTER_LINUX_RENDERER=opengl-gtk.fl_view.cc: usesFlViewRendererSubsurfaceonly when that isTRUEand the display is Wayland.fl_engine_test.cc:RendererDefaultandRendererOpenGLGtk.I have not run the engine tests locally yet. clang-format reports no changes on the modified lines.
Part of #196529: an opt-out while the subsurface issue is investigated. It does not fix the underlying bug, and doesn't fix #195822 either.
No changes to flutter/tests.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.