Xpra: Ticket #1426: scrolling improvements

follow up from #1232.

Issues:

So high that we might has well not bother!



Thu, 02 Feb 2017 11:35:36 GMT - Antoine Martin: attachment set

add logging for scroll timing within encode_scrolling function


Thu, 02 Feb 2017 11:40:41 GMT - Antoine Martin:

Using the patch above, we can see that there are 2 main areas that take too long to complete:

2017-02-02 02:01:12,113 updated scroll data, previously set: True
2017-02-02 02:01:12,116 best scroll guess took 4ms, matches 76% of 1485 lines: -80
2017-02-02 02:01:12,118 encode_scrolling(XShmImageWrapper(BGRX: 0, 0, 1923, 1485), {..}, [], [], {'scroll': True}) window-dimensions=(1923, 1485)
2017-02-02 02:01:12,175  will send scroll packets for yscroll=[-80, -79, -81, -78, -82, -77, -83, -76, -84, -75, -85, -74, -86, -73, -87, -72, -88, -71, 0, -70, -89, -69, -90, -68, -67, -91, -66, -92, -65, -64, -93, -63, -62, -94, -61, -95, -60, -96, -97, -59, -97, -59, -98, -58, -98, -58, 11, -57, 11, -57], elapsed=56ms
2017-02-02 02:01:12,180  scroll groups for distance=-80 : [(71, 429), (599, 167), (865, 540)]
2017-02-02 02:01:12,183  scroll groups for distance=-79 : [(1405, 1)]
2017-02-02 02:01:12,186  scroll groups for distance=-78 : [(1406, 1)]
2017-02-02 02:01:12,189  scroll groups for distance=-77 : [(1407, 1)]
2017-02-02 02:01:12,192  scroll groups for distance=-76 : [(1408, 1)]
2017-02-02 02:01:12,195  scroll groups for distance=-75 : [(1409, 1)]
2017-02-02 02:01:12,198  scroll groups for distance=-74 : [(1410, 1)]
2017-02-02 02:01:12,201  scroll groups for distance=-73 : [(1411, 1)]
2017-02-02 02:01:12,204  scroll groups for distance=-72 : [(1412, 1)]
2017-02-02 02:01:12,208  scroll groups for distance=-71 : [(1413, 1)]
2017-02-02 02:01:12,211  87 lines have not changed
2017-02-02 02:01:12,215  scroll groups for distance=-70 : [(1414, 1)]
2017-02-02 02:01:12,218  scroll groups for distance=-69 : [(1415, 1)]
2017-02-02 02:01:12,221  scroll groups for distance=-68 : [(1416, 1)]
2017-02-02 02:01:12,224  scroll groups for distance=-67 : [(1417, 1)]
2017-02-02 02:01:12,226  scroll groups for distance=-66 : [(1418, 1)]
2017-02-02 02:01:12,229  scroll groups for distance=-65 : [(1419, 1)]
2017-02-02 02:01:12,232  scroll groups for distance=-64 : [(1420, 1)]
2017-02-02 02:01:12,234  scroll groups for distance=-63 : [(1421, 1)]
2017-02-02 02:01:12,237  scroll groups for distance=-62 : [(1422, 1)]
2017-02-02 02:01:12,240  scroll groups for distance=-61 : [(1423, 1)]
2017-02-02 02:01:12,243  scroll groups for distance=-60 : [(1424, 1)]
2017-02-02 02:01:12,248  scroll groups for distance=11 : [(784, 9), (1450, 2)]
2017-02-02 02:01:12,251  distance matching took 73ms
2017-02-02 02:01:12,253  non scroll: 6 packets: [(500, 99), (766, 18), (793, 25), (820, 45), (1439, 11), (1452, 33)]
2017-02-02 02:01:12,257 compress: 137.4ms for 1923x1485 pixels at    0,0    for wid=1     using scroll as  25 rectangles  (11154KB)           , sequence    90, client_options={'scroll': True, 'flush': 6}
2017-02-02 02:01:12,260  splitting took 6ms
2017-02-02 02:01:12,262 sub_image 190377 pixels: 0.1ms
2017-02-02 02:01:12,268 compress:   6.0ms for 1923x99   pixels at    0,500  for wid=1     using   jpeg with ratio   5.4%  (  743KB to    40KB), sequence    91, client_options={'scroll': True, 'flush': 5}
2017-02-02 02:01:12,271 sub_image 34614 pixels: 0.1ms
2017-02-02 02:01:12,276 compress:  14.1ms for 1923x18   pixels at    0,766  for wid=1     using   jpeg with ratio   6.1%  (  135KB to     8KB), sequence    92, client_options={'scroll': True, 'flush': 4}
2017-02-02 02:01:12,279 sub_image 48075 pixels: 0.1ms
2017-02-02 02:01:12,285 compress:  22.5ms for 1923x25   pixels at    0,793  for wid=1     using   jpeg with ratio   1.3%  (  187KB to     2KB), sequence    93, client_options={'scroll': True, 'flush': 3}
2017-02-02 02:01:12,288 sub_image 86535 pixels: 0.1ms
2017-02-02 02:01:12,296 compress:  33.8ms for 1923x45   pixels at    0,820  for wid=1     using   jpeg with ratio   5.0%  (  338KB to    17KB), sequence    94, client_options={'scroll': True, 'flush': 2}
2017-02-02 02:01:12,299 sub_image 21153 pixels: 0.0ms
2017-02-02 02:01:12,304 compress:  41.5ms for 1923x11   pixels at    0,1439 for wid=1     using   jpeg with ratio   1.8%  (   82KB to     1KB), sequence    95, client_options={'scroll': True, 'flush': 1}
2017-02-02 02:01:12,306 sub_image 63459 pixels: 0.1ms
2017-02-02 02:01:12,313 compress:  50.4ms for 1923x33   pixels at    0,1452 for wid=1     using   jpeg with ratio   4.1%  (  247KB to    10KB), sequence    96, client_options={'scroll': True}
2017-02-02 02:01:12,315 non-scroll encoding using jpeg (quality=42, speed=90) took 52ms for 6 rectangles
2017-02-02 02:01:12,318 scroll encoding total time: 199ms

The whole thing should be cythonized. For the v1.0.x branch, maybe we can just limit the size of the search space (<1K vertical?)


Thu, 02 Feb 2017 13:03:10 GMT - Antoine Martin: status, description changed


Thu, 02 Feb 2017 15:07:18 GMT - Antoine Martin: attachment set

this cythonized version of the code is super fast, but also not visually correct (yet)


Fri, 03 Feb 2017 14:44:30 GMT - Antoine Martin: attachment set

almost done - almost renders correctly


Fri, 03 Feb 2017 14:45:45 GMT - Antoine Martin:

r14965 avoids the unnecessary memory copy with the X11 image wrapper class. The patch above is almost complete (just needs an off-by-one fixed)

Other improvements planned:


Sat, 04 Feb 2017 08:31:19 GMT - Antoine Martin:

After much time spent debugging, the new scroll code is just fine, the real problem comes from the new jpeg encoder instead: #1423.

So r14972 commits the new scroll code, and turns off jpeg for non-scroll region updates.

Still TODO:


Useful debugging steps for future reference:

This will make the paint code really slow (lack of delayed flush) and save each FBO to disk so we can see the screen updates one by one, including the scroll ones.

So we can see the original region we sent for non-scroll areas.


Sun, 05 Feb 2017 13:41:15 GMT - Antoine Martin: attachment set

avoid back and forth: stay in cython


Mon, 06 Feb 2017 05:57:39 GMT - Antoine Martin:

Cythonization complete in r14978. We no longer use python-xxhash and ship xxhash with a cython wrapper instead. I've kept "python-xxhash" packaging in the tree for now so we can continue to build updates from trunk for v1.0.x.

Remaining tasks:


Mon, 06 Feb 2017 10:18:04 GMT - Antoine Martin: owner, status changed

r14984 enables scrolling without requiring video support - also some minor fixes and improvements in r14994, r15002. It can be tested with the python client using:

xpra attach --encodings=png,jpeg,rgb --no-mmap -d draw

Or with the HTML5 client, but that one seems to have paint issues with the "scroll" encoding... which will need to be fixed and backported, see #1432.

The non-scroll paints will also be less optimal than before, because we end up in the video codepath - which will often encode a larger region.. This can happen if the page has lots of animation on it.

Partial line scrolling has been moved to #1429, so this is ready for testing.


@afarr: mostly a FYI at this point, scroll should perform better than before but that's quite difficult to quantify without running the profiling tools. The more important improvement is for the HTML5 client, but this still needs fixing...


Thu, 09 Mar 2017 09:30:06 GMT - Antoine Martin:

Fixed some visual corruption, see ticket:1432#comment:4, the html5 client still needs fixing...

Edit: now fixed, see ticket:1432#comment:5


Thu, 20 Jul 2017 20:29:29 GMT - J. Max Mena:

Retesting with a Trunk r16434 Fedora 25 server and client:

I'm still seeing the visual corruption you mentioned in comment:7 despite being well after the version it was fixed with in #1432. I also triggered a few error prints so I'll look into it a bit further. (sidetracked with other things right now) I managed to get it to stick once, but other than that one time it seems to clear up after a second.


Thu, 20 Jul 2017 20:29:47 GMT - J. Max Mena: attachment set


Thu, 20 Jul 2017 22:55:52 GMT - J. Max Mena:

Oh neat I got a traceback while writing a new ticket:

2017-07-20 15:55:17,523 Error during scrolling detection
2017-07-20 15:55:17,524  with image=XShmImageWrapper(BGRX: 1193, 71, 12, 754), options={'scroll': True}
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/xpra/server/window/window_video_source.py", line 1645, in do_video_encode
    return self.encode_scrolling(scroll_data, image, options)
  File "/usr/lib64/python2.7/site-packages/xpra/server/window/window_video_source.py", line 1470, in encode_scrolling
    for scroll, line_defs in raw_scroll.items():
AttributeError: 'list' object has no attribute 'items'

Fri, 21 Jul 2017 04:52:13 GMT - Antoine Martin:

The stacktrace from comment:9 was caused by r16343 (2.1 only - just a few days ago) and is fixed by r16436. Happens when the video region size changes. It should have been harmless, we detect something has gone wrong and fallback to non-video encoding when anything like that happens.


Mon, 24 Jul 2017 23:04:09 GMT - J. Max Mena: owner changed

Alright after the change in r16436 (upped server to r16498) I'm not seeing that traceback any more. I still see the corruption while scrolling(screenshot I attached between comment:8 and comment:9), but it clears after I finish scrolling.

@antoine: Is that still a bug, or is the refresh enough to close this?


Thu, 03 Aug 2017 17:33:30 GMT - Antoine Martin: owner changed

Is that still a bug, or is the refresh enough to close this?

Visual corruption is a bug: the scrolling moves things around, and the regions that are updated separately may get painted with lossy jpeg. But the screenshots show something else, it looks like scroll and paints are happening out of order: attachment/ticket/1426/Xpra%20HTML%20Paint%20Bug.png

The question is: is this related to the improvements to scrolling from this ticket (due to be closed for the 2.0 release...), or a bug in the HTML5 client scroll paint code (which would belong in a different ticket). If you cannot reproduce similar visual corruption with the standard python client (with opengl), then it is the latter.


Fri, 08 Sep 2017 10:38:57 GMT - Antoine Martin: status changed; resolution set

Not heard back, closing.


Mon, 11 Sep 2017 17:35:43 GMT - J. Max Mena: status changed; resolution deleted

Woops look like this one slipped through the cracks.

I can't reproduce it via the Python client, but it's still reproducible as of trunk r16825.


Mon, 11 Sep 2017 17:39:58 GMT - Antoine Martin: status changed; resolution set

I can't reproduce it via the Python client, but it's still reproducible as of trunk r16825.

See comment:12 :

... or a bug in the HTML5 client scroll paint code (which would belong in a different ticket)


Sat, 23 Jan 2021 05:23:59 GMT - migration script:

this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/1426