Bug 7158

Summary: We are out of sync with upstream TigerVNC
Product: ThinLinc Reporter: Pierre Ossman <ossman@cendio.se>
Component: VNCAssignee: Pierre Ossman <ossman@cendio.se>
Status: CLOSED FIXED QA Contact: Bugzilla mail exporter <bugzilla-qa@cendio.se>
Severity: Normal    
Priority: P2 CC: hean01@cendio.se, samuel@cendio.se
Version: 1.3.1Keywords: prosaic
Target Milestone: 4.10.0   
Hardware: PC   
OS: Unknown   
Acceptance Criteria:
* The current master branch of upstream TigerVNC should be merged in to our tree * Any upstream change that requires testing, or a separate mention in the release notes, should have its own bugzilla entry * TigerVNC dependencies should also been upgraded (e.g. libjpeg-turbo and FLTK)
Bug Depends on:    
Bug Blocks: 400, 965, 1122, 2928, 4516, 4735, 5113, 5226, 5229, 5241, 5481, 5719, 6116, 7235, 7236, 7237, 7238, 7239, 7240, 7241, 7242    

Description From cendio 2018-04-23 13:27:14
It's been a long time since we did a vendor drop of TigerVNC and there has been
a lot of work done upstream. We should get back in to sync to get all the
features and fixes done there.
------- Comment #1 From cendio 2018-08-27 10:41:55 -------
The commit used for the previous vendor drop seems to have been
0536d0975bb9e8e4a8aae693bd79157955a896b1. So the following will give a nice log
of what has happened upstream:

$ git shortlog --no-merges 0536d0975bb9e8e4a8aae693bd79157955a896b1..master
------- Comment #3 From cendio 2018-08-29 10:43:02 -------
Vendor drop of current master is done, and everything that warrants separate
bugs have gotten them. Only thing left is seeing if there are any areas that
should get regression testing just to make sure nothing bad got in.
------- Comment #4 From cendio 2018-08-29 10:55:42 -------
We have these functional areas being tested on other bugs:

* Keyboard (tested on bug 400)
* Mouse cursor (tested on bug 1122)
* X11 application compatibility (tested on bug 5241)
* Congestion control (tested on bug 4735)
* Clipboard (tested on bug 7240)

And these that should get some generic testing here:

* Mouse
* Keyboard and mouse grabbing
* Resizing and full screen handling
* Scrolling
* Graphics correctness and performance
* Shadowing
* Authentication
------- Comment #5 From cendio 2018-09-04 13:15:46 -------
Since this is a vendor drop we're doing the testing as part of closing the bug.
------- Comment #6 From cendio 2018-09-11 10:37:56 -------
Can't create a new session using nightly build of Xvnc:

  Tue Sep 11 10:31:14 2018
   Connections: accepted:
   SConnection: Client needs protocol version 3.8
   SConnection: Client requests security type VncAuth(2)
   VNCSConnST:  Replacing existing connection
   VNCSConnST:  Server default pixel format depth 24 (32bpp) little-endian
   VNCSConnST:  Client pixel format depth 24 (32bpp) little-endian rgb888

  Tue Sep 11 10:31:15 2018
   Connections: closed: (Desktop configured a different screen
                layout than requested)
   EncodeManager: Framebuffer updates: 5
   EncodeManager:   Tight:
   EncodeManager:     Solid: 35 rects, 546.836 kpixels
   EncodeManager:            560 B (1:3906.72 ratio)
   EncodeManager:     Bitmap RLE: 2 rects, 12.398 kpixels
   EncodeManager:                 94 B (1:527.83 ratio)
   EncodeManager:     Indexed RLE: 12 rects, 81.527 kpixels
   EncodeManager:                  16.335 KiB (1:19.5045 ratio)
   EncodeManager:     Full Colour: 5 rects, 24.888 kpixels
   EncodeManager:                  6.85352 KiB (1:14.1938 ratio)
   EncodeManager:   Tight (JPEG):
   EncodeManager:     Full Colour: 12 rects, 104.771 kpixels
   EncodeManager:                  61.9844 KiB (1:6.60493 ratio)
   EncodeManager:   Total: 66 rects, 770.42 kpixels
   EncodeManager:          85.8115 KiB (1:35.0795 ratio)
   ComparingUpdateTracker: 665.334 kpixels in / 665.334 kpixels out
   ComparingUpdateTracker: (1:1 ratio)

The issue is reproduced by setting Session size with a odd width such as
1001x768 in combination with Full screen mode. There is no such issue when
connecting to 4.9.0 server.
------- Comment #7 From cendio 2018-09-11 12:05:51 -------
I debugged this, and this is what happens:

1. The server is started with the resolution XxY
2. The client connects, also with resolution XxY (i.e. no change)
3. The client goes to fullscreen, still with resolution XxY

Now if XxY is not the resolution of the client monitor then the client will
trigger a fallback code path that sends a resize request to the server. The
resolution is still XxY, but the id of the monitor is set to 0.

The id on the server is something random, so most likely not 0. From a protocol
point of view the request then means "Turn of the existing monitor, then start
a new one with the same resolution".

However the X server realises that this didn't actually change anything (it
doesn't know about the changed id), so it never sends out any notifications.
And we currently rely on those notifications to update our internal state.
Hence the error message and the client being kicked out.

(This is a regression because of some logic changes in this code. Previously we
disabled the monitor early, then re-enabled it. Now we see if we can re-use it
first, and only disable it if it ended up not being used at all.)
------- Comment #9 From cendio 2018-09-14 09:44:59 -------
✓ Mouse
✓ Keyboard and mouse grabbing
✓ Resizing and full screen handling
✓ Scrolling
✓ Graphics correctness and performance*
✓ Shadowing
✓ Authentication

* Performance is not quite straight-forward since there are large changes in
behaviour compared to 4.9.0. Those will be examined more closely on bug 5719
and bug 2928. I don't see any general changes in performance though.

Should be fine for the general testing.