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

Preview Pane is not cleared on Tmux but fine on foot #241

Closed
niksingh710 opened this issue Jan 13, 2025 · 11 comments
Closed

Preview Pane is not cleared on Tmux but fine on foot #241

niksingh710 opened this issue Jan 13, 2025 · 11 comments
Labels
sep Somebody else's problem

Comments

@niksingh710
Copy link

niksingh710 commented Jan 13, 2025

NOTE: Maybe it is not related to chafa at all but putting it here if any implementation can fix this.

So i just created a small script to preview images and text based on filetype in fzf-preview.

output

chafa works fine on tmux for me, chafa -f sixels <image.png> shows the image in the terminal and tmux window.

Note

The glitch does not happen on foot terminal but only occurs in the tmux window

For reference preview script

@hpjansson
Copy link
Owner

Hmm. I suspect this is a tmux issue. Can you try this:

echo -en '\033[0H' ; chafa -f sixel -s 40 image.png ; echo -en '\033[0H' ; chafa -f sixel -s 20 image.png

Inside plain foot, and inside tmux running under foot. Here it produces a small image overlaid on a big image in foot, but only the small image in tmux. Foot's behavior is the correct one.

However, this is the opposite of what you're seeing in fzf-preview, so maybe it's caused by something else.

@niksingh710
Copy link
Author

Image

Yes, I think you are right. This is happening because of something else. However, I am unable to pinpoint what might be causing this. I have tried all tmux variables and related configurations.

@hpjansson
Copy link
Owner

Let's try to rule out the easy stuff - which Chafa version are you running (chafa --version), and which tmux version (tmux -V)?

Older versions of tmux require the use of passthrough, and Chafa from the master branch is better at detecting when it'll work without passthrough. This gives better results. tmux with passthrough tends to dispose of images immediately like you're seeing in the test above (left hand side).

If you're using a 1.14.x release of Chafa, do you think you'd be able to test with the master branch instead?

@niksingh710
Copy link
Author

Yes, you are right.
My bad—I should have already included this general information.

chafa --version

󰘧 chafa --version
Chafa version 1.14.5

Loaders:  GIF JPEG PNG QOI TIFF WebP XWD
Features: mmx sse4.1 popcnt avx2
Applying: mmx sse4.1 popcnt avx2

Copyright (C) 2018-2024 Hans Petter Jansson et al.
Incl. libnsgif copyright (C) 2004 Richard Wilson, copyright (C) 2008 Sean Fox
Incl. LodePNG copyright (C) 2005-2018 Lode Vandevenne
Incl. QOI decoder copyright (C) 2021 Dominic Szablewski

This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

tmux -V: 3.5a

I successfully built chafa from the master branch. Specifically, the commit I built was 05e7609.

Below is the package override I used:

pkgs.chafa.overrideAttrs (oldAttrs: rec {

  version = "05e7609";
  pname = "chafa";

  src = pkgs.fetchFromGitHub {
    owner = "hpjansson";
    repo = "chafa";
    rev = version;
    sha256 = "sha256-bU/Wp3e0/x+4cYolKBrilrRzFL93hJW8l7xJguiRpIs=";
  };

});

chafa --version

Chafa version 1.15.0

Loaders:  GIF JPEG PNG QOI TIFF WebP XWD
Features: mmx sse4.1 popcnt avx2
Applying: mmx sse4.1 popcnt avx2

Copyright (C) 2018-2024 Hans Petter Jansson et al.
Incl. libnsgif copyright (C) 2004 Richard Wilson, copyright (C) 2008 Sean Fox
Incl. LodePNG copyright (C) 2005-2018 Lode Vandevenne
Incl. QOI decoder copyright (C) 2021 Dominic Szablewski

This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

The behavior persists in the master branch as well.

@hpjansson
Copy link
Owner

hpjansson commented Jan 19, 2025

Thanks, that's useful to know. The next thing to try is to run it in as basic an environment as possible -- e.g. bog standard bash shell and no customizations. A simple way to do it would be to create a new user and try it there.

Edit: You might get different results in a different terminal with sixel or kitty capabilities. I'd check with mlterm, kitty or ghostty too, to see if anything changes.

@niksingh710
Copy link
Author

Okay, So i tried it with clean env also.
Had my tmux config and fzf with the script to preview via using chafa and bat for text.

foot/mlterm -> Preview worked perfectly Fine
foot/mlterm + tmux -> The same glitch happens

kitty -> Gives some alphanumeric values on preview window
ghotty -> Blank in Preview Window

kitty/ghostty + tmux

Image Image

@hpjansson
Copy link
Owner

Good, so we know foot and mlterm produce the same results.

The kitty alnum spew sounds like fzf could be interfering with kitty images. The tmux +++++++++++ placeholder happens when we send tmux a sixel image, but it thinks the terminal is unable to display it - so that could be a tmux issue, or an issue with the terminfo database.

As for the failure-to-clear-sixels glitch, I can't think of anything else to test at the moment - I'll have to find some time to investigate the foot/tmux/fzf/fzf-preview chain locally. Probably next week.

@hpjansson hpjansson added bug Not working as advertised compatibility Compatibility (e.g. terminal quirks) labels Jan 24, 2025
@niksingh710
Copy link
Author

@hpjansson Any updates on the investigation? Were you able to determine the cause of this?

@hpjansson
Copy link
Owner

Yes - it's a tmux bug. I'm attaching a patch that's a WIP (may be buggy), but it works for me when applied to tmux master.

The issue is that screen_write_cell() will skip cell updates when the cell's character/attribute contents didn't change, without considering that there may be part of an image covering the cell. So when fzf tries to clear the image, tmux sees spaces being overwritten by spaces, and does nothing.

Fake edit: GitHub is failing diff uploads for no readily apparent reason, so I'm just dumping it inline. Please try it out and let me know how it goes.

diff --git a/image.c b/image.c
index 43808785..0c030645 100644
--- a/image.c
+++ b/image.c
@@ -46,8 +46,11 @@ image_free_all(struct screen *s)
 	struct image	*im, *im1;
 	int		 redraw = !TAILQ_EMPTY(&s->images);
 
-	TAILQ_FOREACH_SAFE(im, &s->images, entry, im1)
+	TAILQ_FOREACH_SAFE(im, &s->images, entry, im1) {
+		log_debug("%s: image %p retired", __func__, im);
 		image_free(im);
+	}
+
 	return (redraw);
 }
 
@@ -124,7 +127,10 @@ image_check_line(struct screen *s, u_int py, u_int ny)
 	int		 redraw = 0;
 
 	TAILQ_FOREACH_SAFE(im, &s->images, entry, im1) {
+		log_debug ("%s: image %p (%d,%d %dx%d) isect with (*,%d *x%d)?",
+			   __func__, im, im->px, im->py, im->sx, im->sy, py, ny);
 		if (py + ny > im->py && py < im->py + im->sy) {
+			log_debug("%s: image %p retired", __func__, im);
 			image_free(im);
 			redraw = 1;
 		}
@@ -139,16 +145,42 @@ image_check_area(struct screen *s, u_int px, u_int py, u_int nx, u_int ny)
 	int		 redraw = 0;
 
 	TAILQ_FOREACH_SAFE(im, &s->images, entry, im1) {
+		log_debug ("%s: image %p (%d,%d %dx%d) isect with (%d,%d %dx%d)?",
+			   __func__, im, im->px, im->py, im->sx, im->sy,
+			   px, py, nx, ny);
+
 		if (py + ny <= im->py || py >= im->py + im->sy)
 			continue;
 		if (px + nx <= im->px || px >= im->px + im->sx)
 			continue;
+		log_debug("%s: image %p retired", __func__, im);
 		image_free(im);
 		redraw = 1;
 	}
 	return (redraw);
 }
 
+int
+image_check_area_persist(struct screen *s, u_int px, u_int py, u_int nx, u_int ny)
+{
+	struct image	*im, *im1;
+	int		 isect = 0;
+
+	TAILQ_FOREACH_SAFE(im, &s->images, entry, im1) {
+		log_debug ("%s: image %p (%d,%d %dx%d) isect with (%d,%d %dx%d)?",
+			   __func__, im, im->px, im->py, im->sx, im->sy,
+			   px, py, nx, ny);
+
+		if (py + ny <= im->py || py >= im->py + im->sy)
+			continue;
+		if (px + nx <= im->px || px >= im->px + im->sx)
+			continue;
+		log_debug("%s: image %p intersected", __func__, im);
+		isect = 1;
+	}
+	return (isect);
+}
+
 int
 image_scroll_up(struct screen *s, u_int lines)
 {
@@ -164,6 +196,7 @@ image_scroll_up(struct screen *s, u_int lines)
 			continue;
 		}
 		if (im->py + im->sy <= lines) {
+			log_debug("%s: image %p retired", __func__, im);
 			image_free(im);
 			redraw = 1;
 			continue;
diff --git a/screen-write.c b/screen-write.c
index bce56b8e..614bb127 100644
--- a/screen-write.c
+++ b/screen-write.c
@@ -2022,6 +2022,8 @@ screen_write_cell(struct screen_write_ctx *ctx, const struct grid_cell *gc)
 	if (skip) {
 		if (s->cx >= gl->cellsize)
 			skip = grid_cells_equal(gc, &grid_default_cell);
+		else if (image_check_area_persist(s, s->cx, s->cy, width, 1))
+			skip = 0;
 		else {
 			gce = &gl->celldata[s->cx];
 			if (gce->flags & GRID_FLAG_EXTENDED)
diff --git a/tmux.h b/tmux.h
index d448faa3..ae4125d2 100644
--- a/tmux.h
+++ b/tmux.h
@@ -3561,6 +3561,7 @@ int		 image_free_all(struct screen *);
 struct image	*image_store(struct screen *, struct sixel_image *);
 int		 image_check_line(struct screen *, u_int, u_int);
 int		 image_check_area(struct screen *, u_int, u_int, u_int, u_int);
+int		 image_check_area_persist(struct screen *, u_int, u_int, u_int, u_int);
 int		 image_scroll_up(struct screen *, u_int);
 
 /* image-sixel.c */

@hpjansson hpjansson added the sep Somebody else's problem label Feb 9, 2025
@niksingh710
Copy link
Author

@hpjansson just tested the patch and it is working perfectly fine for me.

@hpjansson
Copy link
Owner

Unfortunately, tmux wants a bigger overhaul that'll take a lot more work to produce. Since this is not our bug, I'll close this, but I'll post an update here if/when tmux gets fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sep Somebody else's problem
Projects
None yet
Development

No branches or pull requests

2 participants