novnc / noVNC

VNC client web application
https://novnc.com
Other
11.68k stars 2.31k forks source link

Odd colors when connecting to VMWare VNC #70

Closed ekohl closed 12 years ago

ekohl commented 13 years ago

We're trying to integrate noVNC into our product which uses VMWare ESX(i) as a hypervisor and that works, but the colors are off. The following screenshots shows the difference:

Screenshot of a windows desktop using Remmina

Screenshot of a windows desktop using Remmina

Could this be related to issue #44?

kanaka commented 13 years ago

Based on the color change, this means that the ESX server is sending BGRA rather than RGBA which noVNC is expecting (i.e. red and blue are swapped).

This might mean the server is not listening to the client's pixel format messages. I just pushed out a small updated that prints out the whole pixelFormat received from the server. Can you update your version of noVNC run with "?logging=info" and copy the output (from Chrome debugger or firebug) so that I can see what the server is sending?

Also, here is a small patch that you can try to see if it fixes the color problem (it's not a final patch, just an experiment):

--- a/include/rfb.js
+++ b/include/rfb.js
@@ -1363,9 +1363,9 @@ pixelFormat = function() {
     arr.push16(255);  // red-max
     arr.push16(255);  // green-max
     arr.push16(255);  // blue-max
-    arr.push8(0);     // red-shift
+    arr.push8(16);     // red-shift
     arr.push8(8);     // green-shift
-    arr.push8(16);    // blue-shift
+    arr.push8(0);    // blue-shift

     arr.push8(0);     // padding
     arr.push8(0);     // padding

I don't have any way to test again ESX so you'll have to be my eyes and ears for this and there may be some back-and-forth before we get it right.

ekohl commented 13 years ago

As requested the output with logging=info:

Canvas supports imageData
Using Canvas createImageData
Prefering javascript operations
Data URI scheme cursor supported
Using native WebSockets
New state 'loaded', was 'disconnected'. Msg: noVNC ready: native WebSockets, createImageData rendering
util.js (line 71)
New state 'connect', was 'loaded'.
util.js (line 71)
connecting to ws://localhost:5845/
New state 'ProtocolVersion', was 'connect'. Msg: Starting VNC handshake
util.js (line 71)
Server ProtocolVersion: 003.008
New state 'Security', was 'ProtocolVersion'. Msg: Sent ProtocolVersion: 003.008
util.js (line 71)
New state 'Authentication', was 'Security'. Msg: Authenticating using scheme: 2
util.js (line 71)
New state 'SecurityResult', was 'Authentication'.
util.js (line 71)
New state 'ClientInitialisation', was 'SecurityResult'. Msg: Authentication OK
util.js (line 71)
New state 'ServerInitialisation', was 'ClientInitialisation'. Msg: Authentication OK
util.js (line 71)
Screen: 800x600, bpp: 32, depth: 24, big_endian: 0, true_color: 1, red_max: 255, green_max: 255, blue_max: 255, red_shift: 16, green_shift: 8, blue_shift: 0
New state 'normal', was 'ServerInitialisation'. Msg: Connected (unencrypted) to: vm.example.org
util.js (line 71)
First FBU latency: 17
Timing of full FBU, cur: 165, total: 165, cnt: 1, avg: 165
full FBU round-trip, cur: 368, total: 368, cnt: 1, avg: 368

And it appears to be the same whether your small patch is applied or not.

ekohl commented 13 years ago

http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1246 states:

Configuring the VNC Client for Best Performance

There are two VNC client settings you should ensure are specified before you connect to a virtual machine. Make sure the client is set for hextile encoding and make sure the client is set to use all colors. These settings should avert any display issues.

ekohl commented 13 years ago

Is there anything else I can provide or test?

kanaka commented 13 years ago

@ekohl, please try this patch and set if it corrects the colors. It's not a final fix, just an experiment to make sure I understand the problem.

https://gist.github.com/1083698

ekohl commented 13 years ago

This yields interesting results. What is not visible but becomes clear is that the colors are swapped at the mouse. Moving the mouse away often returns the colors to their original value (depending on how fast you move the mouse away).

noVNC with https://gist.github.com/1083698 applied

kanaka commented 13 years ago

New experiment for you to try. This one actually works for me (i.e. it's not swapped for me). I think I may have misunderstood the color ordering algorithm in the past (and perhaps VMWare doesn't support a non-default order). If so, this should work for you too. If it does, this is probably closer to a full solution. I'll need to get a bunch of people to test it with their VNC servers before I make it the default to make sure I don't break other people.

https://gist.github.com/1085214

ekohl commented 13 years ago

I can confirm that this works for me. Thanks a lot.

kanaka commented 13 years ago

Awesome. Okay, I've created a branch for this bug for testing: https://github.com/kanaka/noVNC/tree/issue-70

The source tarball can be downloaded from https://github.com/kanaka/noVNC/tarball/issue-70

I'm going to ping a bunch of people to try and get this branch tested against a number of VNC servers. Seems like a good time to setup a developer mailinglist too. Too bad that github doesn't have this functionality. I guess most github projects that want a mailinglist use google code for the mailinglist part.

ekohl commented 13 years ago

On Sat, Jul 23, 2011 at 04:12:04PM -0700, kanaka wrote:

Awesome. Okay, I've created a branch for this bug for testing: https://github.com/kanaka/noVNC/tree/issue-70 I noticed this last week and already modified our software to use it as a git submodule. Again, thanks a lot.

drennalls commented 13 years ago

I was having this issue as well. The tarball above worked perfectly, thanks ! The only 'hitch' was I'm on python 2.4 (Centos 5) so I had to make the following change in order for it run (since 'partitiion' was introduced in python 2.5).

--- utils/wsproxy.py.old        2011-07-18 13:17:47.000000000 -0400
+++ utils/wsproxy.py    2011-09-01 02:02:07.000000000 -0400
@@ -255,7 +255,8 @@

     # Parse host:port and convert ports to numbers
     if args[0].count(':') > 0:
-        opts.listen_host, sep, opts.listen_port = args[0].rpartition(':')
+        opts.listen_host, opts.listen_port = args[0].split(':')
+
     else:
         opts.listen_host, opts.listen_port = '', args[0]

@@ -267,7 +268,8 @@
         opts.target_port = None
     else:
         if args[1].count(':') > 0:
-            opts.target_host, sep, opts.target_port = args[1].rpartition(':')
+            opts.target_host, opts.target_port = args[1].split(':')
+
         else:
             parser.error("Error parsing target")
         try:    opts.target_port = int(opts.target_port)
kanaka commented 13 years ago

@ahmedis: yes that tarball was made prior to fixing that issue. The proper fix is actually:

opts.listen_host, opts.listen_port = args[0].rsplit(':', 1)
....
opts.target_host, opts.target_port = args[1].rsplit(':', 1)

The above allows IPv6 addresses to be used (which have ':'s in the address),

ekohl commented 12 years ago

Any chance this is getting merged?

kanaka commented 12 years ago

Just merged, please test.

kanaka commented 12 years ago

Since this is on master and I haven't heard any complaints, I'm closing. Please post if you see problems and I'll re-open.