Skip to content

Draft: Introduce multi-monitor support

Hi all, As discussed, I've started work on implementing multi-monitor support on g-r-d's end over the weekends. I've had huge help from @nmraz, who I'm submitting this MR with.

A couple of things first:

  • This is a draft - the commit messages are rough, there are still debug prints
  • There are probably still some bugs in there and I've encountered a crash or two The purpose of this submission is mainly to know if this is in the right direction. If so, we will tidy up the MR and submit it in proper form to be merged :)

A few more technicalities:

  • I've yet to establish a test client - that would probably prove useful here, but I wanted to direct what little attention I can spare for this over the weekend towards implementing the feature. I recognize this is a necessary part of the process, thus if this MR comes closer to merging we should divert some attention to that as well. I'd be happy to help with that effort
  • This MR assumes pipewire props for the monitor's position and scale haven't been implemented. As discussed, this is probably a good first step towards a final implementation. We can probably very easily adapt this new design to use the pw props once those are implemented.
  • This branch is implemented on top of gnome-42 as I have intentions to backport this change for my own use. Of course, the final MR will be submitted over master and rebased properly. If you notice something I've done here that might be incompatible with master, I would very much appreciate it if you could point that out.

Previous MO

The GrdSessionRdp object holds the rdp_surface, pipewire_stream and stream directly. This implies a single logical GrdRdpSurface and a single GrdRdpPipewireStream per-session. Monitor layout updates are submitted on the socket thread, and applied in the main thread in update_monitor_layout, which resizes the stream. The graphics thread then reacts by changing the buffer size (once a resized frame comes in), and resetting the graphics pipeline.

New MO

We wanted to keep the reactive nature of the graphics thread. We want to make sure the data structures the graphics thread uses are still the source of truth for graphics-related operations. Previous attempts which tried to break this assertion resulted in a synchronization hell.

The fundemental object introduced by this MR is the GrdRdpStreamManager (Name subject to bikeshedding). We've also introduced GrdRdpStream, which is the main object held by GrdRdpStreamManager, and owns the GrdRdpSurface, GrdRdpPipeWireStream and GrdStream.

The GrdRdpStreamManager tracks all the GrdRdpStream objects, each of which corresponds to a monitor. It also tracks the number of monitors in several stages, due to the asynchronous nature of monitor configuration applications:

  • target_stream_count - The targeted monitor count (as set by monitor_config in GrdSessionRdp)
  • present_stream_count - Corresponds directly to the number of pipewire streams present.
  • applied_stream_count - The number of monitors whose configuration has been successfully applied and are now active. These values always satisfy the inequality:
applied <= present <= target

When the value of applied reaches that of target, we consider the config update as complete and notify the session (thus triggering a graphics pipeline reset).

While an update is in progress (applied < target), new monitor config updates are blocked and queued until the previous operation is complete.

We've introduced two locks - the stream_count_mutex and the present_stream_count_rwlock.

stream_count_mutex - Protects the applied and target count, as well as the config_generation. (See below) present_stream_count_rwlock is essentialy a poor-man's RCU mechanism to make sure present monitors aren't deleted when in use. We've kept this lock separate as it is accessed much more often from the socket and graphics thread, and we wanted as little additional friction as possible there. Iterating on the present streams can happen very often (See rdp_input_mouse_event).

There is also a new lock in GrdRdpSurface called the dimension_lock, which protects the size and position of the surface, now that it's subject to change from monitor configuration applications.

config_generation - Since we have no way of knowing the currently active configuration (due to the reactive nature of configuration applications), we need some way to track whether the monitor configuration set in GrdSessionRdp is currently applied, so that we can apply queued configuration updates after finishing the current one. This variable keeps track of the currently applied monitor_config, and an additional monitor_config_generation in GrdSessionRdp keeps track of the last submitted generation.

Known Issues

  • RDP monitor configurations can include negative positions, as they refer to the primary monitor as 0, 0. We need to sanitize the configuration and shift it by the leftmost x and the topmost y. We already have a fix which we will include in the final MR.
  • Lots of g_object_unref: assertion 'G_IS_OBJECT (object)' failed on disconnect - Yet to debug the cause.
  • It seems like connecting with a single monitor and then connecting with multiple sometimes crashes the shell.

As noted above, this MR is far from final, thus all of these will have to be resolved before a merge should even be considered. This is here merely as a request for feedback before we move to spend the time and effort necessary to tidy up this MR.

So, what do you think?

Merge request reports