Add support for multiple seats while using Wayland [chromium/src : main]
Max Ihlenfeldt (Gerrit)
Max Ihlenfeldt added 1 comment![]( "Open in Gerrit")
base::flat_map<uint32_t, std::unique_ptr<WaylandSeat>> extra_seats_;What's the reason for differentiating between the primary seat and the extra seats? This is not a concept that's in the spec, is it?
It seems `seat_` is used in a few places (e.g. [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/host/wayland_connection.cc;l=431;drc=90cac1911508d3d682a67c97aa62483eb712f69a), [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/host/wayland_connection.cc;l=415;drc=90cac1911508d3d682a67c97aa62483eb712f69a], and [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/host/wayland_connection.cc;l=693;drc=90cac1911508d3d682a67c97aa62483eb712f69a)) and `WaylandConnection::seat()` also currently has 27 non-test and non-DCHECK references. I'm not too familiar with the details of multi-seat setups, so I'm not sure if just creating the additional pointer/keyboard/touch devices and leaving all these call sites to be primary-seat-only is enough/correct.
For example, is it enough to handle drags only for the primary seat (i.e. only call `wl_data_device_manager.get_data_device()` for that one seat)? Starting a drag using the data device from seat 1 in response to pointer events from seat 2 seems weird to me.
Related details
- Lachlan Frawley
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lachlan Frawley (Gerrit)
Lachlan Frawley added 1 comment![]( "Open in Gerrit")
base::flat_map<uint32_t, std::unique_ptr<WaylandSeat>> extra_seats_;What's the reason for differentiating between the primary seat and the extra seats? This is not a concept that's in the spec, is it?
It seems `seat_` is used in a few places (e.g. [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/host/wayland_connection.cc;l=431;drc=90cac1911508d3d682a67c97aa62483eb712f69a), [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/host/wayland_connection.cc;l=415;drc=90cac1911508d3d682a67c97aa62483eb712f69a], and [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/host/wayland_connection.cc;l=693;drc=90cac1911508d3d682a67c97aa62483eb712f69a)) and `WaylandConnection::seat()` also currently has 27 non-test and non-DCHECK references. I'm not too familiar with the details of multi-seat setups, so I'm not sure if just creating the additional pointer/keyboard/touch devices and leaving all these call sites to be primary-seat-only is enough/correct.
For example, is it enough to handle drags only for the primary seat (i.e. only call `wl_data_device_manager.get_data_device()` for that one seat)? Starting a drag using the data device from seat 1 in response to pointer events from seat 2 seems weird to me.
I'll have to take a look at this. I never really considered much outside of 'keyboard and mouse works over VNC' when creating the original patch.
Related details
- Max Ihlenfeldt
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lachlan Frawley (Gerrit)
Lachlan Frawley added 1 comment![]( "Open in Gerrit")
base::flat_map<uint32_t, std::unique_ptr<WaylandSeat>> extra_seats_;Lachlan FrawleyWhat's the reason for differentiating between the primary seat and the extra seats? This is not a concept that's in the spec, is it?
It seems `seat_` is used in a few places (e.g. [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/host/wayland_connection.cc;l=431;drc=90cac1911508d3d682a67c97aa62483eb712f69a), [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/host/wayland_connection.cc;l=415;drc=90cac1911508d3d682a67c97aa62483eb712f69a], and [here](https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/platform/wayland/host/wayland_connection.cc;l=693;drc=90cac1911508d3d682a67c97aa62483eb712f69a)) and `WaylandConnection::seat()` also currently has 27 non-test and non-DCHECK references. I'm not too familiar with the details of multi-seat setups, so I'm not sure if just creating the additional pointer/keyboard/touch devices and leaving all these call sites to be primary-seat-only is enough/correct.
For example, is it enough to handle drags only for the primary seat (i.e. only call `wl_data_device_manager.get_data_device()` for that one seat)? Starting a drag using the data device from seat 1 in response to pointer events from seat 2 seems weird to me.
I'll have to take a look at this. I never really considered much outside of 'keyboard and mouse works over VNC' when creating the original patch.
I've done some looking at the code you mentioned. I haven't had the opportunity to test my theory yet, but I think the solution I've got now just causes chrome to interpret the input from extra seats as if it came from the primary seat.
I feel like this fits with how VNC/RDP behave, since they simply replicate the screen of some target system instead of creating a new, separated instance.
Also on the difference of the "primary" and "extra" seats, I don't believe there is any mention of it in the Wayland spec. My view is that the primary seat always represents the local device seat, and can therefore have things like the cursor tied to it. The extra seats should always be transient external connections (like VNC or RDP), and should therefore not have anything associated with them, but can still interact with the window. (And if I'm being completely honest, when I first wrote this patch, I didn't think something so simple would actually work).
Related details
- Max Ihlenfeldt
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |