Reimplement Xinerama tiling code

[Ionic]

The current Xinerama implementation suffers from multiple problems, including it resizing based upon invisible areas if the display configuration is a bit odd.

This PR tries to fix these shortcomings by implementing a new algorithm for tiling the session window into multiple CRTCs.

The algorithm behaves like this:

1. Generate a list of split points based upon the (remote) client's display configuration. For each coordinate space (x and y), check each display edge if it is within the agent's window. If true, add a split point.
2. Tile the agent window based upon split points. At the end of this step, each session window should be split into smaller boxes if there were split points.
3. Magic! At this point, we will have too many tiles for the session window - we do not want to have tiles that do not intersect with any display (i.e., would only intersect with empty space), so go through all tiles and merge those that do not intersect with at least one display with other tiles based upon a policy. Continue doing that until there are no tiles left that without a display intersection or until only one tile is left.

The mentioned "policy" for us will be hardcoded to "prefer vertical splits" (i.e., splits along the X axis) since for most people displays are placed in a horizontal fashion. That means, that when merging tiles, we will first look for neighbors in the north or south and only if there are none to the east and west.

Potentially the policy could be changed and expanded at a later point, if needed.

This PR is still WIP. The label will be removed once it's fully implemented and I've tested it for a bit.

[uli42]

I have not yet looked at the algo or tested anything. But I have seen you are still using the intersect() function. I think we should try switching to Xorg region/box functions instead. See e.g. =nxagentHandleExposeEvent()= in =Events.c=

[Ionic]

I haven't changed the actual code there yet.

Yes, for some data I'm using BoxRec. Regions are not appropriate in this case because they are... special. I don't know of any intersection function that would work with BoxRecs (well, short of converting them to Regions and intersecting these...)

I'm currently hanging at the box merging code, which is way more complicated than I have thought and might even be an NP-hard problem.

[uli42]

What exactly is the problem? Deciding adjacency (is this he right spelling?)?

[Ionic]

I've been playing around with different display placements and came up with such a situation:

.-----------------.
| 1 | 1,2 | 2 |   |
|---|-----|---|---|
| 1 |  1  |   |   |
|---|-----|---|---|
| 3 |     |   |   |
|---|-----|---|---|
|   |     |   |   |
·-----------------·

The numbers in there denote the display on which a box is displayed (and this also includes a special case in which a box is displayed on two heads since the CRTCs overlap).

In this scenario, I do not even know how to tile the window manually so that all boxes have an intersection with a display.

To make matters worse, I thought about legal merge operations and think that merging a box with a neighbor on the south should be forbidden, since that could move a panel that is displayed at the bottom of a display into invisible space. I.e., merging the box marked with 3 with its bottom neighbor should be prevented. Merging with upper neighbors should be fine, though.

In such a case, a possible tiling could be:

.---------------------.
| 1 | 1,2 | 1,2 | 1,2 |
|---|-----|-----|-----|
| 1 |  1  |  1  |  1  |
|---|-----|-----|-----|
| 3 |  3  |  3  |  3  |
|---|-----|-----|-----|
|   |     |     |     |
·---------------------·

That would expand all displays horizontally and leave a band not intersecting with any display at the bottom. I'm not sure this is a good solution, though...

[uli42]

This is not what we want. It would mean the Desktop Environment would not have a crtc for the lower strip. Which would make in unusable. It must be merged with 3

[Ionic]

It would have a fake CRTCs for the lower band not mapping to a real client-side CRTCs.

[Ionic]

If the lower band and 3 would be merged, a panel at the bottom of 3 would move out of the visible area, which is probably worse, since users would have no way to get it back other than to drag the window around again.

[uli42]

Do not make assumptions about the screen contents. The user might also use Unity with the control panel on the left or gnome or kde with the the control panel configured at the top. The point is: if you create a cut just below 3 the desktop will be resized on every movement of the nx window.The whole stoy is that we want to avoid just that!

[Ionic]

Hm, probably a good argument. Panels at the top would be cut off, too... I've been thinking about those left and right, but naturally if merging invisible areas that would happen there, too.

So no constraints on this.

Finding an algorithm for this optimization problem will still be difficult, though...

[sunweaver]

@Ionic: Please close #673 with this PR. Is it mergable, yet?

[uli42]

I have tested a bit:

1. If you move the nxagent window to the lower edge so that only the window decoration bar is visible all outputs get disconnected. This feels wrong, but maybe there are good reasons to do it like this.

2. I have this setup on :0

$ DISPLAY=:0 xrandr | grep conn
LVDS1 connected 1920x1200+0+0 (normal left inverted right x axis y axis) 331mm x 207mm
DP1 disconnected (normal left inverted right x axis y axis)
DP2 disconnected (normal left inverted right x axis y axis)
DP3 disconnected (normal left inverted right x axis y axis)
HDMI1 disconnected (normal left inverted right x axis y axis)
HDMI2 connected primary 1920x1200+1920+0 (normal left inverted right x axis y axis) 518mm x 291mm
VGA1 disconnected (normal left inverted right x axis y axis)
VIRTUAL1 disconnected (normal left inverted right x axis y axis)

The LVDS1 is left and the HDMI2 is right, HMDI2 is the primary output. Now running nxagent on that setup results in the order of the two outputs being swapped. Moving the nxagent window completely to the left output (LVDS1, which is first in the list above) results in the second output inside nxagent being in use:

$ DISPLAY=:55 xrandr | grep conn
NX1 disconnected
NX2 connected 1014x900+0+0 0mm x 0mm

I think we should try to keep the order as reported by the real display, if possible.

(side note: So the primary screen is shown as NX1 here, but it has no "primary" marker. Not sure if we need to clone that, as there might be situations where the primary screen is not visible)

Now, running xrandr -noprimary -display :0 makes the order the nxagent reports matching the one of :0. So I suppose if primary is active that one is put on top of the list that nxagent gets from the real display and we might not be able to sort that out. What do you think?

3. nxagent -ac :55 opens a window on the left of my outputs (as described above). nxagent reports a size of 1914x900:

$ DISPLAY=:55 xrandr 
Screen 0: minimum 320 x 240, current 1914 x 900, maximum 3840 x 1200
NX1 disconnected
NX2 connected 1914x900+0+0 0mm x 0mm
   nx_1914x900   60.00* 

Now, simply moving the window a few pixels to the right leads to the window widening and I get this:

$ DISPLAY=:55 xrandr 
Screen 0: minimum 320 x 240, current 2880 x 900, maximum 3840 x 1200
NX1 connected 984x900+1896+0 0mm x 0mm
   nx_984x900    60.00* 
NX2 connected 1896x900+0+0 0mm x 0mm
   nx_1896x900   60.00* 

I am running xfwm4.

4. Left output (LVDS1): 1920x1200, right output (HDMI2) 1920x1080 (the top row is aligned). Now moving the nxagent window downwards leads to the left part keeping the full height, while the right part has its height reduced with every movement:

$ DISPLAY=:55 xrandr 
Screen 0: minimum 320 x 240, current 2880 x 900, maximum 3840 x 1200
NX1 connected 1288x261+1592+0 0mm x 0mm
   nx_1288x261   60.00* 
NX2 connected 1592x900+0+0 0mm x 0mm
   nx_1592x900   60.00* 

after moving down a bit:

$ DISPLAY=:55 xrandr
Screen 0: minimum 320 x 240, current 2880 x 900, maximum 3840 x 1200
NX1 connected 1287x245+1593+0 0mm x 0mm
   nx_1287x245   60.00* 
NX2 connected 1593x900+0+0 0mm x 0mm
   nx_1593x900   60.00* 

As you can see the height of NX1 is not matching the height of NX2. This clearly a bug, as solving situations like this was original reason for the reimplementation.

[sunweaver]

This PR branch has been stalling for a while. @Ionic: from what I got last time we spoke, you are not 100% satisfied with it, @uli42: from what I got last time we spoke is that this tiling code rewrite is pretty awesome and ready to merge.

@Ionic: @uli42: Please give feedback!

[sunweaver]

As a side note, Ubuntu LTS is 8 months from now, so we should get this into 19.10 and let people test and complain. The time has never been better to merge this.

[Ionic]

I haven't touched this code in a year, but there were multiple issues with it.

I do remember that there was one very hard to debug crash, but I don't know if I ever managed to fix it.

Besides that, some WMs really didn't like the new layouts (probably due to max size restrictions), complained about invalid sizes and completely disabled Xinerama sometimes, with the virtual displays set to an even weirder layout without the possibly to reset this but by detaching and reattaching the session.

Then, there's all the other cases uli described that behave weirdly (for instance disconnecting all outputs when it really should leave one output for the whole screen as a fallback, which likely is an edge case problem I haven't thought about or tested).

[sunweaver]

Ok, so doing nothing on this for now.

Will you pick up the work some time? Or isn't that likely any time soon?

Mike

Am Dienstag, 27. August 2019 schrieb Mihai Moldovan:

I haven't touched this code in a year, but there were multiple issues with it.

I do remember that there was one very hard to debug crash, but I don't know if I ever managed to fix it.

Besides that, some WMs really didn't like the new layouts (probably due to max size restrictions), complained about invalid sizes and completely disabled Xinerama sometimes, with the virtual displays set to an even weirder layout without the possibly to reset this but by detaching and reattaching the session.

Then, there's all the other cases uli described that behave weirdly (for instance disconnecting all outputs when it really should leave one output for the whole screen as a fallback, which likely is an edge case problem I haven't thought about or tested).

-- You are receiving this because your review was requested. Reply to this email directly or view it on GitHub: https://github.com/ArcticaProject/nx-libs/pull/682#issuecomment-52516931

-- Gesendet von meinem Fairphone2 (powered by Sailfish OS).

[Ionic]

I'd like to, but no idea right now. I'm in deep trouble with the X2Go infrastructure again and have to update dnf* packages to the most recent release. 1 week of effort so far and still counting...