Skip to content
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

Incorrect/glitchy sixel output, and crash on high sixel image count #905

Closed
ghost opened this issue Aug 29, 2019 · 14 comments
Closed

Incorrect/glitchy sixel output, and crash on high sixel image count #905

ghost opened this issue Aug 29, 2019 · 14 comments

Comments

@ghost
Copy link

ghost commented Aug 29, 2019

Steps to reproduce:

  1. Install cygwin + inetutils + Oracle JDK at version 1.6 or above.

  2. Download jexer-0.3.2.jar from https://jexer.sourceforge.io, or install ant and build it (git clone https://gitlab.com/klamonte/jexer ; cd jexer ; ant jar).

  3. Since the Jexer xterm backend can't spawn "/bin/sh -c stty" from the Win32/64 java.exe, you must use the telnet server demo. Run 'java -cp jexer-0.3.2.jar jexer.demos.Demo2 4444'.

  4. Open another mintty window, 'telnet localhost 4444'

  5. In the Jexer demo application, select the tools (hamburger) menu on far left, and navigate on the left directory treeview to a directory with an image. Double-click the image filename on the right files list.

  6. The image might be scaled incorrectly due to window manipulation issues #899. Regardless, drag the window with the image down a bit.

  7. At this point, the image flickers/stutters within the image window. Issue Internationalisation #1.

  8. Exit the demo, either with Alt-X or selecting the File | Exit menu item. Click on Yes to exit.

  9. Repeat the 'telnet localhost 4444'. mintty crashes with a stackdump. If using a mintty window for both the 'java -cp jexer.jar' (step Better transparency #3) and the telnet (step Don't keep cmd.exe window open #4/Add option to remove the title bar #7), both windows are unusable. Issue Documentation #2.

  10. The telnet.exe process may also still be running in the background, using 100% CPU.

Further details:

Jexer's sixel output is outlined roughly here. Jexer uses many small sixel images to compose a larger picture, and defaults to 1024 colors. (And if you have run into any other terminal multiplexers / windowing toolkits with overlapping image support, I would love to try them out...)

@mintty
Copy link
Owner

mintty commented Aug 29, 2019

Thanks for the detailed description, I'll try to reproduce, but let me comment already now that the fact that

Jexer uses many small sixel images to compose a larger picture

is a bad idea which I'd call a design flaw of Jexer.

@mintty
Copy link
Owner

mintty commented Aug 29, 2019

I see the flickering but I couldn't get the terminal to crash, or the server to stall.

@mintty
Copy link
Owner

mintty commented Aug 29, 2019

Tracing mintty, jexer apparently splits up the image but not into cell-sized parts. It uses 1 sixel image per ~7 cells. This is weird. Why does it split up at all when there is no overlap?

@ghost
Copy link
Author

ghost commented Aug 29, 2019

Jexer uses many small sixel images to compose a larger picture

is a bad idea which I'd call a design flaw of Jexer.

It's working OK in xterm, mlterm, and yaft, and also enables me to use sixel output for font rendering (double-width/height, CJK, and emoji).

Tracing mintty, jexer apparently splits up the image but not into cell-sized parts. It uses 1 sixel image per ~7 cells. This is weird. Why does it split up at all when there is no overlap?

If sixel images overlap each other or text cells, it can result in a corrupted display. More specifically, the text will always appear where and how it should be, but the image might leave artifacts in the region it was originally drawn in. I haven't dug deep into xterm to determine if that is a bug that should be fixed or not, instead I developed a different strategy that (I hope) should work on any terminal with sixel support:

  • For text across the entire screen, only update cells that have changed. (Like ncurses.)

  • For rows that have image or text changes, and contain image data, also redraw all images on that row. So I don't care if my text updates destroy the image, I'm going to replace it anyway.

  • Since the mouse cursor can move multiple rows in a single screen update, Jexer does not always know which rows containing image data might have changed between the last time the mouse was displayed, and its current position. So it conservatively updates all rows between the old and new mouse pointer rows, inclusive.

  • For a row that is updated, the text will be displayed first, then the image. Adjacent image cells are collected into contiguous blocks as you are seeing. These contiguous blocks are also cached, which helps performance a lot.

With the above points, I never require an image to maintain some kind of coherence after text has overwritten it, and I never draw overlapping images. (At least I shouldn't, if I do that is a bug.)

@ghost
Copy link
Author

ghost commented Aug 29, 2019

Also: my cygwin test environment is a 32-bit VM running Win 7 ( enterprise, I think)?

@mintty
Copy link
Owner

mintty commented Aug 29, 2019

Ah, so you are the jexer owner.

Some jexer observations:
The client window is sometimes resized to 24 lines initially, but not always.
If the client window is larger, the image is displayed in stripes (as you described) with visible gaps. The height in the size report is correct, though. Do you also use CSI 16 t for cell size enquiry?

Some mintty observations:
I reproduce the stalling in a 32-bit mintty client, but not in a 64-bit client.
If the image is moved, the number of mintty images that are repeatedly displayed (for refresh) is doubled etc. Maybe that's causing the flickering, I'll try to analyse it.

@ghost
Copy link
Author

ghost commented Aug 29, 2019

The client window is sometimes resized to 24 lines initially, but not always.

Thank you, I'll add an issue for it.

If the client window is larger, the image is displayed in stripes (as you described) with visible gaps. The height in the size report is correct, though. Do you also use CSI 16 t for cell size enquiry?

0.3.2 is only using CSI 14 t. Git head is now using both CSI 14 t and CSI 16 t, and the 16 t response will win out.

@BrianInglis
Copy link

You should also use CSI 18 t for text size e.g.:
text chars 8 ch 120x60 ^[[18t ^[[8;60;120t

@ghost
Copy link
Author

ghost commented Aug 30, 2019

You should also use CSI 18 t for text size e.g.:
text chars 8 ch 120x60 ^[[18t ^[[8;60;120t

I already have the text size from either 'stty size' (stdin/stdout) or telnet NAWS option. I just need to relate that to pixels to get font size, so that I can know how many cells will be covered by the image.

But I suppose I could also ask for 18 t as a fallback to compare to stty.

@BrianInglis
Copy link

stty seems a heavyweight approach to get rows and cols, and telnet can return WONT NAWS to DO NAWS; of course, support of xterm escape sequences is not consistent or guaranteed. The most portable approach may be using ioctl; ioctl_tty(2) says:
"Get and set window size
Window sizes are kept in the kernel, but not used by the kernel (except in the case of virtual consoles, where the kernel will update the window size when the size of the virtual console changes, for example, by loading a new font).
The following constants and structure are defined in <sys/ioctl.h>.
Get window size. TIOCGWINSZ struct winsize *argp
Set window size. TIOCSWINSZ const struct winsize *argp
The struct used by these ioctls is defined as:"

    struct winsize {
        unsigned short ws_row;
        unsigned short ws_col;
        unsigned short ws_xpixel;   /* unused */
        unsigned short ws_ypixel;   /* unused */
    };

Usage is:

    #include <termios.h>
    int ioctl(int fd, int cmd, [const] struct ... *argp);

e.g.

    #include <termios.h>
    rc = ioctl(1, TIOCGWINSZ, &ws);

@ghost
Copy link
Author

ghost commented Aug 30, 2019

Jexer has an explicit design goal not to call C functions directly, so that it can be referenced for or ported to languages with poor C FFI. I started writing another technical bit about the how/why in answer here, but realized that it would be an even better addition to the Jexer porting wiki page .

So thank you both for a productive writing session this morning. :) I've also included the tip on "CSI 8 t" there.

@mintty
Copy link
Owner

mintty commented Sep 2, 2019

As you noticed and also described in https://gitlab.freedesktop.org/terminal-wg/specifications/issues/12#note_218071, the mintty image rendering mechanism is currently not prepared to digest such dynamic usage of sixel output. Particularly, images that get overwritten completely are still maintained AND displayed - just overwritten afterwards. That may be the cause of the flickering.
I'm working on the feature...

@mintty
Copy link
Owner

mintty commented Sep 4, 2019

After fixing one serious bug and optimising sixel rendering for completely overlapped images (which are now removed), I hope this issue is fixed.
There are still some artefacts while moving the cursor over a sixel image in jexer.

@mintty
Copy link
Owner

mintty commented Sep 26, 2019

Released 3.0.3.

@mintty mintty closed this as completed Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants