bakkeby / st-flexipatch

An st build with preprocessor directives to decide which patches to include during build time
MIT License
347 stars 107 forks source link

Reworking sixel implementation based on veltza's implementation #117

Closed bakkeby closed 6 months ago

bakkeby commented 6 months ago

Raising pull request for sixel improvements (credits go to veltza and all his support).

Closes #111 Closes #102 Closes #30

step- commented 6 months ago

I didn't realize I wasn't subscribed to PR notifications, so I couldn't see 113-117 coming. Anyway, I just discovered that the overlap between an image and the text line above, which is shown in https://github.com/bakkeby/st-flexipatch/issues/102#issuecomment-1972642682, is due to enabling ANYSIZE_PATCH. The overlap amount depends on window size: print an image to st and resize the window vertically with your mouse up and down a few times to see overlap dynamics.

step- commented 6 months ago

Another thing I noticed is that inside tmux, when I print an image then drag the window border to resize, the image disappears for good. I'm talking about the regular tmux buffer, not the scroll-back one. Tested with only SIXEL_PATCH enabled, tmux 3.4 with no ~/.tmux directory and no ~/.tmux.conf. I see that @veltza's st-sx behaves the same way.

bakkeby commented 6 months ago

I didn't realize I wasn't subscribed to PR notifications, so I couldn't see 113-117 coming. Anyway, I just discovered that the overlap between an image and the text line above, which is shown in #102 (comment), is due to enabling ANYSIZE_PATCH. The overlap amount depends on window size: print an image to st and resize the window vertically with your mouse up and down a few times to see overlap dynamics.

I can imagine there being some sixel positioning that is not taking into account the vertical and horizontal padding that the anysize patch introduces. I understand the reasoning behind the patch, but the effect of centering the content within the frame makes for a rather nasty shaking / swimming effect when you resize the window. I'd recommend trying the ANYSIZE_SIMPLE_PATCH instead, which just lets the window to be resized freely by only changing the resize hints.

bakkeby commented 6 months ago

Another thing I noticed is that inside tmux, when I print an image then drag the window border to resize, the image disappears for good. I'm talking about the regular tmux buffer, not the scroll-back one. Tested with only SIXEL_PATCH enabled, tmux 3.4 with no ~/.tmux directory and no ~/.tmux.conf. I see that @veltza's st-sx behaves the same way.

Yes we discussed this in one of the earlier pull requests. We believe that this is due to tmux itself. May be a bug or it may simply be intentional. The level of sixel support in tmux is pretty limited according to their documentation. For what it is worth I tried this in xterm as well and the images disappear there too.

step- commented 6 months ago

Great. I just tried ANYSIZE_SIMPLE_PATCH and you're right, it's smoother. Also, image and text don't overlap anymore. Thank you also for confirming that images disappear after tmux resize.

veltza commented 6 months ago

I can confirm that sixels also disappear in WezTerm, so it's all about tmux.

This patch should fix the sixel positioning with ANYSIZE_PATCH:

diff --git a/x.c b/x.c
index 38cba1d..a7cefca 100644
--- a/x.c
+++ b/x.c
@@ -3110,8 +3110,13 @@ xfinishdraw(void)
        memset(&gcvalues, 0, sizeof(gcvalues));
        gcvalues.graphics_exposures = False;
        gc = XCreateGC(xw.dpy, xw.win, GCGraphicsExposures, &gcvalues);
+       #if ANYSIZE_PATCH
+       XCopyArea(xw.dpy, (Drawable)im->pixmap, xw.buf, gc, 0, 0,
+           width, height, win.hborderpx + im->x * win.cw, win.vborderpx + im->y * win.ch);
+       #else
        XCopyArea(xw.dpy, (Drawable)im->pixmap, xw.buf, gc, 0, 0,
            width, height, borderpx + im->x * win.cw, borderpx + im->y * win.ch);
+       #endif // ANYSIZE_PATCH
        XFreeGC(xw.dpy, gc);
    }
    #endif // SIXEL_PATCH
step- commented 6 months ago

This patch should fix the sixel positioning with ANYSIZE_PATCH

I can confirm that it fixes "image overlaps text line above" with ANYSIZE_PATCH. Thank you.