Bug 7158 - We are out of sync with upstream TigerVNC
Summary: We are out of sync with upstream TigerVNC
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.10.0
Assignee: Pierre Ossman
URL:
Keywords: prosaic
Depends on:
Blocks: 400 965 1122 2928 4516 4735 5113 5226 5229 5241 5481 5719 6116 7235 7236 7237 7238 7239 7240 7241 7242
  Show dependency treegraph
 
Reported: 2018-04-23 13:27 CEST by Pierre Ossman
Modified: 2018-09-14 09:44 CEST (History)
2 users (show)

See Also:
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)


Attachments

Description Pierre Ossman cendio 2018-04-23 13:27:14 CEST
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 Pierre Ossman cendio 2018-08-27 10:41:55 CEST
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 Pierre Ossman cendio 2018-08-29 10:43:02 CEST
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 Pierre Ossman cendio 2018-08-29 10:55:42 CEST
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 Pierre Ossman cendio 2018-09-04 13:15:46 CEST
Since this is a vendor drop we're doing the testing as part of closing the bug.
Comment 6 Henrik Andersson cendio 2018-09-11 10:37:56 CEST
Can't create a new session using nightly build of Xvnc:

  Tue Sep 11 10:31:14 2018
   Connections: accepted: 127.0.0.1::35772
   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 rgb888
   VNCSConnST:  Client pixel format depth 24 (32bpp) little-endian rgb888

  Tue Sep 11 10:31:15 2018
   Connections: closed: 127.0.0.1::35772 (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 Pierre Ossman cendio 2018-09-11 12:05:51 CEST
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 Pierre Ossman cendio 2018-09-14 09:44:59 CEST
✓ 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.

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