Display monitor hotplugging v2

[yfyf]

A revamped version of #155.

I tested the following:

• ./build vm with QEMU options modified to -device vga,max_outputs=2 displays output on both monitors
• Hotplugging works with a PlayOS live USB on my laptop+external screen

What I didn't manage to test:

• QEMU graphics drivers do not offer a way to modify the number of display outputs at run time, so cannot test hotplugging with QEMU.
• I can't figure out a way to make xrandr work from a virtual console neither on PlayOS, nor on my own laptop. E.g. if I run DISPLAY=:0 XAUTHORITY=<..> xrandr --output <secondary> --off from the xsession, it works, but if I Ctrl+Alt+F1 and run the same command from the virtual console it errors out with Configure crtc 0 failed. This would be critical for testing how the kiosk responds to screen resolution changes.

Some discussion points:

• What is the reason for the full HD (i.e. --mode 1920x1080) default in the scaling preferences? If the screen doesn't support it, xrandr will fail without a fallback, which is not great. Is it to scale down 4K screens down to 1080p?
• If there are multiple screens with different available modes/resolutions, then using identical scaling preferences / modes for all screens is problematic since they might be incompatible. Perhaps there should be a fallback to --auto or at least a check that 1080p is actually a supported mode.
• No idea how to perform kiosk-level testing for this, due to the xrandr issues mentioned above.

Checklist

• Changelog updated
• Code documented
• User manual updated

[yfyf]

From xrandr man pages:

For disconnected but enabled outputs, this will disable them.

So if all are disconnected, this will simply disable them. I suggest to replace this with a warning, unless there's some reason to do this.

[yfyf]

Removed this in b1d2db7

[guyonvarch]

Working well when unplugging the HDMI cable, and re-plugging it. But starting the system without any plugged screen, and plugging it afterward, it connects to the screen, but it doesn’t use all the available space:

I built PlayOS with ./build lab-key, installed playos-live to a USB stick, then booted from it.

[knuton]

• I can't figure out a way to make xrandr work from a virtual console neither on PlayOS, nor on my own laptop. E.g. if I run DISPLAY=:0 XAUTHORITY=<..> xrandr --output <secondary> --off from the xsession, it works, but if I Ctrl+Alt+F1 and run the same command from the virtual console it errors out with Configure crtc 0 failed. This would be critical for testing how the kiosk responds to screen resolution changes.
• No idea how to perform kiosk-level testing for this, due to the xrandr issues mentioned above.

I was just writing the same report as @guyonvarch.

I SSH'd into a machine running 0b4145b, and late plugging works, the screen is used, with a tiny kiosk taking up less than a quarter of the screen, and the rest of the screen black. I then killed the kiosk process and when it restarted, it filled the entire screen.

We should set you up with an option for doing this, @yfyf. You can add an SSH pubkey and enable SSHing in from origins other than ZeroTier, of course.

• What is the reason for the full HD (i.e. --mode 1920x1080) default in the scaling preferences? If the screen doesn't support it, xrandr will fail without a fallback, which is not great. Is it to scale down 4K screens down to 1080p?

Yes, that's the purpose. It is a bit daring to default to full HD, but we have not had issues with it so far. I guess we could abstain from adjusting the resolution if full HD is not supported at all.

• If there are multiple screens with different available modes/resolutions, then using identical scaling preferences / modes for all screens is problematic since they might be incompatible. Perhaps there should be a fallback to --auto or at least a check that 1080p is actually a supported mode.

I would leave a multi-screen scenario as "undefined", or do the simplest thing that's unlikely to blow up. It's not supported. Either mirror or leave all but one blank if multiple are detected.

[yfyf]

Thank you for the bug reports @guyonvarch and @knuton! Will look for a work-around, I have various suspects in mind.

[knuton]

You did it! 👏

[yfyf]

@knuton @guyonvarch: pushed changes that make kiosk aware of display output changes.

It seems to work, and hopefully resolves a split-brain situation between xrandr and kiosk in terms of which output is considered to be primary (that existed before). This PR makes xrandr mark the first output which is successfully configured with the specified scaling preferences as the primary. Qt docs claim that the Qt application's primary screen is "the screen where QWindows are initially shown, unless otherwise specified".

However, based on my testing the docs seem to not be entirelly true, Qt is more proactive with determining the primary screen:

• xrandr --output FOO --primary will trigger the primaryScreenChanged signal in Qt
• xrandr --output FOO --off will also trigger the primaryScreenChanged signal (and also remove the output from QGuiApplication.screens())

Previously, xrandr calls did not set --primary, implying that Qt and xrandr could interpret different outputs as primary. Now it should be somewhat synchronized.

Along the way I had to resolve an annoying race-condition - Chromium (and hence all of kiosk) was crashing when both the mode/geometry and the available screens change simultaneously. I created a test to reproduce it, the crash log is attached if you're curious. Chromium would attempt to resize before it had updated the available screens and would trigger this assert failure. I fixed it in 4212df8 by changing to a non-blocking (queue'd) signal connection. I threw in a precautionary sleep as well. The test seems to pass without it, but I don't think anything guarantees the current ordering of events. On the other hand, in the worst case the kiosk just crashes and restarts, so it's not awfully horrible and the sleep could be emitted.

Note that this is not a synthetic scenario: this could in practice happen if scaling is set to native (so non-default) and we are connecting/disconnecting two screens with different default modes (e.g. 1080p and 4K).

Could one of you maybe test this on hardware, if you have screens like that?

[guyonvarch]

My previous test is now passing, well done!

The test with 2 screens needs to be run manually, but can we have an (or many) automatic test for that would only require one screen?

For example:

1. power on with one screen pluged in, store pixels,
2. unplug screen,
3. plug screen again,
4. check pixels are ≃ equal,
5. shutdown,
6. power on with screen unpluged,
7. wait for browser window,
8. plug screen, check pixels are ≃ equal.

Also, at first, I didn’t read the note on top of the test, and thought the test had been misplaced. Could it be moved to a manual directory for example?

[yfyf]

The test with 2 screens needs to be run manually, but can we have an (or many) automatic test for that would only require one screen?

Yes, but...

For example:

1. power on with one screen pluged in, store pixels,
2. unplug screen,

There is no way to "unplug" screen in QEMU drivers, so best you can do is "xrandr --off", like I am currently doing in the tests, which is not the same.

3. plug screen again,
4. check pixels are ≃ equal,

This can be done, but note that this test would pass even if the kiosk did not do anything at all. So not very useful.

5. shutdown,
6. power on with screen unpluged,

As stated earlier, this is impossible, or at least very difficult to do without introducing significant workarounds.

7. wait for browser window,
8. plug screen, check pixels are ≃ equal.

Again, this would pass even if kiosk did not do anything.

How about a slightly different single-output automated test which is maybe a bit more useful?

1. Boot, wait for kiosk window, capture screen.
2. Change mode to a smaller (but proportional) resolution, capture screen
3. Check if screen pixels from 1 ≃ scaled screen pixels from 2
4. Turn off output
5. Turn on output with original resolution, capture screen.
6. Check if initial screen pixels (1) are ≃ current screen pixels.

Also, at first, I didn’t read the note on top of the test, and thought the test had been misplaced. Could it be moved to a manual directory for example?

Sure, can do that.

[guyonvarch]

How about a slightly different single-output automated test which is maybe a bit more useful?

1. Boot, wait for kiosk window, capture screen.

2. Change mode to a smaller (but proportional) resolution, capture screen

3. Check if screen pixels from 1 ≃ scaled screen pixels from 2

4. Turn off output

5. Turn on output with original resolution, capture screen.

6. Check if initial screen pixels (1) are ≃ current screen pixels.

Looks good!

[yfyf]

@guyonvarch added an additional integration test that can be run non-interactively: 35eb638. Took me much longer than I expected, because resampling the different resolution screenshots led to pixel noise, so had to rework the background into something slightly more complex that proportionally scales under different resolutions.

Going back to my earlier comment:

Note that this is not a synthetic scenario: this could in practice happen if scaling is set to native (so non-default) and we are connecting/disconnecting two screens with different default modes (e.g. 1080p and 4K).

Do you maybe have displays like that to attempt to test the fix in 4212df8 on real hardware?

[knuton]

Do you maybe have displays like that to attempt to test the fix in 4212df8 on real hardware?

@krksgbr Could you test switching between UHD and Full HD when provided a PlayOS live system (or installer)?

[krksgbr]

Do you maybe have displays like that to attempt to test the fix in 4212df8 on real hardware?

@krksgbr Could you test switching between UHD and Full HD when provided a PlayOS live system (or installer)?

Unfortunately I don't have a Full HD screen, but I might be able to borrow one.

[guyonvarch]

Good job! A lot of comments are nitpick.

Do you maybe have displays like that to attempt to test the fix in 4212df8 on real hardware?

I unfortunately only have full hd screens.

[guyonvarch]

Nice! What do you think about the last open comment?
EDIT: seeing your response now.