Fixes #155045

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• I followed the breaking change policy and added Data Driven Fixes where supported.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

As I understand it, this switches from the Flutter engine scheduling the render and platform threads into a single thread to us doing the same thing in the Linux embedder. So it should be the same result in terms of performance. We can't do the rendering directly into GTK, so there's no scope for performance improvement that other platforms might have.

So the only value I can see is that we can then remove than functionality from the Flutter engine at the cost of some additional complexity here. Is there any push to remove this from the engine?

As noted, I think this can be simplified if we use the GTK APIs for scheduling on the main thread.

As I understand it, this switches from the Flutter engine scheduling the render and platform threads into a single thread to us doing the same thing in the Linux embedder. So it should be the same result in terms of performance. We can't do the rendering directly into GTK, so there's no scope for performance improvement that other platforms might have.

So the only value I can see is that we can then remove than functionality from the Flutter engine at the cost of some additional complexity here. Is there any push to remove this from the engine?

As noted, I think this can be simplified if we use the GTK APIs for scheduling on the main thread.

Not quite. When the runner is not specified, the linux embedder will by default use separate raster thread (different from the platform thread). The result of this PR is a separate raster thread, just like with any other embedder.

With this PR the actual Flutter rendering is done on raster thread, but the final blit (fl_renderer_present_layers) is performed on main thread. This works because as mentioned above gdk_gl_context_make_current, once the context is realized, is just a thin wrapper around glXMakeContextCurrent and eglMakeCurrent without any problematic sideffects.

Gtk APIs for scheduling on main thread can not be used here because of special task runner mode used when waiting for frame during resizing (introduced in flutter/engine#25884 to prevent wobbling during resizing).

I wanted to document this in the PR better before requesting review, but you seem to have beaten me to it :)

Thanks for the clarifications.

I've allocated some time to look at this next week.

Thanks again for working on this.

I'm a bit concerned about the complexity this change is adding, and it makes me think there are cleaner ways of solving this problem. This flows on from the smooth resizing work which I think also needs revisiting.

I think the solution to the above is in documentation, which will either indicate better ways of solving this or make it clearer to understand. The best way would be to write a design doc with the whole interaction between Flutter, GTK and threads laid out.

I don't want to be discouraging, however I feel we've already collected some inappropriate complexity in the Linux embedder that has already been expensive to modify and delayed the multi-window work, so I don't want to make that worse.

If we were to land this now, and roll the complexity in with the resizing issue the question is: is this feature worth taking now and paying the debt down later or better to wait and rethink?

@mattkae would love if you could have a look at this for a second opinion, thanks!

This is prerequisite to be able to have UI isolate running on platform thread. The major implication of that is that you can call Gtk methods directly and synchronously, as well as calling dart code synchronously from Gtk callbacks for example, without having to go through platform channels. More information on that in #150525.

While the complexity is certainly there, it's not that much different to what the macOS and Windows embedders are doing. I'll be more than happy to document the flow, as well as assist with any issues you'll have while implementing multi view.

I think that especially for multiwindow, which will introduce decent chunk of API surface that interacts with the host operating system, being able to do this directly and synchronously from dart has major advantages.

Some nit picks and a question around some indirection, but looks good :)

[mattkae]

Since it is only used once, I find the fact that this function is generic to be a bit of an unnecessary indirection. Could we instead just provide:

Suggested change

	// Executes the given function on the main thread and blocks until it completes.
	static void fl_renderer_call_blocking(FlRenderer* self,
	void (*callback)(gpointer data),
	gpointer callback_data) {
	// Presents layers on the main thread with the provided data and waits for
	// the presentation to complete before calling [fl_renderer_present_layers_trampoline]
	static void fl_renderer_present_layers_blocking(FlRenderer* self,,
	_FlRendererPresentLayersData* callback_data) {

And then always pass fl_renderer_present_layers_trampoline as the callback in _FlRendererCallBlockingTrampolineData?

I've simplified the changes toFlRenderer quite a bit. There is only one spot where we need to synchronize threads so the code before was unnecessary generic and hard to follow. There is now only

fl_renderer_present_layers:
called on raster thread, schedules fl_renderer_present_layers_trampoline on main thread and blocks raster thread.

fl_renderer_present_layers_trampoline:
calls fl_renderer_present_layers_on_main_thread and unblocks raster thread.

@mattkae, @robert-ancell, this should be now ready for another pass.

I will leave it to @robert-ancell to take it over the line. I have one question but I am out until Tuesday after today 😄

I'm currently working on documenting the Linux embedder threading model, so we can see how this changes things through that.

An update on playing around with the theading model:

I think the "GTK way" of doing this is to use a main loop in the render callback, and thus keep processing all events avoiding any blocking in the GTK thread. In pseudocode:

// Called by Flutter when we have an updated frame
present_cb() {
  if (in_render_cb_loop) {
    render_frame()
    loop.quit()
  } else {
    store_frame_for_later()
  }
}

// Called by GTK when it renders the window.
render_cb() {
  if (have_frame_to_render) {
    render_stored_frame()
  } else {
    loop = MainLoop()
    loop.run()
  }
}

I have the above working in a hacky branch, but for some reason there is glitching when resizing the window. I haven't yet been able to determine why this is.

@robert-ancell, I'm a little bit confused. Your branch doesn't have separate raster thread so what exactly does it solve? If we don't have separate raster thread then we can't have merged UI/platform thread without performance degradation.

EDIT: I guess the separate raster thread was not really the point of the hacky branch?

Also, have you find any issue with the model used in this PR? This is basically what the other two desktop embedders do, are you experiencing any problems with it? It moves the rendering to separate thread and resize works as expected, on both X11 and wayland.

// Called by GTK when it renders the window.
render_cb() {
  if (have_frame_to_render) {
    render_stored_frame()
  } else {
    loop = MainLoop()
    loop.run()
  }
}

We don't do that currently (rendering frame from within present callback), does it cause any issues (missed frame updates with compositor, etc)?

It seems to me that GtkWidget render is simply a result of GdkFrameClock paint event. Is there something inherently special about presenting the surfaces while the paint event is in progress? I don't quite see the connection here.

Now we can definitely improve frame pacing on linux. We don't even have custom vsync waiter right now. But it seems to me like that would be driven through GdkFrameClock in a way to ensure that we present the surfaces in a correct frame phase so that the compositor picks it up. But that also seems somewhat orthogonal to this PR.

I tested the hacky branch, the reason why resizing is wobbly on wayland is that during resizing wayland events are dised while running the inner event loop. You can test this by disabling wayland event dis while running nested loop, i.e. add the following code

  if (g_main_depth() > 1) {
    return;
  }

into _gdk_wayland_display_queue_events (GdkDisplay *display).

That will fix the resizing wobbliness. Of course it's just a hack. This is the main reason we don't pump the Gtk main loop while waiting for frame during resizing.