Add graphics in terminal support: - Sixel and iTerm2 protocols #2973
Conversation
|
You are a hero my brother |
|
It works! but gifs are being duplicated on the terminal when expanding and collapsing the keyboard Regular images are being displayed fine
|
This is not a bug. img2sixel displays animations by sending each frame as a different sixel with a command to send the cursor to position (1,1) between them (CSI H). This causes each frame to overwrite the previous one. When you remove the keyboard, the screen gets taller, and termux handles this by scrolling down, so the previous frame which was at position (1,1), is now lower, so it is not covered by the following frames. |
|
oh, so everything's working fine then |
Here's a small guide for anyone else who wants to test this PR
|
|
Has issues with big images. For example when using Test image (7779x4191): https://user-images.githubusercontent.com/107305601/188685258-87eb0704-12c5-4b43-a47a-a1deae99e208.jpg Device config: Pixel 5, Android 13, 8 GiB RAM. |
Image displays fine on termux-monet with Device Config: Xiaomi POCO F1, Android 12L, 6GB RAM
|
|
I believe if MatanZ gets this imported and issue 142 gets solved, then MatanZ gets my $100 bounty on Bountysource for that issue. It will be well worth it!! :-D |
|
Thanks, but anyway there is some proper workaround needed to avoid OOM. Larger heap indeed can solve it for a particular case. Not all devices have lots of memory and part is always used by other apps including system, max heap size also vary between device models. Unfortunately I can't propose the best way to implement that, but I think refusing to display supersized bitmaps is better than crashing all sessions. For example if dimensions are too big, user can get a toast message describing the issue. Command line programs should not be able to crash terminal or cause resource overload by just sending some control sequence (no matter whether it is sixel or something else). |
I don't believe that making the terminal refuse to display oversized images is necessary, since most of command line programs always will be able to crash the application when overloaded or abused. That would only limit how the user should use his terminal, creating a "fake error" that doesn't even exist for some users, like the people who has enough RAM for displaying those images. Those people who can display the images wouldn't be able to. But if you guys decide that's the best thing to do, i propose placing a limit only for devices with less than 4GB RAM. |
Just so I can get more testing from the nice people who test my code:
|
|
Tested with imagemagick, and it's displaying fine on my side |
I hope this solves the problem. I put a try{}catch() block around each bitmap operation that allocates memory. This should be similar to what you suggest - ignoring large images, without hard coding a definition of large. I believe it is better without a toast notification. Usually when a terminal cannot (or does not want to) perform a certain operation it just ignores it, without notifying the user. Maybe a line in the log. |
|
@MatanZ Please use dedicated class for long sixel logic in Trying to catch Checking current memory before even trying to display large image could be done too. https://developer.android.com/topic/performance/graphics/load-bitmap#read-bitmap
This is the way. No toasts. Being done in other places too. |
The way graphics display is implemented, it is not possible to implement kitty protocol. It is possible to implement very simplified ("put image here"), but image and placement management, as well as combining text and images is impossible. |
Refactored some code to two new classes. I don't see anything in TerminalEmulator that belongs in another class, or which may become clearer.
I am not sure this is a good idea. I would have to add assumptions about how much memory is used by the android for various bimap manipulations (decode, scale, resize). I'll settle for trying anything the user asks for, and catching the errors. I did not crash termux with this, with all the above, and various other large images. |
|
Thanks. Will take another look later myself during merging. You can also let |
|
Seems like there is a problem with some GIFs. Notice the artifacts at the left part. Same as #142 (comment)? Gif file doesn't seem to be corrupted. screen-20220908-005128.2.mp4This doesn't happen with another gif: screen-20220908-005944.2.mp4 |
|
I cannot see those videos. And can you please include the actual files used, and the commands that cause the problem? |
|
Perhaps the issue with Termux I can reproduce it when connected over SSH to Termux from Konsole on laptop. But I can't reproduce the issue when animation is played via Screenshot with the problem (notice the block with lines on the left side): You can get the file from https://upload.wikimedia.org/wikipedia/commons/thumb/2/2c/Rotating_earth_%28large%29.gif/274px-Rotating_earth_%28large%29.gif libsixel package version differences:
The difference in a build time configuration. Though maybe OpenSUSE also applies some patches to the package. |
Fixes issue with some GIFs. See my comment at termux/termux-app#2973 (comment) Also enable libcurl support.
|
Commit termux/termux-packages@b72be3c resolves |
Fixes issue with some GIFs. See my comment at termux/termux-app#2973 (comment) Also enable libcurl support.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
If some package doesn't work - open a new issue in https://github.com/termux/termux-packages/issues. |
Sixel coverage:
Conclusion: All applications that uses and/or supports Sixel works properly iTerm2 coverage:
Conclusion: Not all applications that uses and/or supports iTerm2 works properly |
Report InfoUser Action: Crash DetailsCrash Thread: Crash Message: Stacktrace |
|
I uninstalled termux so fast I forgot to backup my configuration. |
|
For what it's worth, I think the APK signatures are compatible. Over here, I can install one over the other (and vice versa) without loosing any data... though, I'm not sure how good an idea it is. |
Not if you got your Termux from f-droid sadly |
|
Waiting for kitty support |
|
Out of scope for this PR, please do not reply to dormant threads unnecessarily. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Implement the following CSI escape sequences from https://invisible-island.net/xterm/ctlseqs/ctlseqs.html: > CSI Ps ; Ps ; Ps t > [..] > Ps = 1 4 ⇒ Report xterm text area size in pixels. > Result is CSI 4 ; height ; width t > [..] > Ps = 1 6 ⇒ Report xterm character cell size in pixels. > Result is CSI 6 ; height ; width t Extracted from changes in #2973 by @MatanZ and adopted to play well with the just merged #3098 (.ws_xpixel and .ws_ypixel values in winsize).
Implement the following CSI escape sequences from https://invisible-island.net/xterm/ctlseqs/ctlseqs.html: > CSI Ps ; Ps ; Ps t > [..] > Ps = 1 4 ⇒ Report xterm text area size in pixels. > Result is CSI 4 ; height ; width t > [..] > Ps = 1 6 ⇒ Report xterm character cell size in pixels. > Result is CSI 6 ; height ; width t Extracted from changes in #2973 by @MatanZ and adopted to play well with the just merged #3098 (.ws_xpixel and .ws_ypixel values in winsize).
Implement the following CSI escape sequences from https://invisible-island.net/xterm/ctlseqs/ctlseqs.html: > CSI Ps ; Ps ; Ps t > [..] > Ps = 1 4 ⇒ Report xterm text area size in pixels. > Result is CSI 4 ; height ; width t > [..] > Ps = 1 6 ⇒ Report xterm character cell size in pixels. > Result is CSI 6 ; height ; width t Extracted from changes in termux#2973 by @MatanZ and adopted to play well with the just merged termux#3098 (.ws_xpixel and .ws_ypixel values in winsize).
- In TerminalEmulator, interpret sixel sequences, and send them to TerminalBuffer for constructing a bitmap. - Sixel sequences may be longer than 8192 characters, so break them in natural places ($,-,#), rather than collecting all in the buffer. - The bitmap is sliced to character cell sized slices, and each the the style attribute is used to store which bitmap slice is displayed in place of this character. - In TerminalRenderer the style is interpreted, and drawn using drawBitmap, instead of drawText. Support iTerm inline image protocol (OSC 1337): - Using the same bitmap display infrastructure introduced for sixels. - Collects the image data outside of the OSC buffer. - Ignoring some parameters. Small emulator changes: - Also eat APC sequences, not echoing to screen. - Fix `CSI 14 t` to give actual size - Add `CSI 16 t` - Add `4` (sixel) to device attributes
Add missing {} that change the logic.
- For iterm2 images - catch the error, and cancel the image. - For sixels - if it happens when resizing the bitmap, than ignore drawing outside of the current image.
- Move working bitmap code (drawing current bitmap) to WorkingTerminalBitmap class. - Move bitmap handling code to TerminalBitmap class.
To avoid removing elements from the map while iterating over it. This should solve the Concurrent Modification Exception in the bitmap garbage collection.
Avoid crash when BitmapFactory cannot decode image
…last column. This creates a zero length text run, so skip it.
This commit implements graphics in the terminal, including two different protocols for placing images:
I ran this for a few days, and it seems to me that some people tried it from #142, and saw no crashes, so I hope it is reasonable safe.
TerminalBuffer for constructing a bitmap.
natural places ($,-,#), rather than collecting all in the buffer.
the style attribute is used to store which bitmap slice is displayed
in place of this character.
drawBitmap, instead of drawText.
Support iTerm2 inline image protocol (OSC 1337):
Small emulator changes:
CSI 14 tto give actual sizeCSI 16 t4(sixel) to device attributesFor those who tried this branch already, I force pushed to it in order to remove changes that may not be acceptable.
The main known issue is that if graphics sequence is interrupted (in specific places), the emulator remains stuck waiting for the sequence to end, thus ignoring all input. This requires terminal reset from the menu.