Bug 7007 - new monitors aren't handled in full screen
Summary: new monitors aren't handled in full screen
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: VNC (show other bugs)
Version: 1.3.1
Hardware: PC Unknown
: P2 Normal
Target Milestone: 4.14.0
Assignee: Samuel Mannehed
URL:
Keywords: frifl_tester, relnotes, wilsj_tester
: 7156 (view as bug list)
Depends on: 7160 7785
Blocks:
  Show dependency treegraph
 
Reported: 2017-07-04 14:36 CEST by Pierre Ossman
Modified: 2022-01-25 13:17 CET (History)
5 users (show)

See Also:
Acceptance Criteria:
• ☐ When changing the system's monitor configuration [1] with a ThinLinc client running, the ThinLinc display configuration should still be honored. • ☐ Changing the system's monitor configuration should not affect ThinLinc's saved display configuration. • ☐ If the saved display configuration cannot be fully realized on the current system monitor configuration, an approximate display configuration that closely resembles that of the saved configuration should be used [2]. • ☐ The screen configuration UI in both tlclient and vncviewer should reflect the system's monitor configuration [3]. [1] This can include (but may not limited to): connecting a display, disconnecting a display, mirroring two displays, joining two displays. [2] For the "Full screen on selected monitors" case, some complexity trade-offs were made in bug 7006. With "Full screen on selected monitors", changing the system's monitor configuration with ThinLinc running is expected to behave in the same manner as when starting the ThinLinc client after the system's monitor configuration has changed, as specified by bug 7006. [3] Unsaved changes made to the screen selection (if running with "Full screen on selected monitors") are expected to be discarded if the system's monitor configuration changes.


Attachments
Broken macOS client after disconnecting a monitor (3.00 MB, image/png)
2022-01-04 16:39 CET, William Sjöblom
Details
Broken Windows 10 client after disconnecting a monitor (1.35 MB, image/png)
2022-01-04 16:40 CET, William Sjöblom
Details
Broken Linux (X11) client after disconnecting a monitor (2.61 MB, image/png)
2022-01-04 16:41 CET, William Sjöblom
Details

Description Pierre Ossman cendio 2017-07-04 14:36:44 CEST
If a user plugs in a new monitor, then that new monitor isn't immediately noticed by the client. This is relevant when the client is configured to cover all monitors in full screen mode. Ideally the client should resize automatically to continue covering all screens.

(This bug happens to be a workaround for bug 7006, so we might want to hold off fixing it before that bug).
Comment 1 Peter Åstrand cendio 2018-04-20 08:45:17 CEST
*** Bug 7156 has been marked as a duplicate of this bug. ***
Comment 2 Peter Åstrand cendio 2018-05-07 10:48:51 CEST
https://github.com/TigerVNC/tigervnc/pull/642
Comment 3 Hugo Lundin cendio 2021-06-18 15:08:17 CEST
The `FL_SCREEN_CONFIGURATION_CHANGED` (`RRScreenChangeNotify` in X11/xrandr) event is never raised for me on Fedora 34 (using Wayland). 

It does not seem to be an issue with FLTK though, because the output of `$ xev -event randr` doesn’t show `RRScreenChangeNotify` either.
Comment 4 Hugo Lundin cendio 2021-07-07 19:03:21 CEST
The issue mentioned above was reported upstream (https://bugzilla.redhat.com/show_bug.cgi?id=1979892) and resulted in this merge request: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/711.
Comment 5 Pierre Ossman cendio 2021-11-17 13:16:06 CET
Also note that if the options dialog is open then the monitor selection widget (for bug 7006) should also update right away.
Comment 6 William Sjöblom cendio 2022-01-04 13:53:26 CET
Note that due to bug 7160 we do not expect hotplug on macOS to work flawlessly. Nonetheless, we still expect it to work given that keyboard grab is disabled.
Comment 7 William Sjöblom cendio 2022-01-04 13:57:02 CET
Also, we want to test this on all client platforms: Windows, macOS, Linux (both Wayland and X11).
Comment 8 William Sjöblom cendio 2022-01-04 16:38:28 CET
Okay, so it seems like we have some differences when comparing the
hotplug behavior on the Linux, Windows 10, and macOS clients.

Steps to reproduce:
1. Start a session with full screen enabled on two of the three
   connected monitors (in my case the two to the right)
2. Press F8 and then "Options" and enter the "Display" tab
3. Disconnect the third screen (the one which is not occupied by the
   ThinLinc client)


If you perform the above steps you will see drastically different
behavior on the different platforms (also see attached screenshots):


Linux, X11
══════════

  The client will be in full screen on the right hand monitor, but the
  Options dialog will show it on the left hand monitor.


Windows 10 and macOS
════════════════════

  The client will be in full screen on the right hand monitor, but the
  Options dialog will show it on being on both of the remaining
  monitors.
Comment 9 William Sjöblom cendio 2022-01-04 16:39:25 CET
Created attachment 1015 [details]
Broken macOS client after disconnecting a monitor
Comment 10 William Sjöblom cendio 2022-01-04 16:40:14 CET
Created attachment 1016 [details]
Broken Windows 10 client after disconnecting a monitor
Comment 11 William Sjöblom cendio 2022-01-04 16:41:14 CET
Created attachment 1017 [details]
Broken Linux (X11) client after disconnecting a monitor
Comment 12 William Sjöblom cendio 2022-01-05 08:43:29 CET
After a bit of asking around, the behavior exhibited in comment 8 seems to be intended. The Options dialog is still not being synced up correctly to the actual screen setup, but the behavior of the window only occupying one of the two selected screens after the third was disconnected is an intended trade-off when implementing bug 7006. The acceptance criteria should thus be updated to reflect this.
Comment 13 William Sjöblom cendio 2022-01-05 09:23:35 CET
The acceptance criteria are now updated. Since the problems with the out-of-sync Options dialog are independent of hotplug behavior, we can still continue testing the "Full screen over all monitors" and "Full screen over current monitor" parts of the acceptance criteria while awaiting a fix for the Options dialog.
Comment 14 William Sjöblom cendio 2022-01-05 09:37:04 CET
Verified all acceptance criteria not related to full screen over selected monitors on macOS 12.1:

• [3/3] Full screen over all monitors
  • ☑ Connecting a monitor should extend the full screen to cover that
    screen as well
  • ☑ Disconnecting a monitor should keep full screen enabled on the
    remaining monitors
  • ☑ Mirroring/joining of displays should behave the same way as when
    disconnecting/connecting monitors
• [0/7] Full screen over selected monitors
• [2/2] Full screen over current monitor
  • ☑ Connecting a new monitor should result in full screen still being
    enabled on one monitor
  • ☑ When disconnecting a monitor (including that which houses the
    ThinLinc client), the ThinLinc client should still be in full
    screen on one monitor
Comment 15 William Sjöblom cendio 2022-01-05 10:29:32 CET
Verified all acceptance criteria not related to full screen over selected monitors on Fedora 35 running XFCE4 against X11:

• [3/3] Full screen over all monitors
  • ☑ Connecting a monitor should extend the full screen to cover that
    screen as well
  • ☑ Disconnecting a monitor should keep full screen enabled on the
    remaining monitors
  • ☑ Mirroring/joining of displays should behave the same way as when
    disconnecting/connecting monitors
• [0/7] Full screen over selected monitors
• [2/2] Full screen over current monitor
  • ☑ Connecting a new monitor should result in full screen still being
    enabled on one monitor
  • ☑ When disconnecting a monitor (including that which houses the
    ThinLinc client), the ThinLinc client should still be in full
    screen on one monitor
Comment 16 William Sjöblom cendio 2022-01-05 12:29:48 CET
Verified all acceptance criteria not related to full screen over selected monitors on Fedora 35 running GNOME against Wayland:

• [3/3] Full screen over all monitors
  • ☑ Connecting a monitor should extend the full screen to cover that
    screen as well
  • ☑ Disconnecting a monitor should keep full screen enabled on the
    remaining monitors
  • ☑ Mirroring/joining of displays should behave the same way as when
    disconnecting/connecting monitors
• [0/7] Full screen over selected monitors
• [2/2] Full screen over current monitor
  • ☑ Connecting a new monitor should result in full screen still being
    enabled on one monitor
  • ☑ When disconnecting a monitor (including that which houses the
    ThinLinc client), the ThinLinc client should still be in full
    screen on one monitor

During the process of connecting and disconnecting monitors, I managed to get to a state where full screen over all monitors would only result in full screen on one of the monitors. This problem persisted over several restarts of the client (all with reasonable client logs) and got resolved once a screen was disconnected and connected again. This very much sounds like a GNOME/Wayland bug, so even though I have not been able to reproduce it I will not spend any more time investigating this.
Comment 17 William Sjöblom cendio 2022-01-05 12:56:34 CET
While testing the upstream changes on Windows 10 I stumbled upon another bug (unique to this platform).

Steps to reproduce:
1. Start client on 3 screen setup
2. Options -> Display -> Full screen on all monitors
3. Connect
4. Minimize the session
5. Mirror two of the screens in the Windows display settings
6. Unminimize the client window
7. The client will now be in full screen on two of the three physical monitors, even though we requested it to cover all monitors

Similar issues can be seen if we start the client with two of the three monitors mirrored and then transition to extending the two previously mirrored displays.
Comment 18 Samuel Mannehed cendio 2022-01-07 19:07:33 CET
I created a PR upstream for fixing the GUI sync problem described in comment #8:

https://github.com/TigerVNC/tigervnc/pull/1405

I noticed that a requirement for the GUI to get out of sync would be that the monitor you disconnect was configured to be the left-most monitor. This would lead to the MonitorArrangement index and the FLTK screen index to point to different actual displays.

I have tested local builds with the changes seen above on Windows 10, macOS 12.1 and Fedora 35 and it seems to fix things. Once the PR is merged I'll do a vendordrop to bring this into ThinLinc.
Comment 21 Samuel Mannehed cendio 2022-01-11 10:15:14 CET
The fix for comment #8 has now been merged with a vendordrop. I have tested the latest jenkins build with 3 monitors on Fedora 35 and Windows 11, the GUI is now properly synced.
Comment 22 Samuel Mannehed cendio 2022-01-11 10:15:45 CET
The issue in comment #17 is not a regression, I could reproduce the problem on Windows 11 with the 4.13.0 client. It is not something we will look further into right now.

As far as I can gather, that means this bug is now ready for testing.
Comment 24 Samuel Mannehed cendio 2022-01-11 14:45:59 CET
(In reply to Samuel Mannehed from comment #22)
> The issue in comment #17 is not a regression, I could reproduce the problem
> on Windows 11 with the 4.13.0 client. It is not something we will look
> further into right now.

Bug 7821 was created for this.
Comment 25 William Sjöblom cendio 2022-01-13 14:34:34 CET
During testing, I discovered bug 7822 which impacts hotplug behavior when mirroring. This remained undiscovered during my previous tests since I used F8->Minimize to get rid of the keyboard grab before pressing Win+P to mirror the screens. Since this is not a regression we will not take care of this right now.
Comment 26 William Sjöblom cendio 2022-01-13 14:43:14 CET
I have now finalized the testing of the acceptance criteria that were started in comment 16. This was done on a Fedora 35 machine running GNOME and Wayland with client build #2314.

• [3/3] Full screen over all monitors (see comment 16)
• [7/7] Full screen over selected monitors
  • ☑ The screen configuration UI in both tlclient and vncviewer should
    reflect the current monitor configuration after each change of the
    physical configuration
  • ☑ Connecting a monitor should not change the number of monitors with
    full screen enabled
  • ☑ Disconnecting a monitor (selected or non-selected) should keep
    full screen enabled on at least one of the remaining monitors
  • ☐ Mirroring/of displays should behave the same way as when
    disconnecting/connecting monitors (works fine if we ignore bug 7822)
  • ☑ Connecting with two or more monitors selected, then exiting the
    client, then reconnecting with one of the previously connected
    monitors disconnected should result in full screen over the same
    number of monitors (if possible)
  • ☑ Given full screen over one selected monitor, when disconnecting
    said monitor the ThinLinc client should still be usable
  • ☑ Given a previous session with full screen over selected monitors,
    disconnecting said monitors and then starting the client should
    result in full screen still being enabled on one of the monitors
• [2/2] Full screen over current monitor (see comment 16)
Comment 29 William Sjöblom cendio 2022-01-13 16:35:26 CET
I have now finalized the testing of the acceptance criteria that were started in comment 15. This was done on a Fedora 35 machine running XFCE and X11 with client build #2314.

• [3/3] Full screen over all monitors (see comment 15)
• [7/7] Full screen over selected monitors
  • ☑ The screen configuration UI in both tlclient and vncviewer should
    reflect the current monitor configuration after each change of the
    physical configuration
  • ☑ Connecting a monitor should not change the number of monitors with
    full screen enabled
  • ☑ Disconnecting a monitor (selected or non-selected) should keep
    full screen enabled on at least one of the remaining monitors
  • ☐ Mirroring/of displays should behave the same way as when
    disconnecting/connecting monitors (works fine if we ignore bug 7822)
  • ☑ Connecting with two or more monitors selected, then exiting the
    client, then reconnecting with one of the previously connected
    monitors disconnected should result in full screen over the same
    number of monitors (if possible)
  • ☑ Given full screen over one selected monitor, when disconnecting
    said monitor the ThinLinc client should still be usable
  • ☑ Given a previous session with full screen over selected monitors,
    disconnecting said monitors and then starting the client should
    result in full screen still being enabled on one of the monitors
• [2/2] Full screen over current monitor (see comment 15)
Comment 30 William Sjöblom cendio 2022-01-13 16:38:06 CET
What remains to be verified now is the acceptance criteria for "Full screen over selected monitors" on Windows 10 and macOS.
Comment 31 William Sjöblom cendio 2022-01-14 14:07:52 CET
I've changed up the acceptance criteria quite a bit to make these more general and independent of implementation details. Still, the verification of the previous acceptance criteria verifies the newer acceptance criteria.

So, what remains to be tested with respect to the new acceptance criteria is verifying all of these with for "Full screen on selected monitors" option on Windows and Mac.
Comment 32 William Sjöblom cendio 2022-01-17 13:45:51 CET
I have found another issue that is seemingly exclusive to Windows. I have not been able to reproduce it on neither macOS nor Fedora 35 (GNOME+Wayland).

Steps to reproduce:
1. Start a ThinLinc session with "Full screen on selected monitors" with all three available side-by-side monitors selected
2. Unplug the middle monitor
3. Watch the client silently die

tlclient.log
> CConn:      End of stream
xinit.log
> VNCSConnST:  closing 127.0.0.1::33458: Desktop configured a different screen
>              layout than requested
Comment 33 William Sjöblom cendio 2022-01-17 14:35:41 CET
Looking at the following xinit.log excerpt (logged once the second screen was disconnected) this looks like a server-side bug that should not fall under the umbrella of this bug:

> Mon Jan 17 14:23:44 2022
>  XserverDesktop: Got request for framebuffer resize to 3840x1200
>  XserverDesktop: 2 screen(s)
>  XserverDesktop:          23449 (0x00005b99): 1920x1200+0+0 (flags 0x00000000)
>  XserverDesktop:           2347 (0x0000092b): 1920x1080+1920+0 (flags
>               0x00000000)
>  XserverDesktop:
>  RandR:       Resizing screen framebuffer to 3840x1200
>  RandR:       Temporarily disabling output 'VNC-0'
>  ComparingUpdateTracker: 774.33 kpixels in / 378.208 kpixels out
>  ComparingUpdateTracker: (1:2.04737 ratio)
>  RandR:       Reconfiguring new output 'VNC-1' to 1920x1200+0+0
>  RandR:       Reconfiguring new output 'VNC-0' to 1920x1080+1920+0
>  XserverDesktop: client gone, sock 14
>  VNCSConnST:  closing 127.0.0.1::33626: Desktop configured a different screen
>               layout than requested
Comment 34 William Sjöblom cendio 2022-01-17 15:33:42 CET
The issue described in comment 32 and comment 33 has now been broken out into bug 7824.
Comment 35 William Sjöblom cendio 2022-01-18 15:37:30 CET
I have now verified the acceptance criteria on macOS 12.1, Windows 10,
Fedora 35 (GNOME+Wayland and XFCE+X11), Fedora 33 (MATE+X11 and
Plasma+X11) and all look good.

• ☑ When changing the system's monitor configuration with a ThinLinc
  client running, the ThinLinc display configuration should still be
  honored.

  At large, yes. For this I have tested connecting monitors,
  disconnecting monitors (including disconnecting all monitors),
  mirroring/joining monitors, as well as rearranging the order and
  orientation of monitors. This was done under all available
  full-screen modes. Things work very much as expected if we exclude
  bug 7824 and bug 7822.

• ☑ Changing the system's monitor configuration should not affect
  ThinLinc's saved display configuration.

  It does not!

• ☑ If the saved display configuration cannot be fully realized on the
  current system monitor configuration, an approximate display
  configuration that closely resembles that of the saved configuration
  should be used instead.

  Yes! This is mainly a concern when using "Full screen on selected
  monitors". When in this display mode, the monitors are indexed from
  left to right (and top to bottom). When the system's monitor
  arrangement changes in this display mode, the client attempts to
  occupy the same monitor indices in the new monitor arrangement (as
  implemented in bug 7006) which I deem a sufficient approximation. If
  the original monitor arrangement is restored, so will ThinLinc's
  choice of full screen monitors. This behavior is produced
  consistently over all native client platforms.

• ☑ The screen configuration UI in both tlclient and vncviewer should
  reflect the system's monitor configuration.

  Yes! The screen selection dialog now seemingly stays completely in
  sync.

Worth noting however is that bug 7822 has proven to be more problematic
than initially thought as this currently impacts mirroring on at least
Fedora 35 running either GNOME or XFCE as well as Fedora 33 running
either MATE or KDE Plasma. In other words, it smells like a general
Linux issue that seems to occur consistently when enabling mirroring
that should probably be looked at right away.
Comment 36 William Sjöblom cendio 2022-01-18 15:41:10 CET
What remains now is that someone not too involved have a look at the release notes as well as the testing performed.
Comment 37 Pierre Ossman cendio 2022-01-24 17:08:37 CET
We noticed on bug 7824 that some error conditions aren't properly logged. We won't fix those corner cases right now, but we want to at least fix the logging so we can clearly identify them if they happen in the wild.
Comment 39 Pierre Ossman cendio 2022-01-25 10:16:48 CET
This is fixed now. The message you get is:

>  RandR:       Cannot find an available output for new screen layout
Comment 40 Frida Flodin cendio 2022-01-25 12:19:39 CET
I have looked over the release notes and the testing done on this bug. The release notes look good and the testing seems reasonable, I could not think of anything more to test.
Comment 41 William Sjöblom cendio 2022-01-25 13:17:03 CET
I have verified that the above fix works as intended in the scenario described in bug 7822, comment 16 using server build #2406.

Previously, the only information that was given was the following line in the client-side logs and nothing in the server-side `xinit.log' 
> SetDesktopSize failed: 3
Now, in addition to the rather unfriendly client-side message, we also get the message in comment 39 in `xinit.log'. I have also done a side-by-side comparison of the newer logs with logs from a couple of server builds back and the slight logging refactoring results in seemingly identical logs apart from the added logging in the case of an error condition.

Worth noting is that I have not tested the other error conditions that also got additional logging in the above fix. This was deemed unnecessary since this additional only consists of logging of string constants without any format string specifiers. The added logging look very much sane though and handles the most relevant scenarios.

Marking as closed.

Note You need to log in before you can comment on or make changes to this bug.