Final prep before Linux Wayland support#20336
Conversation
f71ca53 to
5db9a79
Compare
| G_DEFINE_QUARK(fl_renderer_error_quark, fl_renderer_error) | ||
|
|
||
| typedef struct { | ||
| FlView* view; |
There was a problem hiding this comment.
This has created an unnecessary circular dependency. fl_renderer_get_view is only used at initialization, so the view can just be passed through those methods.
There was a problem hiding this comment.
Since It's used in a few places in construction in X11 and in map/unmap functions on Wayland (which you can now see in my draft PR: https://github.com/flutter/engine/pull/20629/files), I thought it was cleaner to allow accessing it from the renderer. We could pass it around everywhere and store a copy in FlRendererWayland instead of FlRenderer, but I didn't see the advantage of that.
There was a problem hiding this comment.
I'm reviewing #20629 and it looks like a lot of the fl_renderer_get_view calls aren't necessary.
| g_set_error(error, fl_renderer_error_quark(), FL_RENDERER_ERROR_FAILED, | ||
| "Can not create EGL surface: FlRendererX11::window not set"); | ||
| GdkWindow* window = gtk_widget_get_window( | ||
| gtk_widget_get_toplevel(GTK_WIDGET(fl_renderer_get_view(renderer)))); |
There was a problem hiding this comment.
Why does this require the toplevel widget?
There was a problem hiding this comment.
It doesn't and that was actually causing a rendering bug. Fixed.
5e69091 to
5a249d5
Compare
|
@wmww I've fixed the smaller issues and updated your branch. I'm having trouble reviewing these changes as a lot of interfaces are changing, and it's not clear if this is the right way forward. I think what would work better is a WIP PR that contains all the Wayland changes, and then make smaller PRs that bring some of the changes over one step at a time. I think "Consolidate FlRenderer initialization into fl_renderer_start() " and "Replace FlRenderer::get_visual() with more generic FlRenderer::setup_window_attr()" should be good to go now, but "Expose FlRenderer's FlView" needs more thought. |
|
Full Wayland support PR up. Do you understand the |
60ccf62 to
7a25479
Compare
7a25479 to
1cb75c5
Compare
Description
FlRenderersetup process (fl_renderer_start()is now the only relevant publicly facing method)FlViewfrom anFlRenderer. This view can be used as a widget, and from it you can get a window. This prevents a patchwork of sending widgets and windows to various functions at various times.get_visual()withsetup_window_attr(), which allows renderer implementations to customize window creation in a cleaner and more flexible wayAfter this is merged, all that's left is to add the
FlRendererWaylandfile and Wayland support is good to go.Related Issues
flutter/flutter#57932
Tests
None
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read handling breaking changes.