follow up from #1232.
Issues:
scroll encoding total time: 89ms
So high that we might has well not bother!
add logging for scroll timing within encode_scrolling function
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
elapsed=56ms is the time spent figuring out which scroll values to send
distance matching took 73ms
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?)
this cythonized version of the code is super fast, but also not visually correct (yet)
almost done - almost renders correctly
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:
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:
XPRA_PAINT_FLUSH=0 \ XPRA_OPENGL_SAVE_BUFFERS=png \ xpra attach -d draw --no-mmap
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.
XPRA_SAVE_TO_FILE=png \
xpra start -d scroll
So we can see the original region we sent for non-scroll areas.
avoid back and forth: stay in cython
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:
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...
Fixed some visual corruption, see ticket:1432#comment:4, the html5 client still needs fixing...
Edit: now fixed, see ticket:1432#comment:5
Retesting with a Trunk r16434 Fedora 25 server and client:
xpra start :13 --html=on --start-via-proxy=no --start-child=xterm --start-new-commands=yes --bind-tcp=0.0.0.0:
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.
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'
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.
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?
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.
Not heard back, closing.
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.
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)
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/1426
