BeginFrameSource implementation for Wayland [chromium/src : main]
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
@fang...@chromium.org: I don't know exactly what it would take to get this production-ready or if it's even the right approach. Will comment on code where I have some questions I wasn't able to answer.
I based this on the existing classes and factory method and iOS implementation which has similar constraints (needing to go across process boundaries). It does seem to work, and I thought it might be a good starting point to investigate.
Run with `./chrome --enable-features=WaylandExternalBeginFrameSource --vmodule=begin_frame_source_wayland=1`.
Related details
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
Mitchell Cohen added 4 comments![]( "Open in Gerrit")
// There won't be a frame callback with no damage so schedule one.is this something that could be changed on the WaylandFrameManager side?
// Safety fallback in case the frame callback doesn't arrive (e.g., windowThis fallback mechanism doesn't feel great to me and it's the part I struggled with the most in this PoC.
- If it runs on every presentation feedback interval, or close to it, we basically end up reinventing a delay-based pacer, and Wayland frame callbacks are useless. - - But if it's on a longer delay, Chrome freezes while waiting for frame callbacks that never come (common when switching tabs).
So the current approach of frequently needing to schedule frames in OnBeginFrameAck conflicts with the idea of frames being event-driven. There's probably a more sophisticated way to deal with missing frame callbacks and/or to make sure fewer are dropped.
std::unique_ptr<ui::BeginFrameSourceWayland> begin_frame_source_;The tree host seems like a strange place to own this driver, and other platforms don't use it this way, but this placement had several advantages: the tree host can pre-configure the compositor to use the external frame source with existing params (that for some reason aren't used by other impls), and it has the earliest access to the compositor, PlatformWindow and Wayland extension.
ui::GetWaylandToplevelExtension(*platform_window()) != nullptr;When I tried to enable the driver for all windows (not just toplevels) I ran into failed DCHECKs in the Viz process. That surprised me because each surface has its own compositor and paces its own frames. I was able to get it to work by removing the check and clearing the frame sink in Viz, but that change seemed too drastic and could not be contained to the PoC.
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kramer Ge (Gerrit)
Kramer Ge added 3 comments![]( "Open in Gerrit")
Sorry for the delay. Some initial thoughts
// There won't be a frame callback with no damage so schedule one.is this something that could be changed on the WaylandFrameManager side?
I think if you want you can apply an empty commit in `waylandframemanager` in that case to manually trigger next `frame_callback`.
// Safety fallback in case the frame callback doesn't arrive (e.g., windowThis fallback mechanism doesn't feel great to me and it's the part I struggled with the most in this PoC.
- If it runs on every presentation feedback interval, or close to it, we basically end up reinventing a delay-based pacer, and Wayland frame callbacks are useless. - - But if it's on a longer delay, Chrome freezes while waiting for frame callbacks that never come (common when switching tabs).
So the current approach of frequently needing to schedule frames in OnBeginFrameAck conflicts with the idea of frames being event-driven. There's probably a more sophisticated way to deal with missing frame callbacks and/or to make sure fewer are dropped.
Like switch to delay-based beginframe that refers to present time interval, can switch back to callback-based when callback catches up to the most recent frame.
Does chrome freeze currently when switching tabs? I imagine the same issue exists as of now if frame callback does not come in time.
Related details
- Mitchell Cohen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
// There won't be a frame callback with no damage so schedule one.Kramer Geis this something that could be changed on the WaylandFrameManager side?
I think if you want you can apply an empty commit in `waylandframemanager` in that case to manually trigger next `frame_callback`.
Thanks for this suggestion, I implemented the empty commit in patchset 4 and performance is much more consistent. It rarely falls back to the timer now (usually once when switching tabs).
Related details
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
Mitchell Cohen added 2 comments![]( "Open in Gerrit")
@fang...@chromium.org: I don't know exactly what it would take to get this production-ready or if it's even the right approach. Will comment on code where I have some questions I wasn't able to answer.
I based this on the existing classes and factory method and iOS implementation which has similar constraints (needing to go across process boundaries). It does seem to work, and I thought it might be a good starting point to investigate.
Run with `./chrome --enable-features=WaylandExternalBeginFrameSource --vmodule=begin_frame_source_wayland=1`.
@fang...@chromium.org I added no-damage commits and presentation feedback and this works a lot better now.
I'm seeing promising performance in a lightly optimized build. The external frame source clearly performs better on a site with a high refresh rate animation (https://testufo.com/) on my 144hz display. The frequent vsync misses go away completely when I enable the flag:
Current default delay-based timer:
```
❯ ./out/Release/chrome
[2575752:2575752:0308/153547.234354:ERROR:chrome/browser/ui/views/user_education/impl/browser_user_education_interface_impl.cc:163] Attempting to show IPH IPH_ExtensionsMenu before browser initialization complete; IPH will not be shown.
[2575752:2575778:0308/153552.361774:ERROR:google_apis/gcm/engine/registration_request.cc:291] Registration response error message: DEPRECATED_ENDPOINT
[2575802:2575938:0308/153556.645870:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.03 ms
[2575802:2575938:0308/153556.766338:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.195 ms
[2575802:2575938:0308/153557.886113:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.002 ms
[2575802:2575938:0308/153558.406438:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.035 ms
[2575802:2575938:0308/153558.503106:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.082 ms
[2575802:2575938:0308/153559.143282:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.111 ms
[2575802:2575938:0308/153559.783466:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.037 ms
[2575802:2575938:0308/153600.063306:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.043 ms
[2575802:2575938:0308/153600.143264:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.193 ms
[2575802:2575938:0308/153600.663748:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.017 ms
[2575802:2575938:0308/153600.744175:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.113 ms
[2575802:2575938:0308/153601.384445:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.059 ms
[2575802:2575938:0308/153601.984590:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.062 ms
[2575802:2575938:0308/153602.641420:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.082 ms
[2575802:2575938:0308/153603.007525:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.067 ms
[2575802:2575938:0308/153603.144642:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.069 ms
[2575802:2575938:0308/153604.361920:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.002 ms
[2575802:2575938:0308/153604.481820:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.051 ms
[2575802:2575938:0308/153604.881989:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.045 ms
[2575802:2575938:0308/153605.641689:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.008 ms
[2575802:2575938:0308/153605.841712:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.062 ms
[2575802:2575938:0308/153606.043037:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.131 ms
[2575802:2575938:0308/153606.179438:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.011 ms
[2575802:2575938:0308/153606.299371:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.201 ms
[2575802:2575938:0308/153606.379519:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.077 ms
[2575802:2575938:0308/153606.562626:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.009 ms
[2575802:2575938:0308/153606.642455:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.061 ms
[2575802:2575938:0308/153606.802601:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.058 ms
[2575802:2575938:0308/153606.860069:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.003 ms
[2575802:2575938:0308/153607.282455:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.047 ms
[2575802:2575938:0308/153607.363061:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.11 ms
[2575802:2575938:0308/153608.562667:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.035 ms
[2575802:2575938:0308/153608.836783:ERROR:components/viz/service/display/display.cc:286] Frame latency is negative: -0.027 ms
^C[2575752:2575789:0308/153609.257804:ERROR:content/browser/browser_main_loop.cc:275] GLib: g_main_context_pop_thread_default: assertion 'stack != NULL' failed
QThreadStorage: entry 2 destroyed before end of thread 0x39fc026fe940
QThreadStorage: entry 1 destroyed before end of thread 0x39fc026fe940
```
New frame source:
```
❯ ./out/Release/chrome --enable-features=WaylandExternalBeginFrameSource
[2576157:2576157:0308/153617.687014:ERROR:chrome/browser/ui/views/user_education/impl/browser_user_education_interface_impl.cc:163] Attempting to show IPH IPH_ExtensionsMenu before browser initialization complete; IPH will not be shown.
[2576157:2576185:0308/153622.815586:ERROR:google_apis/gcm/engine/registration_request.cc:291] Registration response error message: DEPRECATED_ENDPOINT
[2576157:2576185:0308/153647.310875:ERROR:google_apis/gcm/engine/registration_request.cc:291] Registration response error message: DEPRECATED_ENDPOINT
[2576157:2576237:0308/153647.657540:ERROR:content/browser/browser_main_loop.cc:275] GLib: g_main_context_pop_thread_default: assertion 'stack != NULL' failed
[2576157:2576239:0308/153647.657539:ERROR:content/browser/browser_main_loop.cc:275] GLib: g_main_context_pop_thread_default: assertion 'stack != NULL' failed
[2576157:2576238:0308/153647.657540:ERROR:content/browser/browser_main_loop.cc:275] GLib: g_main_context_pop_thread_default: assertion 'stack != NULL' failed
^C[2576208:2576208:0308/153659.328258:ERROR:gpu/command_buffer/service/shared_image/shared_image_manager.cc:252] SharedImageManager::ProduceSkia: Trying to Produce a Skia representation from a non-existent mailbox.
[2576157:2576195:0308/153659.468110:ERROR:content/browser/browser_main_loop.cc:275] GLib: g_main_context_pop_thread_default: assertion 'stack != NULL' failed
QThreadStorage: entry 2 destroyed before end of thread 0x3d5c004f9da0
QThreadStorage: entry 1 destroyed before end of thread 0x3d5c004f9da0
```
// Safety fallback in case the frame callback doesn't arrive (e.g., windowKramer GeThis fallback mechanism doesn't feel great to me and it's the part I struggled with the most in this PoC.
- If it runs on every presentation feedback interval, or close to it, we basically end up reinventing a delay-based pacer, and Wayland frame callbacks are useless. - - But if it's on a longer delay, Chrome freezes while waiting for frame callbacks that never come (common when switching tabs).
So the current approach of frequently needing to schedule frames in OnBeginFrameAck conflicts with the idea of frames being event-driven. There's probably a more sophisticated way to deal with missing frame callbacks and/or to make sure fewer are dropped.
Like switch to delay-based beginframe that refers to present time interval, can switch back to callback-based when callback catches up to the most recent frame.
Does chrome freeze currently when switching tabs? I imagine the same issue exists as of now if frame callback does not come in time.
Done
Related details
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kramer Ge (Gerrit)
Kramer Ge added 1 comment![]( "Open in Gerrit")
`Frame latency is negative` is a surprising message to me, it's supposed to mean draw timing is after swap timing but does not mean much to the wayland compositor latch. Unclear to me why this is printed.
Related details
- Mitchell Cohen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
It only started appearing for me in recent versions of Chromium, so I think it's a new message, but what's interesting is that it occurs much less often when I enable this driver, especially after I added presentation feedback.
Related details
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kramer Ge (Gerrit)
Kramer Ge added 3 comments![]( "Open in Gerrit")
// Copyright 2026 The Chromium AuthorsI think this should live inside `ui/ozone/platform/wayland/host/`.
begin_frame_source_ = std::make_unique<ui::BeginFrameSourceWayland>(We want to hide the impl detail of the ozone platform.
I think it would better if we ask on the `platform_window()` to produce a `BeginFrameSource` extended by `BeginFrameSourceWayland`. life time is easier to manage if `platform_window()` owns the `begin_frame_source`.
This way you may also control that only toplevel can create the BF source.
Related details
- Mitchell Cohen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kramer Ge (Gerrit)
Kramer Ge added 2 comments![]( "Open in Gerrit")
base::ObserverList<WaylandFrameTimingObserver> frame_timing_observers_;`WaylandWindow` and `WaylandFrameManager` is 1:1, also 1:1 with `ui::Compositor`, so I did not expect more than 1 here.
May be the `WaylandBeginFrameSource` and `WaylandWindow` can be 1:N, where there is 1 `WBFS` per `WaylandWindow` tree.
if (frame_callback_freeze_detected_ &&Do you know if the beginframessource is behaving ok when browser is being captured by say https://meet.google.com, and goes into minimize/restore states?
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
begin_frame_source_ = std::make_unique<ui::BeginFrameSourceWayland>(We want to hide the impl detail of the ozone platform.
I think it would better if we ask on the `platform_window()` to produce a `BeginFrameSource` extended by `BeginFrameSourceWayland`. life time is easier to manage if `platform_window()` owns the `begin_frame_source`.
This way you may also control that only toplevel can create the BF source.
I originally placed `BeginFrameSourceWayland` in the ozone host, but there's a problem: it needs access to `ui::Compositor` to issue frames over mojo. (The iOS frame source works the same way, which is why it lives in `content/browser`.)
So we would need to remove the dependency on `ui/compositor` with a new delegate/interface for this to work. I can try it.
Related details
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kramer Ge (Gerrit)
Kramer Ge added 1 comment![]( "Open in Gerrit")
begin_frame_source_ = std::make_unique<ui::BeginFrameSourceWayland>(Mitchell CohenWe want to hide the impl detail of the ozone platform.
I think it would better if we ask on the `platform_window()` to produce a `BeginFrameSource` extended by `BeginFrameSourceWayland`. life time is easier to manage if `platform_window()` owns the `begin_frame_source`.
This way you may also control that only toplevel can create the BF source.
I originally placed `BeginFrameSourceWayland` in the ozone host, but there's a problem: it needs access to `ui::Compositor` to issue frames over mojo. (The iOS frame source works the same way, which is why it lives in `content/browser`.)
So we would need to remove the dependency on `ui/compositor` with a new delegate/interface for this to work. I can try it.
Ah that is unfortunate. I guess we should probably put it in `ui/base/wayland/` then.
Related details
- Mitchell Cohen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
begin_frame_source_ = std::make_unique<ui::BeginFrameSourceWayland>(Mitchell CohenWe want to hide the impl detail of the ozone platform.
I think it would better if we ask on the `platform_window()` to produce a `BeginFrameSource` extended by `BeginFrameSourceWayland`. life time is easier to manage if `platform_window()` owns the `begin_frame_source`.
This way you may also control that only toplevel can create the BF source.
Kramer GeI originally placed `BeginFrameSourceWayland` in the ozone host, but there's a problem: it needs access to `ui::Compositor` to issue frames over mojo. (The iOS frame source works the same way, which is why it lives in `content/browser`.)
So we would need to remove the dependency on `ui/compositor` with a new delegate/interface for this to work. I can try it.
Ah that is unfortunate. I guess we should probably put it in `ui/base/wayland/` then.
It has the same problem, `ui/compositor` depends on `ui/base/wayland`. I don't see a place to move the frame source without increasing complexity.
The Linux tree host already makes decisions based on Wayland vs. X11 and decides whether the compositor should use an external mojo frame source, so I think this placement is more justified than it seems at first. (An abstraction on `PlatformWindow` would also not be used by any other platforms.)
The biggest problem is leaking logic about toplevels right? Maybe the best solution is to get the driver working with all types of windows. ;)
Related details
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
if (frame_callback_freeze_detected_ &&Do you know if the beginframessource is behaving ok when browser is being captured by say https://meet.google.com, and goes into minimize/restore states?
Good question, it crashes when screen recording so I'll fine tune this more.
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
@fang...@chromium.org, it took me a long time to figure out why this was crashing for non-toplevel windows, when screen recording, etc. The solution was really simple: the source ID counter just needed to start at 1 instead of 0. New `ExternalBeginFrameSourceMojo` instances have their `original_source_id_` set to 0 until they receive their first frame, so we have to avoid broadcasting on ID 0 to prevent conflicts.
With the most recent change everything works with screen sharing , and there don't seem to be any conflicts with the freeze logic because the no-damage frame callbacks do not have buffers and are not affected by the issue.
Related details
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
base::ObserverList<WaylandFrameTimingObserver> frame_timing_observers_;`WaylandWindow` and `WaylandFrameManager` is 1:1, also 1:1 with `ui::Compositor`, so I did not expect more than 1 here.
May be the `WaylandBeginFrameSource` and `WaylandWindow` can be 1:N, where there is 1 `WBFS` per `WaylandWindow` tree.
Done
Related details
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
Mitchell CohenDo you know if the beginframessource is behaving ok when browser is being captured by say https://meet.google.com, and goes into minimize/restore states?
Good question, it crashes when screen recording so I'll fine tune this more.
Done
Kramer Ge (Gerrit)
Kramer Ge added 1 comment![]( "Open in Gerrit")
begin_frame_source_ = std::make_unique<ui::BeginFrameSourceWayland>(Mitchell CohenWe want to hide the impl detail of the ozone platform.
I think it would better if we ask on the `platform_window()` to produce a `BeginFrameSource` extended by `BeginFrameSourceWayland`. life time is easier to manage if `platform_window()` owns the `begin_frame_source`.
This way you may also control that only toplevel can create the BF source.
Kramer GeI originally placed `BeginFrameSourceWayland` in the ozone host, but there's a problem: it needs access to `ui::Compositor` to issue frames over mojo. (The iOS frame source works the same way, which is why it lives in `content/browser`.)
So we would need to remove the dependency on `ui/compositor` with a new delegate/interface for this to work. I can try it.
Mitchell CohenAh that is unfortunate. I guess we should probably put it in `ui/base/wayland/` then.
It has the same problem, `ui/compositor` depends on `ui/base/wayland`. I don't see a place to move the frame source without increasing complexity.
The Linux tree host already makes decisions based on Wayland vs. X11 and decides whether the compositor should use an external mojo frame source, so I think this placement is more justified than it seems at first. (An abstraction on `PlatformWindow` would also not be used by any other platforms.)
The biggest problem is leaking logic about toplevels right? Maybe the best solution is to get the driver working with all types of windows. ;)
ozone platform specific code best stay inside ozone, which is why I think it's best to have the `platformwindow` create a generic `beginframesource`, so that `desktop_window_tree_host_linux` does not have to include a `wayland` class. I think we should break the direct dependency unfortunately.
Related details
- Mitchell Cohen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
begin_frame_source_ = std::make_unique<ui::BeginFrameSourceWayland>(Mitchell CohenWe want to hide the impl detail of the ozone platform.
I think it would better if we ask on the `platform_window()` to produce a `BeginFrameSource` extended by `BeginFrameSourceWayland`. life time is easier to manage if `platform_window()` owns the `begin_frame_source`.
This way you may also control that only toplevel can create the BF source.
Kramer GeI originally placed `BeginFrameSourceWayland` in the ozone host, but there's a problem: it needs access to `ui::Compositor` to issue frames over mojo. (The iOS frame source works the same way, which is why it lives in `content/browser`.)
So we would need to remove the dependency on `ui/compositor` with a new delegate/interface for this to work. I can try it.
Mitchell CohenAh that is unfortunate. I guess we should probably put it in `ui/base/wayland/` then.
Kramer GeIt has the same problem, `ui/compositor` depends on `ui/base/wayland`. I don't see a place to move the frame source without increasing complexity.
The Linux tree host already makes decisions based on Wayland vs. X11 and decides whether the compositor should use an external mojo frame source, so I think this placement is more justified than it seems at first. (An abstraction on `PlatformWindow` would also not be used by any other platforms.)
The biggest problem is leaking logic about toplevels right? Maybe the best solution is to get the driver working with all types of windows. ;)
ozone platform specific code best stay inside ozone, which is why I think it's best to have the `platformwindow` create a generic `beginframesource`, so that `desktop_window_tree_host_linux` does not have to include a `wayland` class. I think we should break the direct dependency unfortunately.
OK, I got this working nicely in patchset 13. I chose to implement a separate `PlatformWindow` extension and created an adapter in `ui/compositor` to use as a delegate. This made it possible to move the frame source over to `WaylandWindow` and remove all special code from the Linux tree host, at the cost of two small abstractions.
Related details
- Kramer Ge
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |