New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add graphics in terminal support: - Sixel and iTerm2 protocols #2973
base: master
Are you sure you want to change the base?
Conversation
- 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.
You are a hero my brother |
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). |
- 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.
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. |
- Move working bitmap code (drawing current bitmap) to WorkingTerminalBitmap class. - Move bitmap handling code to TerminalBitmap class.
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? |
To avoid removing elements from the map while iterating over it. This should solve the Concurrent Modification Exception in the bitmap garbage collection.
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.
It is a bug. should be fixed now. |
I get outofmemory crash error just for keept open a few text editor and if try to change the orientation of display , or change size. |
@Anonymous2716 what's your device specifications and OS version? |
@HardcodedCat Crash DetailsCrash Thread: Crash Message:
Stacktrace
Termux App InfoAPP_NAME: Device InfoSoftwareOS_VERSION: IS_DEBUGGABLE: HardwareBOARD: SUPPORTED_ABIS: 4GB of RAM |
21804bc
to
3f7a939
Compare
Make sure you are running one of the latest GitHub CI builds, Sixel support isn't in the stable release yet. |
I literally just installed it. It used to work fine on one of the versions, but after a certain point, that's it. Maybe I should reinstall termux from scratch? |
First of all, thank you all for your work on this! 🙏🏾
Thanks for these... been working around these for a quite a while. In addition, setting the Most importantly, I have some concerns with the last point here (from the OP):
It would be greatly helpful to know what exact parameters are ignored (without having to figure it out by oneself). I suggest that all unimplemented protocol features, protocol extensions and inconsistencies with other implementations (particularly the reference implementation by iTerm2 itself) be clearly stated and documented for the sake of developers writing software to use this protocol. One of the major headaches in developing cross-terminal software is inconsistency in implementation of protocols/specifications... and it's sincerely quite a pain. Talking about inconsistencies... for instance,
Note that I'm not saying the answers to questions like these should be documented. Instead, if the answers differ from those of the reference implementation, then those differences should be documented... or better still, fixed! This also kinda applies to the sixel implementation (maybe see this for reference/benchmarks/tests). Once I'm able to get a working build of Termux with this branch and some time, I'll test as much as I can and give any necessary feedback. Thanks once again 🙏🏾 |
This is in #3098.
Checking the code, it seems that the only ignore parameter is
The images are also cleared.
Each cell may display either a part of an image or a character. Each cell displays the last thing (text or image) printed to it. I believe this is the standard behaviour for sixel and iterm2 protocols for all terminal emulators. kitty protocol has more advanced behaviour, and cannot really be supported with the current graphics implementation.
In general, I am more interested in getting current applications working than in bug to bug compatibility with 40 years old hardware. But even there I will sometimes choose a more reasonable behaviour. For example - xterm limits sixel images to 1024 and screen width (the smaller of those). I limit to 8192 and not to screen width. This breaks lsix.
If you want to run termux with this PR (and some other), without building yourself, there is https://github.com/HardcodedCat/termux-monet |
Oh, great!
Will try that out. Thanks. |
Is this being worked on? Can't wait for this feature but I'm seeing there has been no activity for over half a year |
The feature is finished as far as I am aware and pending inclusion in a future release. |
i want. test apk or old sixel worked version ! what and called CI where is this? edited-> CI maybe meaning continuous integration GitHub action. |
idc if this get merged, where can I get an apk ??? o.o |
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 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 t
to give actual sizeCSI 16 t
4
(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.