broken tray icons

[totaam]

Issue migrated from trac ticket # 521

component: core | priority: major | resolution: fixed

2014-02-18 03:44:49: onlyjob created the issue

---

Noticed with 0.11.3: application's tray icons are corrupted -- they appear with coloured vertical stripes. It happens only with some encodings, notably with "PNG (8bpp colour)". The following errors appear in client log:

2014-02-18 14:38:26,788 error processing draw packet
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/xpra/client/ui_client_base.py", line 1509, in _draw_thread_loop
    self._do_draw(packet)
  File "/usr/lib/python2.7/dist-packages/xpra/client/ui_client_base.py", line 1557, in _do_draw
    window.draw_region(x, y, width, height, coding, data, rowstride, packet_sequence, options, [record_decode_time])
  File "/usr/lib/python2.7/dist-packages/xpra/client/client_tray.py", line 105, in draw_region
    assert coding in ("rgb24", "rgb32", "png", "mmap", "webp"), "invalid encoding for tray data: %s" % coding
AssertionError: invalid encoding for tray data: png/P

RAW/RGB_zlib is also exhibiting this problem although without logging any errors.

Icons look OK with H.264 so to reproduce the problem it may be necessary to override default encoding on attach.

[totaam]

2014-02-18 03:47:16: totaam changed status from new to assigned

[totaam]

2014-02-18 03:47:16: totaam changed owner from antoine to totaam

[totaam]

2014-02-18 03:47:16: totaam commented

---

Thanks, we should never send the tray icons as png/P or png/L: those encodings save space by subsampling colours, but since tray icons are small this doesn't save anything at all, in fact in some cases it may even make things bigger (adding the palette to the pixel data).

Could well be related to #500

[totaam]

2014-02-18 04:49:20: onlyjob commented

---

I forgotten to add that it looks like tray icons might get broken in 0.11.3 (at least with RAW/RGB+zlib encoding). It is a regression as I don't remember anything like this in 0.11.2 and earlier...

[totaam]

2014-02-28 10:04:06: totaam commented

---

First, I will probably have to cherry pick some fixes for v0.11 from #500#comment:11.

---

I can reproduce the bug, r5628 will need to be backported and fixes png/P and png/L support for the system tray: if the server wants to use this encoding, let it!

We should pick a better encoding, but that's a different issue.

What is odd is that everything looks correct in the logs:

• with PNG (which displays OK):

ClientTray(2).draw_region[0, 0, 22, 22, 'png', '604 bytes', 0, 4, {}, \
    [<function record_decode_time at 0x33d38c0>]]
ClientTray(2).after_draw_update_tray(True)
ClientTray(2).set_tray_icon() format=('rgb32', 22, 22, 88)
ClientTray(2).set_tray_icon() has_alpha=True

• with RGB (which does not):

ClientTray(2).draw_region[0, 0, 22, 22, 'rgb32', '468 bytes', 66, 5, {'lz4': True, 'rgb_format': 'RGB'}, \
    [<function record_decode_time at 0x33d3a28>]]
ClientTray(2).after_draw_update_tray(True)
ClientTray(2).set_tray_icon() format=('rgb32', 22, 22, 66)
ClientTray(2).set_tray_icon() has_alpha=True

As I expected, the PNG compression ends up being worse than lz4! (604 bytes vs 468 bytes)

[totaam]

2014-02-28 10:38:45: totaam commented

---

I was wrong in the comment above, there is something wrong: the rgb_format says RGB even though this is rgb32 with alpha...
On the server side, I see:

rgb_encode(rgb32, XImageWrapper(BGRA: 0, 0, 64, 64), {}) rgb_formats=['RGB']
rgb_reformat(XImageWrapper(RGB: 0, 0, 64, 64)) converted from BGRA (16384 bytes) to RGB (12288 bytes) in 0.1ms, rowstride=192

Fixes:

• 5629 removes the unecessary encoding check (backport of r5628)
• r5632 ensures we tell the server we can handle RGBA for the tray, backport in 5633
• r5630 ensures that we don't remove RGBA from the list of supported rgb formats unless necessary (for old, broken clients) - which on its own, fixes the symptoms for {{rgb}}} encoding mode, backport in 5631

Still TODO:

• r5624 needs to be backported so we can deal with old / broken clients correctly by sending them the correct encoding for the data (rgb32 for RGBA pixel format instead of rgb24)

Note: this bug has been present for a long time, the only thing that changed is that we are now more likely to use rgb for the tray (and rightly so since it takes up less space and much less effort)

[totaam]

2014-02-28 11:55:20: totaam changed status from assigned to new

[totaam]

2014-02-28 11:55:20: totaam changed owner from totaam to onlyjob

[totaam]

2014-02-28 11:55:20: totaam commented

---

Totally non-trivial backport of seemingly innocuous r5624 in 5634: this has a small side effect: a change in how encoding statistics are recorded... More than I would like for a stable change, but better than keeping the old bug in. And maybe I should have backported the whole of r5605..

Please test and close so I can release 0.11.4, thanks!

[totaam]

2014-03-03 02:49:44: onlyjob changed owner from onlyjob to totaam

[totaam]

2014-03-03 02:49:44: onlyjob commented

---

Tested v0.11.x@5643, works great. Thank you.

[totaam]

2014-03-03 03:26:31: totaam changed status from new to closed

[totaam]

2014-03-03 03:26:31: totaam changed resolution from ** to fixed