Comment 1 Pierre Ossman 2017-07-04 14:37:29 CEST

Bug 7007 allows a crude workaround for the moment. Start the client with two monitors connected, then connect the third monitor after the client is up and running.

Comment 3 Pierre Ossman 2017-07-04 14:44:44 CEST

Citrix has (had?) this ability via a configuration variable, but not the UI. It used a similar syntax as _NET_WM_FULLSCREEN_MONITORS.

We'd like to add our support to this - it's something that has affected us for some time as the standard corporate configuration is a laptop with 2x external monitors. The laptop has a different DPI/resolution to the external monitors, and Thinlinc/VNC doesn't handle this well - and generally, the expectation is that the laptop screen is used for Windows-based tasks - so a way of choosing specific monitors would solve multiple issues for us.

Comment 8 Hugo Lundin 2021-06-14 13:20:48 CEST

I am working on implementing the feature mentioned in this bug. Currently, I am researching how to uniquely identify displays (both for the implementation and configuration). For future reference, I have written down some of my thoughts about some different approaches below: 

# ~~~ Introduction ~~~

Today, there are two fullscreen related settings available in "Options > Screen": 
- "Full-screen mode"
- "Enable full-screen over all monitors"

When "full-screen mode" is enabled, one display will show vncviewer in fullscreen. When "full-screen over all monitors" is enabled, vncviewer will span over all available screens. The aim of this bug is to add support for choosing which screens to have vncviewer in fullscreen on. To avoid confusion for existing TigerVNC users, we want to maintain the current behaviour when fullscreen is used on one display only, i.e. we want to keep "Full-screen mode" as it is. 

I suggest that we replace "Enable full-screen over all monitors" with "Enable full-screen over selected monitors", that also will show a GUI where the user can identify and choose what monitors to use in this mode. Selecting all monitors will therefore maintain the current behaviour of "Enable full-screen over all monitors". 

Since most users probably will be exposed to this setting via the GUI, we want to make that experience as easy and understandable as possible. For example, it seems reasonable to provide the user with as much information as possible (available on the current platform) about a particular display, to be able to identify it. 

It should also be possible to configure this with a configuration file and from a command line. In those cases, we want to identify the different displays in a robust way, that is easy to write (preferably not containing spaces and different casing) and in the best case is consistent between different platforms. 

# ~~~ Background

To understand this problem, I have looked through the code for a number of open source-projects, to find out how they solve this issue. My conclusions are presented below. 
Monitor identification
TigerVNC uses FLTK for its cross-platform GUI. FLTK has no support for identifying monitors by name. However, `Fl::screen_count()` gives how many screens are available and provides `Fl::screen_xywh(..., int n)` which gives the x- and y-position of the w (width) x h (height) area also given by the function. It also states that "Under MSWindows, Mac OS X, and the Gnome desktop, screen #0 contains the menubar/taskbar" [0].

There is an X11 (Linux only) extension called RANDR (<x11/extensions/Xrandr.h>) that has the ability to identify screens more extensively. This is, for example, what the command line application `xrandr` uses. On systems using X11, the names that can be retrieved are similar to "HDMI0", "DP1" and "VGA0". Those are easy-to-understand identifiers that the user intuitively would understand. However, on systems using Wayland, they are called "WAYLAND0", "WAYLAND1" and so on - which is much less clear. Additionally, this solution requires the RANDR extension to be available, which cannot be guaranteed in all cases. 

On macOS, a name describing the monitor can be retrieved with `NSScreen` and IOKit (<IOKit/graphics/IOGraphicsLib.h>). These names are localized, but are not suitable for a configuration file. For example, it can be "Skärm 1 (1920x1200)". Similarly, on Windows, a name in the format "\\. \DISPLAY1" can be retrieved from `EnumDisplayDevices`· 

Extended Display Identification Data (EDID) is a metadata format used by different display devices to describe their capabilities. Both Linux, Windows and macOS have methods for retrieving this data, which among other things contains the serial number, the manufacturer id and an ASCII display name.

# ~~~ Similar situations

VLC media player is an open source cross-platform application. In "Settings > Video" it has a setting requiring identification of physical displays. Internally, it uses a function provided by QT, `QGuiApplication::screens()`. It provides human-readable names similar to above: "WAYLAND0" on Linux (Fedora with Gnome and Wayland), "\. \\DISPLAY1" on Windows and "Skärm 1 (1920x1200)" on macOS. 

wxWidgets has a class called wxDisplay, which in theory is able to identify a display with `wxDisplay::GetName`. However, its documentation states that "The returned value is currently an empty string under all platforms except MSW" [1]. 

GTK has a function called `get_model()` on its GTKScreen class. According to `...` it hasn’t got its own EDID parser, but instead asks the window manager for it. Mutter (a window manager related to the Gnome project and GTK) has its own EDID parser [2].  
 
# ~~~ Discussion

Considering that TigerVNC is a cross-platform project, we want a robust solution that works well with different windowing systems and operating systems. We also do not want to rely on extensions or frameworks being available, or we at least want to have a solution to fall back on in the case that a dependency is missing on the system.

I think it is important that we go with a solution that is easy to communicate to the user. If the solution is consistent and documentable, it is easy to communicate and refer to if the user has problems. Below, I present some of the alternatives that I have thought about, and discussed with @samuel.  

## --- Alternative 1: Identifying with FLTK’s index

The index provided by FLTK is implementation dependent and cannot be relied upon, if the version of FLTK were to change in the future, or something that FLTK relies on from the system were to change how it enumerates displays. Additionally, it isn’t easy to communicate to the user, because we cannot really be sure how it works internally. Therefore, my conclusion is that this is not a reliable and consistent alternative.

## --- Alternative 2: Identifying with name

Identifying a monitor with its name is tempting, because a name such as "HDMI0" is easy for the user to understand and corresponds well to the real world. Wayland breaks this abstraction, which makes it harder to understand for the user if they aren’t familiar with xrandr and display server protocols, i.e. not really a good alternative for the everyday-user. 

Whilst the configuration does not necessarily need to look the same on different operating systems, this simplifies both documentation, configuration and implementation. Furthermore, having names containing spaces, capitalization and potentially special characters (e.g. "\\. \DISPLAY0") further complicates the configuration for the user.

Having a name as the primary identifier is also complicated because it relies on X11 extensions and macOS/Windows APIs to always provide correct information, with no exceptions. If it turned out that this was not the case in only one case, we *need* to provide some kind of fallback path, which complicates how it is communicated to the user, but also how it is implemented. 

## --- Alternative 3: Identifying with EDID

As previously mentioned, EDID can provide us with the serial number of a display, which potentially could be used as an identifier. I have read (but haven’t been able to try it on my own) that some systems do not report reliable information, which makes this solution questionable.

There is also a particular case to consider here, that I think can be common; that a user has a laptop + an external display available in multiple different locations. If we were to use the serial number for identification, the configuration would have to be changed when moving the laptop between different locations. This can be confusing for the user, but also annoying, having to change the configuration regularly. 

## --- Alternative 4: Our own numerical index

FLTK consistently provides the resolution and position of a display, according to the system configuration. If the RANDR extension is available on the system, we can retrieve a name for the display that can be found in other places too. 

My suggestion is that we create our own index from the resolution and position of the display. For example, we could have documentation that describes that the displays are enumerated; left to right, top to bottom (at all times). This is easy to communicate and would unlike alternative 3 allow the user to have a better experience with a laptop and different external displays, because the configuration and experience would work even though it is not the same physical display. 

Additionally with being coupled tightly with the real world (the user can look at their displays and count the order easily) we could provide the indices with a command line flag for advanced users that want to put it manually in a configuration file. For example `--enumerate-displays` or similar. By communicating this information ourselves, we don’t rely on, for example, that the xrandr tool provides information that corresponds to us, nor the desktop environment.

Because a numerical index is not that clear GUI-wise, we don’t even need to show it there. Instead, we can query the system for all information it has available (for example from the RANDR extension, operating system API:s, EDID). However, if no information is to be found, we still have a fallback that is easy to understand.

# ~~~ Conclusion

Identifying hardware and communicating things to the user is hard. We do not want to rely on X11 extensions not available on all systems. We want the implementation to be robust, and therefore want to use information that is not fully reliable (such as display name) for making displays recognizable in the GUI and use more reliable information (resolution and position) for making our own, consistent index. 

# ~~~ Sources

[0] `screen_xywh() [4/4]`, https://www.fltk.org/doc-1.3/group__fl__screen.html. 
[1]https://docs.wxwidgets.org/trunk/classwx_display.html#ab8c2e28185f3df8f06a8ff44a034964e
[2] https://gitlab.gnome.org/GNOME/mutter/-/blob/master/src/backends/edid-parse.c

Comment 9 Pierre Ossman 2021-12-06 15:43:00 CET

There is some bug in this code:

https://github.com/TigerVNC/tigervnc/issues/1387

It looks like getting the display name can sometimes fail. We probably need a bit more defensive coding here.

Comment 11 William Sjöblom 2021-12-13 11:38:35 CET

Created attachment 1006 [details]
ASan heap-buffer-overflow in vncviewer

When running an ASan instrumented vncviewer, I get the following error when attempting to connect or open the Options dialog. I have not done any further digging into this but it looks very much related to the changes introduced in this bug.

This vncviewer was built from the master branch of TigerVNC (2b25d837e) using the following cmake line:
> cmake -DCMAKE_CXX_FLAGS='-fsanitize=address' -DCMAKE_C_FLAGS='-fsanitize=address' .

Comment 12 Pierre Ossman 2021-12-16 08:58:32 CET

As part of this work we should make sure that the special widget we made is usable by any FLTK users, as we want collaboration around it rather than having it specific to TigerVNC.

This means:

 * Relicensing it in a FLTK compatible way
 * Making sure it doesn't use any TigerVNC stuff (since TigerVNC is strictly GPL)
 * Possibly renaming it with a Fl_ prefix to clearly separate it from TigerVNC

This is possible for now since we are the sole copyright holders for that piece of code.

Comment 14 Pierre Ossman 2021-12-28 11:07:42 CET

Unfortunately this change exposes some limitations with our configuration handling that needs to be fixed first:

 * Handling of enumerations cannot be combined with command line override (which we need for -x to work)

 * We don't clearly separate between user config and system config, which makes migration difficult

Comment 15 Pierre Ossman 2021-12-28 16:11:20 CET

I have something that I think should work now. Regression items to check:

 * xdm mode:
   * forced full screen, all screens
   * full-screen options disabled

 * command line force and lock flags

 * disabling features by removing binaries

 * server name and port on command line

 * config file handling:
   * unix system config
   * unix user config
   * windows system config
   * windows user config
   * .tlclient file on command line

 * REMOVE_CONFIGURATION
   * .tlclient file on unix
   * .tlclient file on windows
   * unix user config

 * linkkit (see bug 3513 though)

Comment 16 Pierre Ossman 2021-12-29 14:04:45 CET

(In reply to Pierre Ossman from comment #15)
>  * xdm mode:
>    * forced full screen, all screens

Yup

>    * full-screen options disabled

Yup

> 
>  * command line force and lock flags
> 

Yup. Tested all flags that override options as well.

>  * disabling features by removing binaries
> 

Yup. Got the following in the log:

2021-12-29T12:22:53: Disabled local drives due to missing NFS server
2021-12-29T12:22:53: Disabled serial port redirection due to missing serial port daemon
2021-12-29T12:22:53: Disabled sound due to missing sound daemon
2021-12-29T12:22:53: Disabled smart card export due to missing daemon
2021-12-29T12:22:53: Disabled local printer due to missing pdf2ps

>  * server name and port on command line
> 

Works. Host in server name field and the port number in the options.

>  * config file handling:
>    * unix system config

Correctly reads settings from system conf if there is no user conf. 
Correctly saves settings back to user conf and doesn't touch system conf.
Correctly uses built-in defaults is both confs are missing and saves only to user conf.

>    * unix user config

Correctly overrides system config with settings from user conf.

>    * windows system config

Correctly reads settings from system conf if there is no user conf. 
Correctly saves settings back to user conf and doesn't touch system conf.
Correctly uses built-in defaults is both confs are missing and saves only to user conf.

>    * windows user config

Correctly overrides system config with settings from user conf.

>    * .tlclient file on command line
> 

Correctly overrides user and system config. Changes are saved back in to the .tlclient file, not the user config.

Tested on both Linux and Windows

>  * REMOVE_CONFIGURATION
>    * .tlclient file on unix

Correctly removed after open. Doesn't try to save anything to the user config.

>    * .tlclient file on windows

Correctly removed after open. Doesn't try to save anything to the user config.

>    * unix user config

Correctly removed after open. Doesn't try to save any configuration at all.

> 
>  * linkkit (see bug 3513 though)

After hacking around the bugs we have in the linkkit I got it working and the new widget is included and works fine.

Comment 31 Pierre Ossman 2022-01-03 17:33:50 CET

Tested on:

 * Fedora 34, X11
 * Fedora 34, Wayland
 * macOS 12
 * Windows 11

One monitor was also rotated to make sure that works.

> * It should be possible to configure to use a specific sub set of monitors in full screen

Yup. Tried two out of three local monitors.

> * It should be possible to configure things graphically

Yup. Can click and select monitors.

> * It should be easy to understand which monitor are configured in the GUI

Yup. Monitors are displayed in roughly the same as in the system monitors configuration, which in turn matches their physical layout (assuming the user configured something that matched).

> * The GUI should use the same labels as the system's monitor configuration GUI (BONUS)

Not completely. GNOME shows the name the monitor identifies itself with, which isn't directly available via Xrandr. I guess they parse they EDID information to get this.

However Xfce shows the same thing we do, which is the names of the output ports. E.g. "DP-2".

On Wayland this gets worse as we only get fake output names (e.g. "WAYLAND0"). The properties with EDID information are also missing, so we can't get that information either.

macOS shows the name of the monitor, which is easily accessible for us so we show the same. However this is broken on ARM:

https://github.com/TigerVNC/tigervnc/issues/1399

Windows doesn't give any useful details at all about the monitors. It just assigns an arbitrary number to them that it can display on each monitor to let you figure out which is which. Since this number is arbitrary it is not something we match.

> * It should be visible which "extra" monitors are also included

Yup. Those are rendered using a chequered pattern, instead of the normal filled.

> * The configuration values should follow a system that is easy to understand

I think so, yes.

> * The configuration should be stable as long as the logical layout of screens is stable

Yup. The only input to the system is screen dimensions, nothing else.

> * Two systems with the same logical monitor setup should have identical configuration

Yup. Same here.

> * The configured monitors should be remembered, even if another mode is temporarily used

Yup. Tried both switching whilst connected, and doing a series of reconnects with different modes.

> * If the configured setup isn't possible then a useful fallback should be used

Seems sensible enough:

 * If some monitors cannot be found, then only the valid ones are used
 * If no monitors can be found, then the current monitor is used
 * Same thing if no monitors are configured at all

> * There should be a log entry when the configured setup cannot be used

Yes, you get multiple "Cannot find monitor for entry: 5" from tlclient. A bit redundant, but too much is better than too little.

> * There should be a log entry when the stored configuration is invalid

Yes, you get "Invalid monitor entry: foo" from tlclient.

> * Changes to which monitors are selected should take effect immediately

Yup. Opening options and changing selected monitors works well whilst connected.

> * A session created in full-screen mode should be created with the correct dimensions already from the start

Yup.

> * If an old configuration is used then it should be migrated to an equivalent new configuration (BONUS)

Yup. Tested with FULL_SCREEN_ALL_MONITORS both off and on, and both in user config and system config. You also get a log entry stating "Migrated old full screen configuration".

Comment 32 Pierre Ossman 2022-01-03 17:35:26 CET

Also need to check mirroring, since that required special handling.

Comment 34 Pierre Ossman 2022-01-04 09:40:40 CET

Mirroring now fixed and working properly.

Note that neither GNOME nor Xfce allows you to configure partial mirroring (i.e. 1 & 2 are mirrored, but not 3). So you'll have to do it from the command line using something like:

> $ xrandr --output DP-1 --same-as DP-2

Comment 35 Linn 2022-01-05 14:58:53 CET

Tested client build 2308 on Windows 10. Tested all regression tests from comment 15, except for 'linkkit'.


> * command line force and lock flags
All flags work as expected. 'Force' enables the option related to each flag (but allows the option to be changed), and 'lock' prevents the value from being changed.


> * disabling features by removing binaries
I deleted all .exe files except tlclient, which gave me the following output in the log file:

2022-01-05T13:04:41: Disabled local drives due to missing NFS server
2022-01-05T13:04:41: Disabled serial port redirection due to missing serial port daemon
2022-01-05T13:04:41: Disabled sound due to missing sound daemon
2022-01-05T13:04:41: Disabled smart card export due to missing daemon

Note that the line about local printer is not present, even though file 'pdftocairo.exe' was removed. When opening Options -> Local Devices, all settings except for 'Printer' are removed. 'Printer' should probably be removed as well, but this issue has been separated to bug 7819.


> * server name and port on command line
 Correct server name in the server field and correct port in options.

> * config file handling:
>   * windows system config
Uses settings from system config if there is no user config, and uses default settings if both configs are missing. Saves changes to settings into user config, not system config.


>   * windows user config
Has priority over system config.


>   * .tlclient file on command line
When using this, all changes are saved to that separate file, without saving to the user config.


> * REMOVE_CONFIGURATION
>   * .tlclient file on windows
Truncates the file and saves changes in there, which is correct behaviour if the file cannot be removed. Does not save any changes to user config



Also tested upgrading from 4.13.0 and can confirm the settings are kept during an upgrade. 
I also tested to manually change the values of the config to both valid and invalid values. The valid value were reflected correctly in the GUI, and the invalid values did not cause any warnings in the log.

Comment 36 Linn 2022-01-11 17:14:52 CET

Found a regression when running two nested clients. Tested with 3 monitors.

Setup
=====
* Outer client: Build 2309
  - Fullscreen on all monitors
    OR
  - Fullscreen on selected monitors, all monitors selected

* Inner client: Build 2310

Scenario
========
1. Log in to the outer client

2. Start the inner client. Change its diplay settings to 'Windowed'.

3. Log in to the inner client

4. Change to 'Fullscreen on current monitor' through the F8 menu
  - either directly in the menu or by going to Options -> Display

5. Disconnect the inner client through the F8 menu

6. Start the inner client again. Check that the display settings are still on 'Windowed'. Log in

7. The client starts in fullscreen. Trying to leave fullscreen through the F8 menu does not work.
  - either directly in the menu or by going to Options -> Display


If the outer client is started with 'Fullscreen on current monitor', the inner client will correctly start in windowed mode in step 7.


This is probably quite an unusual case, but since it is a regression we should have a look at it.

Comment 37 Pierre Ossman 2022-01-12 15:35:40 CET

This is the accidental full-screen request code that isn't working for some reason. I can see this in the log of the local window manager (marco 1.22.4):

> Window manager warning: Treating resize request of legacy application 0x3200004 (ossman@oss) as a fullscreen request

I'm not sure why though. We've changed so we no longer adjust the position, just the size. But looking at the source for marco it is supposed to examine both position and size. So adjusting the size should be sufficient, but for some reason isn't.

Comment 38 Pierre Ossman 2022-01-12 17:09:28 CET

The problem seems to be that marco doesn't respect our initial position. I can't find the code for it, but debug dumps shows that it places the window at 0,0 rather than the request 50,100. Hence triggering this legacy logic. My best guess is that it realises that the window won't fit at those coordinates, so it adjusts them down until it reaches 0,0.

These issues are probably inherited from metacity (of which marco is a fork), and is likely still in place in mutter (GNOME 3) as well.

Comment 39 Pierre Ossman 2022-01-12 17:38:21 CET

I can also see why you cannot get out of full screen when this happens. We leave full screen via a request to the window manager. The window manager then tries to restore us to our previous position. But since we started in full screen that previous position are the coordinates that triggered a legacy full screen request, so we get thrown back in to full screen again.

This doesn't normally happen when you start the client in full-screen mode though. The reason for that is that we don't actually create the window in full-screen mode. We create it as a normal window, and then switch to full screen once it is mapped. There were simply too many bugs in window managers when it comes to creating full-screen windows (this here being one of them).

Comment 41 Pierre Ossman 2022-01-12 17:52:19 CET

Improved workaround in place. Need to retest resizes where the size has the same dimensions as the monitor.

Comment 42 Pierre Ossman 2022-01-12 17:55:10 CET

Also note that the last bug generally only happened with multiple monitors. The reason being that we make sure the session fits on the _work area_, not just the monitor. The work area is pretty much always smaller than the monitor, so no risk of triggering this bug.

However in multihead, two issues arise:

 * Secondary monitors can have work areas that cover the entire monitor

 * FLTK cannot figure out the work area for even the primary monitor when multiple monitors are active, so it just gives you the dimensions for the entire monitor

Comment 43 Linn 2022-01-13 16:31:18 CET

Found another regression when testing with client build 2310. Reproduced this both on macOS 12 and Ubuntu 20.04.

Scenario
========
1. Change FULL_SCREEN_MODE in tlclient.conf to a number execpt for 0 and 1 (I used 9708).

2. Start the client and log in. You will get error message "Your session lasted less than 10 seconds", and find this line in the log:
> 2022-01-13T13:11:52: vncviewer[E]:  Config:      Bool parameter FullScreen: invalid value '9708'

In 4.13, it is possible to log in for this scenario. Also note that if FULL_SCREEN_MODE instead is changed to a random string (that also should be invalid), it is possible to log in.

Comment 44 Pierre Ossman 2022-01-14 09:13:40 CET

This is a variation on bug 7697. Although FULL_SCREEN_MODE was more tolerant to garbage before, the overall situation is the same; that you get unreliable behaviour if you have invalid stuff in your configuration.

As such, this is not a blocking regression.

Comment 45 Linn 2022-01-18 14:54:32 CET

Tested the acceptance criteria against a server on SLES 15 server build 2394 with client build 2310 on the following platforms:
  - Ubuntu 20.04
  - Windows 10
  - macOS 12

Used 3 monitors for all platforms.


> * It should be possible to configure to use a specific sub set of monitors in full screen
Yes. Can be set both through the GUI and through the config file.


> * It should be possible to configure things graphically
Yes.


> * It should be easy to understand which monitor are configured in the GUI
Yes, the monitors are shown in the expected order and it is clear which monitors are selected and which are unselected.


> * The GUI should use the same labels as the system's monitor configuration GUI (BONUS)
Yes, when hovering the monitors in the GUI the system monitor name and its resolution is shown as the tooltip.


> * It should be visible which "extra" monitors are also included
Yes, the "extra" monitors are clearly distinguishable with the chequered pattern. The "extra" monitors also only included when they should be, i.e. when that monitor is in between two selected monitors.


> * The configuration values should follow a system that is easy to understand
Yes, and more about the new parameters can be found in these 2 commits to TigerVNC:
https://github.com/TigerVNC/tigervnc/pull/1282/commits/c084e586927ff040014d6a8ea8c519b3a9a368d2
https://github.com/TigerVNC/tigervnc/pull/1282/commits/0d43b96d1c2385fc8202462cf170d7ac7da5d2f7


> * The configuration should be stable as long as the logical layout of screens is stable
It is.


> * Two systems with the same logical monitor setup should have identical configuration
Yes. Tested connecting to the SLES 15 server from both an Ubuntu 20.04 machine and a Fedora 33 machine, both showed the same monitor setup.


> * The configured monitors should be remembered, even if another mode is temporarily used
Yes. Tested both with changing modes inside the session and changing modes outside of the session, reconnecting in after each change.


> * If the configured setup isn't possible then a useful fallback should be used
Different scenarios and their fallback:

- No monitors selected in GUI
- Setting FULL_SCREEN_SELECTED_MONITORS to invalid value in tlclient.conf
  -> Fallback: Fullscreen on current monitor

- FULL_SCREEN_SELECTED_MONITORS contains a mix of valid and invalid monitors
  -> Fallback: Fullscreen only on the valid monitors, which includes "extra" monitors if applicable

- Setting FULL_SCREEN_ALL_MONITORS to invalid value in tlclient.conf
- Setting FULL_SCREEN_MODE to invalid value in tlclient.conf
- Setting FULL_SCREEN_MONITOR_MODE to invalid value in tlclient.conf
  -> Fallback: Starting session in windowed or fullscreen mode, depending on the platform.

Note that if FULL_SCREEN_MODE is set to an invalid int, you get the error below. This is the same issue as seen in bug 7697, so it is left for that bug.
2022-01-13T13:11:52: vncviewer[E]:  Config:      Bool parameter FullScreen: invalid value '9708'


> * There should be a log entry when the configured setup cannot be used
Yes, when FULL_SCREEN_SELECTED_MONITORS=99 this line is logged:
2022-01-10T15:12:39: Cannot find monitor for entry: 99


> * There should be a log entry when the stored configuration is invalid
Yes, when FULL_SCREEN_SELECTED_MONITORS=﷐[U+1F604]﷑ this line is logged:
2022-01-10T15:07:00: Invalid monitor entry: ﷐[U+1F604]﷑

Note that when variables FULL_SCREEN_ALL_MONITORS, FULL_SCREEN_MODE or FULL_SCREEN_MONITOR_MODE are invalid, no logging about this is done.


> * Changes to which monitors are selected should take effect immediately
Yes, tested changing modes inside the session.


> * A session created in full-screen mode should be created with the correct dimensions already from the start
It does.


> * If an old configuration is used then it should be migrated to an equivalent new configuration (BONUS)
Yes. Tested by modifying both user conf and the system conf so it contained old variable FULL_SCREEN_ALL_MONITORS and then started the client. The value was then correctly migrated to the variable FULL_SCREEN_MONITOR_MODE. If the value of FULL_SCREEN_ALL_MONITORS was invalid, the new value defaulted to 'current'.
  - FULL_SCREEN_ALL_MONITORS=1 -> FULL_SCREEN_MONITOR_MODE=all
  - FULL_SCREEN_ALL_MONITORS=0 -> FULL_SCREEN_MONITOR_MODE=current
  - FULL_SCREEN_ALL_MONITORS=bad_value -> FULL_SCREEN_MONITOR_MODE=current

Comment 46 Linn 2022-01-18 15:25:05 CET

Found two different issues regarding mirroring on 2 out of 3 monitors that doesn't work on _some_ Linux desktop environments.

Setup
=====
Fedora 33
Client build 2310
3 monitors, 2 of the screens are mirrored.


Scenario 1
==========
1. Set fullscreen on all monitors, either through 'selected' or 'all'.

2. Log in. Fullscreen is only on current monitor.

-> Should be fullscreen on all monitors.


Scenario 2
==========
1. Start the client, move it to the unmirrored monitor.

2. In the display options, select only the mirrored monitor. Log in

3. The client starts in fullscreen on the unmirrored monitor, i.e. the current monitor and not the monitor that was chosen.

-> Should be fullscreen on the mirrored monitor.


Tested Desktop Env:s
====================

LXDE
----
Does not have multihead support, so scenario 1 and 2 cannot work correctly. _NET_WM_FULLSCREEN_MONITORS is not set.


Openbox
-------
Does not have multihead support (see bug 7171), so scenario 1 and 2 cannot work correctly. _NET_WM_FULLSCREEN_MONITORS is not set.


GNOME
-----
Faulty behaviour for scenario 1 and 2, _NET_WM_FULLSCREEN_MONITORS is _not_ set when mirroring. However, _NET_WM_FULLSCREEN_MONITORS is set when _not_ using mirroring, so there is some support for handling it.


MATE
----
Correct behaviour for scenario 1 and 2. _NET_WM_FULLSCREEN_MONITORS is set both when using and not using mirroring.


KDE Plasma
----------
Correct behaviour for scenario 1 and 2. _NET_WM_FULLSCREEN_MONITORS is set both when using and not using mirroring.


Since we have the correct behaviour on Mate and KDE, it is not a bug on our side, but rather a bug in the desktop environments. These two scenarios will not be fixed as part of this bug.

Comment 47 Pierre Ossman 2022-01-18 16:44:13 CET

It's also worth noting that only KDE has support for setting up this kind of partial mirroring from the UI. So GNOME might consider this an unsupported configuration and hence not a bug.

Comment 48 Frida Flodin 2022-01-18 16:54:44 CET

Did some regression testing with client build 2309 on macOS 10.15.7. Tested all regression tests from comment 15, except for 'linkkit'.

>  * xdm mode:
>    * forced full screen, all screens
Yes


>    * full-screen options disabled
Yes


> 
>  * command line force and lock flags
> 
Works


>  * disabling features by removing binaries
> 
Yes, and the features can not be found in the settings.


>  * server name and port on command line
> 
Works. Host in server name field and the port number in the options.


>  * config file handling:
>    * unix system config
Works as expected.


>    * unix user config
Correctly overrides system config with settings from user conf.


>    * .tlclient file on command line
> 
Correctly overrides user and system config. Changes are saved back in to the .tlclient file, not the user config.


>  * REMOVE_CONFIGURATION
>    * .tlclient file on unix
Correctly removed after open. Doesn't try to save anything to the user config.


>    * unix user config
Correctly removed after open. Doesn't try to save any configuration at all.

Comment 49 Linn 2022-01-19 09:12:50 CET

Tested mirroring against a server on SLES 15 server build 2394 with client build 2310 on the following platforms:
  - Fedora 33
  - Windows 10
  - macOS 12
  
Used 3 monitors.

Mirroring on all 3 monitors:
  ✓ Fullscreen all monitorsᵃ
  ✓ Fullscreen selected monitorᵃ
  ✓ No selected monitorsᵃ
    - Fallback: Fullscreen on current
  
Mirroring on 2 out of 3 monitors:
  ✓ Fullscreen all monitorsᵇ
  ✓ Fullscreen selected monitorsᵇ 
  ✓ Fullscreen selected: mirrored screenᵇ
  ✓ Fullscreen selected: unmirrored screenᵇ


ᵃ: Not tested on Windows 10, cannot mirror 3 screens out-of-box.

ᵇ: On Linux, it depends on the desktop environment if this works or not. See comment 46 for more details.

Comment 50 Linn 2022-01-19 10:04:20 CET

Tested parts of the regression tests from comment 15 with client build 2318 on Fedora 33.

> * config file handling:
>   * unix system config
Uses settings from system config if there is no user config, and uses default settings if both configs are missing. Saves changes to settings into user config, not system config.
   
   
> * unix user config
Has priority over system config.
   

> * .tlclient file on command line
Uses values from this file, and saves additional config here.


> * REMOVE_CONFIGURATION
>   * .tlclient file on unix
Yes, the file is removed when the client is started and changes are not saved into the user config.

> * unix user config
Yes, the file is removed when the client is started.

Comment 51 Linn 2022-01-21 12:49:45 CET

Tested linkkit for 64-bit linux, works well. Also looked through the code, it looks good.

With that, everything is tested. Closing!