Kitty image preview

[mark-dawn]

ISSUE TYPE

• Improvement/feature implementation

RUNTIME ENVIRONMENT

• ranger version: ranger-master 1.9.0
• Python version: 3.6.4 (default, Jan 5 2018, 02:35:40) [GCC 7.2.1 20171224]
• Locale: en_US.UTF-8
• Operating system and version: Arch Linux (up to date)
• Terminal emulator and version: kitty 0.7.1 and XTerm(331)
• Python version: 3.64 - 2.7.14
• Ranger version/commit: #0b4f6b2a

CHECKLIST

• The CONTRIBUTING document has been read [REQUIRED]
• All changes follow the code style [REQUIRED]
• All new and existing tests pass [REQUIRED]
• Changes require config files to be updated
  • Config files have been updated
• Changes require documentation to be updated
  • Documentation has been updated
• Changes require tests to be updated
  • Tests have been updated

DESCRIPTION

• Added support for previews in kitty using its innate image protocol (similar to terminology or iTerm2). Support come in two flavors, one leveraging the local storage (faster) and one streaming the image alongside the draw commands (slower but network transparent). Manapages and rc.conf have been updated to advertise this new option.
• (Minor) Refactored some code in the img_display.py module that was used in three subclasses as an helper function.
• Fixed support and docs for terminology: during tests for the above change I notices that support for terminology was broken, independently from the changes (due to a missing \n). This has been fixed; also the lack of documentation for the terminology support was fixed too, adding it to both the manpages and rc.conf

Notes and issues

Since kitty only likes png or raw RGB data PIL is required as a dependency. I also plan on implementing an extra backend using the external ImageMagick toolkit, like kitty does for his own icat utility. I left it for a later time since this method has been shown to fail occasionally, and is potentially slower since it has to invoke external processes. In any case, PIL is only imported at the class level and not at the module level, leaving other options alone.
It is also possible to change KittyImagePreview to automagically select between streaming and local file, but due to a bug and my incompetence with stdin this feature is delayed, and the choice is made in rc.conf

MOTIVATION AND CONTEXT

The changes have been motivated by the fact that kitty has native support for images, meaning less edge cases, and in general w3m doesn't work on terminals that refresh independently of input, like kitty. This brings full image preview to kitty, basically.

#983 Relevant discussion

TESTING

The new branch has been used both on python2 and 3 (packages installed with pacman are python-pillow and python2-pillow).

The standard test suite has been run.

The core of the codebase is mostly untouched, except for intialization of ImageDisplay classes in fm.py naturally. The _get_image_displayer function has been updated with the relevant branches and excluded from pylint too-many-return-statements since ""fixing"" the warning would amount to add a local variable and make the function one line longer and potentially slower.

The other touched part is obviously img_display.py with most changes being orthogonal to the original code, except for the excision of a small common helper function to temporarely move the cursor (now wrapped nicely as a context manager). Terminology, which had the most complex use case has seen no change to its (buggy and now fixed) behavior. I couldn't test iTerm2 graphically but the text output looks fine.

IMAGES / VIDEOS

kitty showing images for more than half a millisecond (you will have to believe me on the persistence of the output)

[toonn]

👏 Great work. I didn't really get into the code changes because it's so much and I'm not familiar with this part of the codebase but there's a couple typos I noticed.

[toonn]

Small typo "vector graphics".

[toonn]

Typo "quickly through". "by with" drop one.

[toonn]

End the sentence with a '.' : )

[toonn]

Maybe move the url to a footnote?

[toonn]

Needs a rephrasing. "and scrolling too fast through a folder..."

[toonn]

"bit" -> "but"

[toonn]

"it makes it possible to see previews from ranger over the network" (emphasis for clarity not intended as emphasis)

[toonn]

Still missing terminology setting?

[toonn]

Shouldn't the height be determined dynamicall?

[toonn]

Typo "temporarly".

[mark-dawn]

Oh, wow, those are some embarrassing-grade typos. I even left in notes about the scrolling bug when it has been solved...I should have double checked this stuff. Thanks for the rundown.

Shouldn't the height be determined dynamicall?

*dynamically :P Jokes aside, that's a very bad name when taken out of context. It is not related to the viewbox rectangle, but it is the max tolerated height before the image gets scaled down internally to ease the transfer. I should just base it off the actual area now that I think of it.

[toonn]

Even though it's technically not referring to the viewport it would seem to me like it's definitely dependent on the viewport. If you're scaling down anyway it doesn't make sense not to scale down all the way to a small viewport. Having a hard cap makes some sense but should imo be based on (10Gb) LAN so not a very hard cap ; )

[mark-dawn]

Technically you are half right, but the issue is not only bandwidth, but also the fact that the payload in the APC is base64 encoded. Encoding a filename is easy, encoding a whole big image in RBG(A) is a minute worth of time, at least with the basic python implementation. I thought it would also benefit slow hdds when saving the temporary image when not in network mode.
However, turn out this point is moot, since I am saving to a temporary file, which python correctly creates in /tmp, so they are mapped to RAM! Indeed I am testing this on my laptop which has a slow hdd and I don't see any difference when I get rid of the resize.

So for now I am going to execute the resize only in the case of kitty-network, but in a future commit I want to implement what you suggested, scaling down the image to the best viewport I have. Since kitty fills the viewport (which is expressed in columnsxrows) stretching the image (with a nearest-neighbor filter) you can get some artifacts despite my best efforts. If I could resize the image with a better filter the effect would be nicer, but for that I will have to find out the size in pixel of a character cell.

[toonn]

Thank you for all the work on this. I'm waiting for a couple kitty users to test this. Or some more maintainer eyes on the patch, since it's significant.

[tkerber]

I've been testing this PR for personal use and have noticed the following:

• When using python2 and ranger, no images display, stack traces with stdout.buffer not existing get outputted.
• The error message for PIL not being found is undescriptive on its own, and doesn't terminate in a new line.
• Images smaller than the preview area get scaled up, leading to unwanted artifacting.
• SVG images fail to display.

Apart from that, great job, I'm looking forward to being able to see images again :)

[mark-dawn]

Thanks @tkerber for the tests!

• When using python2 and ranger, no images display, stack traces with stdout.buffer not existing get outputted.

My mistakes, and this actually makes sense. What it doesn't make sense is that I tested and it worked for me before. Probably a typo when calling python2. Anyway it is an easy fix.

• The error message for PIL not being found is undescriptive on its own, and doesn't terminate in a new line.

I actually had no idea on how to show an error message, I thought that exception were silently ignored, but it turns out that if you have a message attached it gets printed in red to the status bar. I am moving it there in the next commit, possibly adding something like "please search your repositories for the python library pillow" (suggestions welcome).

• Images smaller than the preview area get scaled up, leading to unwanted artifacting.

This is "by design" in a way, but I do agree that it looks kinda of meh. I didn't notice because I was testing with a medium sized terminal in a folder with big images. Anyway, since I was going to implement resizing the thumbnail to the viewport size (with some decent filter) for downsizing, it should made upsampling a bit better too. However I do agree that it might be good to want images sized as close as possible to the native size, so would it make sense to implement a config option to switch between this different behaviors? It could possibly be back-ported to other protocols, too.
I would like some opinions on this.

• SVG images fail to display.

svgs are just too different to be read by pillow. They fail on w3m too, and the only terminal that supports them seems to be terminology. Support might be coming if there is a simple way to generate a raster image from them. Maybe if the user has inkscape installed it could be possible to generate a png in a reasonable timeframe? Alternatively a native library could be the solution. This could also benefit other protocols, too.
TL; DR: if you make use of a lot of svg in your workflow I suggest to switch to the terminology, because they are not coming anytime soon™

[toonn]

I'd make the error message something along the lines of "Image previews require PIL".

Having the scaling be configurable would be good with at least three options: no scaling, downscaling only and both. Backporting would be ideal but if that requires PIL it'd definitely have to be optional like most of ranger's dependencies, silently falling back to sane behavior if it's not available.

SVG support for other methods would also be great if the performance is good enough but this is definitely an enhancement, not of the highest priority. These last two things would be seperate PRs of course.

@tkerber, thank you for testing. Image previews are probably the most common reason for issue reports so we want to be careful adding new methods lest they introduce even more issue reports : )

[mark-dawn]

• Fixed error in opening stdio in binary mode across py2/py3
  • Backported said method to other part of the module since it is not based on hardcoded numbers but on getattr (luckly only one spot thanks to precedent work)
• Realized that some stuff is not working when __init__ is called (curses is not functional yet, ranger crashes when an exception is raised independently of -d), so moved stuff that requires those into a _late_init method that's called on the first draw
  • this allows to print PIL import errors nicely in the statusbar
  • but also to enable automatic detection of filesystem sharing, so now a single option needs to be specified in rc.conf for this whole thing to work. This allows some cool setups, like a local machine that can also be accessed via ssh and the previews will work on the local ranger anyway without having to change configs depending on the situation
• deleted part of the docs that are no longer necessary (relating@network-kitty)

I'll try to tackle the rectangle resize menace right now, and see if I can cook up some solution for that

[kovidgoyal]

Just dropping in with some tips, that might be helpful:

1. You can display SVG images using rsvg-convert (that is how kitty icat itself does it)

2. Rather than creating files in /tmp (/tmp is not always mounted in RAM) you should either create files in /dev/shm, or if that does not exist (for example on non-linux platforms) use the kitty image protocol support for POSIX shared memory directly (see man shm_overview and the kitty graphics protocol page for how to use it). Though I dont know if that is possible to do using only python+ctypes or not.

3. You can get the exact pixel size of a character cell trivially using termios, see https://github.com/kovidgoyal/kitty/blob/master/graphics-protocol.asciidoc#getting-the-window-size and icat.py inkitty for code to do it in python. Once you have that, it is perfectly possible to draw images using the graphics protocol avoiding all scaling.

And let me finish by saying that it is cool to see the kitty image protocol being used in the wild. I am happy to take suggestions for improving the protocol based on actual usage.

[toonn]

Nitpick: I'd stick closer to the original phrasing. "Allows remote image previews, for example in an ssh session." I'm not sure the performance implications should be in the manpage but I could be convinced otherwise.

[mark-dawn]

@kovidgoyal thanks for the tips! I think I am going to leave svgs to a later commit, but this is helpful to know. Talking about /tmp I am using in the sense that NamedTemporaryFile saves there by default on my system. For know I will keep it simple and puth my faith in python choosing a suitable location.

@toonn Noted!

Also I found out that kitty display images in their native size if a rectangle is not specified, so the next commit will solve all issues about resizing artifacts.

[mark-dawn]

I got working previews without artifacts. Smaller images are not scaled at all, while big images are fit in the best way to the preview pane. It was deceivingly simple to implement, contrary to my belief!

Anyway if the test from other users go well and everything else code-wise is fine, I would say this is a great point for merging, since future improvement are kinda worth they own pull request.

[toonn]

Grammar nazi patrol : )
Missing new line before this one. Don't actually care about whether there's one or not but we should keep it consistent.

[FichteFoll]

All other sections have this line in a separate paragraph.

[toonn]

I'd drop "in the protocol that" then add "this" before allows. Something about the sentence seems off otherwise.

[toonn]

I'd like to have at least 2-3 more people test this. If that takes too long I'll get at least one other maintainer to review the code because this is a valuable addition and I don't want it to get neglected for lack of exposure.

[Lompik]

image previewed correctly on kitty as well here. Thanks !

It seems like it tries to display all files thrown at it, regardless of the extension. Shouldn't it align with what ranger think is an image ?

One comment: maybe catch OSError from PIL ?

ranger version: ranger-master 1.9.0
Python version: 3.6.4 (default, Jan  5 2018, 02:35:40) [GCC 7.2.1 20171224]
Locale: en_US.UTF-8
Current file: '/home/lompik/.w3m/w3msrc49966-0'

Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/PIL/ImageFile.py", line 214, in load
    s = read(self.decodermaxblock)
  File "/usr/lib/python3.6/site-packages/PIL/PngImagePlugin.py", line 618, in load_read
    cid, pos, length = self.png.read()
  File "/usr/lib/python3.6/site-packages/PIL/PngImagePlugin.py", line 113, in read
    length = i32(s)
  File "/usr/lib/python3.6/site-packages/PIL/_binary.py", line 76, in i32be
    return unpack(">I", c[o:o+4])[0]
struct.error: unpack requires a buffer of 4 bytes

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/ranger/ranger/core/main.py", line 196, in main
    fm.loop()
  File "/tmp/ranger/ranger/core/fm.py", line 398, in loop
    ui.draw_images()
  File "/tmp/ranger/ranger/gui/ui.py", line 402, in draw_images
    self.browser.columns[-1].draw_image()
  File "/tmp/ranger/ranger/gui/widgets/pager.py", line 115, in draw_image
    self.fm.notify(ex, bad=True)
  File "/tmp/ranger/ranger/core/actions.py", line 166, in notify
    raise obj
  File "/tmp/ranger/ranger/gui/widgets/pager.py", line 111, in draw_image
    self.wid, self.hei)
  File "/tmp/ranger/ranger/ext/img_display.py", line 597, in draw
    image.save(tmpf, format='png', compress_level=0)
  File "/usr/lib/python3.6/site-packages/PIL/Image.py", line 1897, in save
    self.load()
  File "/usr/lib/python3.6/site-packages/PIL/ImageFile.py", line 219, in load
    raise IOError("image file is truncated")
OSError: image file is truncated

[NeoTheFox]

Got it working on my setup without any issues, it works, I've tried some edge cases and so far it works really well!

[soredake]

Tested on gentoo, working good.

[FichteFoll]

Ah, my precious ISO dates NotLikeThis

[FichteFoll]

All other sections have this line in a separate paragraph.

[FichteFoll]

Seems like this file needs to be regenerated because this section doesn't exist in the ranger.pod anymore (there are also some typos above).

[mark-dawn]

@freed00m that's weird, it looks like the console got garbled up. Can you tell me if the files that cause this have something in common? Size maybe, or an aspect ratio? Are they in a specific folder, or have weird filenames?
Also, are you on python 2 or python 3? Also if you send the command redraw_window dos it get fixed?

[freed00m]

@mark-dawn It's not file specific, here an example file https://ibb.co/bLpb3d .

This file can be renamed confortably at ~/Pictures/screenshotblur.png but whenever this file sits at /tmp/screenshotblur.png it triggers this bug.

I am running Manjaro Testing branch so I am cca week behind Arch in terms of versions, currently Python 3.6.6.

I use redraw_window to recover from this bugged state when focused on non image file.

I am thinking maybe the rendering of font Awesome is triggering this? Or maybe my font of choice "Fira font medium" causing this?

If you have some cool breakpoints I could debug I could :)

[freed00m]

I've tried disabling devicons and even changed fonts to no effect.
The weirdest part is when running ranger with vscode's debugger attached, the bug is not triggered..

:O what the heck is going on?

[mark-dawn]

Can reproduce. With all the weirdness. It is partially my fault: I am saving the temporary resized thumbnail that kitty reads and then deletes there.
This causes flashes since ranger quickly updates and orders a redraw, which causes a new temp file which causes a redraw and you get the idea.
What's weird is that the annoying flashing should be it. However those quick refreshes seem to glitch ranger's UI up.
Anyway, I can come up with an "hacky" patch (it would be nice to solve at the root, but it might be impossible depending on what triggers ranger) by basically implementing caching, so temporary files are not continually swapped in and out.
For now I suggest to simply set imagePreviews false when going into /tmp

[kovidgoyal]

@mark-dawn Couple of suggestions:

1. Use shared memory rather than /tmp I dont know if python binds shm_open() which is what you would need, but it should be possible via ctypes
2. Use a sub-directory in /tmp
3. At least on linux, check if /ev/shm is available and use that instead of /tmp (which obviates the need for shm_open

[toonn]

Simplest solution that comes to mind is use a hash instead of the filename. That way you avoid most collisions with little effort. (I assume filenames that equal hash(filename) are rare and people don't store many files with hashes as names in /tmp : )

[mark-dawn]

@toonn The filenames are already hashed.
What triggers the bug is ranger refreshing way to quickly. You can get the same effect by browsing $somedir while running while true; do touch $somedir/test; rm $somedir/test; done. The effect is way bigger than normal since the file is getting linked/unlinked at way higher rates, but the principle is the same.
The weird part is that while refreshing the UI glitches only while previewing an image file, but not with other previews. It might happen due to escape codes from the image commands getting jumbled up in the pipes.
I tried using terminology in the same situation, and while it gives me still a nice epileptic flashing, the UI doesn't glitch.
The only way I see this happening is if ranger can stop a ongoing draw operation if some update happens.

I tried fixing it by using a fixed file that gets rewritten, instead of creating a new file every time. This indeed reduces the flickering, but the file updating still causes refreshes every now and then that corrupt the interface when trying to issue a command.
Another thing I've tried is @kovidgoyal suggestion, and while it doesn't fix the problem, it should make it extremely unlikely (and I could still disable the previews in case some user wanders in /dev/shm). However I would like to check if a binding for shm_open exists in the standard library, as it would make the preview POSIX complaiant

[toonn]

Ah, didn't understand you're continuously deleting the thumbnails.

[mark-dawn]

I found the core of the issue, somehow when the cursor get moved to a new spot to draw the image it might "fail" to get back before the next refresh. In quotes because it might just be late.
The moving is handled in a context manager, and I am now sure that the call to draw is actually blocking, so the cleanup code must be called before ranger can go on with its business. Indeed, by wrapping the calls to curses with stderr debug statement I can see that the "store cursor" and the "load cursor" get called in pairs, even during the glitch.

This restrict the fuckup to curses, since it is the only component that might be asynchronous. So when the cleanup code is called, curses/terminal is instructed to put back the cursor where it was before (async), then the draw subroutine exit, and at this point if a redraw comes fast enough the cursor might not have been back.
I wonder if there is a way to wait for the cursor to be restored. I already flush stdout, so our only hope lies in the terminfo interface. Which might be a sign to look for a workaround like shm or a subfolder.

[toonn]

Why would a subdirectory help exactly?

[mark-dawn]

AH! FIXED! (sort of)
So, for the save and restore cursor stuff I was basically reusing the code that was around the other classes, that goes like this

curses.putp(curses.tigetstr("sc"))  # save cursor pos
sys.stdout.write(curses.tigetstr("cup"), y, x)  # move cursor
do_stuff()
curses.putp(curses.tigetstr("rc"))  # restore cursor pos
sys.stdout.flush()

The problem is that putp must not put stuff directly in stdout, so when python flushes the restore command might not be in stdout yet. The solution is simple, switch putp for a write to python stdout interface so everything goes along in the correct order.
The image still flicker, but at a much lower rate and without any slowdown or glitch, which is a great improvement.

@toonn Oh, I could just not preview anything in the subfolder. Not ideal, but we could avoid disabling previews in the entire /tmp. We could even snuck it in $XDG_RUNTIME_DIR, where it wouldn't even be out of place. The use of /tmp was entirely motivated by python standard library

[mark-dawn]

@toonn I have a new version ready to push that should clean some bugs, but when I run test pylint goes absolutely berserk and reports errors all over the source tree. Pytest and flake8 are fine and everything is rebased on the latest master. Do you have any idea on why that might be the case?
Maybe Arch has a version of pylint that's too new? Could you give me the version the project is currently using? Or is it broken on your side too and pylint is ignored atm on travis?

[FichteFoll]

@mark-dawn

ranger/Pipfile

Line 12 in 8e9451e

pylint = "<2.0.0"

Just use pipenv install --dev to set up your environment.

[mark-dawn]

Thanks, that fixed up everything, except my blindness. I will probably make a new pull request since I am on a new branch.

[Moelf]

on latest commit master 1.9.2, only one line set preview_images true in rc.conf. Ranger still trying to execute w3m?

[toonn]

@Moelf, yes. The kitty method only works in kitty so the default is still w3m of course. I've thought about adding an auto method that tries to detect the terminal in use. That's not exactly straightforward sadly, so it wouldn't always work. No work has been done on this though, it's not a priority tbh.

[kovidgoyal]

Just FYI, the kitty graphics protocol supports detection at runtime. See icat/main.py (the detect_support() function for how the kitty icat tool itself does it).

[mark-dawn]

@toonn we could at least merge the preview_image and preview type settings into one, so people don't get confused, but I don't know if it's worth it to mess with old settings.

@kovidgoyal that would be actually very useful, I still check at runtime if kitty is actually there but I ask the user to set an env variable, which is shitty. I'll take a look asap, thanks.

[toonn]

I don't think we can merge them. That'd mean you can't toggle the setting, because what does toggling mean for a setting that can be an ever expanding number of options?

[Moelf]

@Moelf, yes. The kitty method only works in kitty so the default is still w3m of course. I've thought about adding an auto method that tries to detect the terminal in use. That's not exactly straightforward sadly, so it wouldn't always work. No work has been done on this though, it's not a priority tbh.

Makes sense, but how can I tell ranger to use Kitty method since I always use Kitty anyways?

[toonn]

@Moelf, add/change set preview_image_method kitty in rc.conf. It's all in the man page.

[ThatOneCalculator]

Hey, I know this thread is kinda old, but does anyone know how to use this feature with Kitty? I'm kinda lost.

[freed00m]

@theonecalculator
Did you setup in rc.conf ?

set preview_images true
set preview_images_method kitty

[ThatOneCalculator]

@theonecalculator
Did you setup in rc.conf ?

set preview_images true
set preview_images_method kitty

Worked flawlessly! My bad.