hackerb9 / lsix

Like "ls", but for images. Shows thumbnails in terminal using sixel graphics.
GNU General Public License v3.0
4.01k stars 127 forks source link

Workaround the cursor placement bug #51

Open hackerb9 opened 3 years ago

hackerb9 commented 3 years ago

It has been pointed out by @j4james that, in correct sixel implementations, the text cursor after showing a sixel image should overlap with the bottom of the image. It seems likely that terminal emulators will gradually change to reflect that because that is how a full screen sixel image can be shown without scrolling. There are also subtle problems that occur with different heights of sixel images. See:

This patch checks $TERM and adds a text newline if the terminal is known to have a correct sixel implementation. Otherwise, it is presumed the terminal will be adding its own newline.

Relatedly, this patch also removes the graphics newline that is appended to the end of sixel images generated by ImageMagick. Again, @j4james has helped by explaining that the final graphics newline is actually harmful as it can cause the terminal to scroll on a fullscreen image. It can also be difficult for a program like lsix to tell whether a text newline is necessary or not. I believe ImageMagick and libsixel will eventually be updated to remove the final graphics newline. By doing it ourselves now, lsix is prepared for that transition and also avoids the question of extra space between rows of tiles when we send a text newline.

PerBothner commented 1 year ago

The latest DomTerm also implements the "correct" behavior. Hence, please add xterm-domterm to the These terminals are correct pattern.

Also, plain xterm now works, as of version 374.

j4james commented 1 year ago

Also, plain xterm now works, as of version 374.

Weird that there's no mention of that change in the log. But I've just tested now, and the cursor positioning is certainly improved. It's still not entirely correct, though, if you're expecting it to emulate a VT340. If the final sixel line spans two rows, the cursor ends up on the bottom row when it should be the top row.

It also has some weird bugs, like it can sometimes scroll early, even though the sixels haven't reached the bottom of the screen, and if you output a sixel image with no content, the text cursor actually moves up!

PerBothner commented 1 year ago

"Weird that there's no mention of that change in the log" It is mentioned: "change default for sixelScrolling resource to better match VT330/VT340 DECSDM setting (patch by Ben Wong)."

"It's still not entirely correct, though, if you're expecting it to emulate a VT340. If the final sixel line spans two rows, the cursor ends up on the bottom row when it should be the top row."

That seems broken. That means you would sometimes have emit two newlines to avoid following text overlapping.

j4james commented 1 year ago

change default for sixelScrolling resource to better match VT330/VT340 DECSDM setting

That's just a correction to the sixelScrolling resource, which had the opposite meaning of what it was intended to be. It has nothing to do with the cursor positioning.

That seems broken. That means you would sometimes have emit two newlines to avoid following text overlapping.

If you are trying to align images with text, you typically want your images to be an exact multiple of the text line height. To achieve that with sixel, the last sixel line will often extend beyond the final text row, with the extraneous pixels being transparent. In that scenario, you want the text to overlap those transparent pixels, so DEC's algorithm positions the cursor exactly where you need it to be. XTerm positions it too far down.

You can argue that there are situations where a different algorithm may be better, but the doesn't make DEC's algorithm broken. And the one massive advantage that the DEC algorithm has over anything else is that it hasn't changed in something like 36 years. XTerm is already on its fourth iteration (at least), and I doubt it will be the last.

dnkl commented 7 months ago

Foot also supports correct positioning, since 1.15.0.

The changelog mentions the behavior matching xterm's, but that was before I realized xterm doesn't handle last sixel spanning two rows correctly - foot does handle it correctly (as in, it implements the DEC algorithm).

dnkl commented 7 months ago
diff --git a/lsix b/lsix
index a621c57..f8150a0 100755
--- a/lsix
+++ b/lsix
@@ -122,7 +122,7 @@ autodetect() {

     # SIXEL CURSOR PLACEMENT BUG WORKAROUND
     case "$TERM" in
-   vt340*|contour*)
+   vt340*|contour*|foot*)
        # These terminals are correct. 
        sixelcursorbug=""   
        ;;