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

sixel inside tmux and ssh workaround #17815

Closed
galchinsky opened this issue Aug 28, 2024 · 5 comments
Closed

sixel inside tmux and ssh workaround #17815

galchinsky opened this issue Aug 28, 2024 · 5 comments
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conpty For console issues specifically related to conpty

Comments

@galchinsky
Copy link

Windows Terminal version

1.22.240823002-preview

Windows build number

10.0.22631.4037

Other Software

tmux next 3.5 (latest so far 34807388b064 commit built with --enable-sixel)

Steps to reproduce

In WSL2 tab img2sixel logo.png just works, hurray. It also works if I ssh to a remote server from the WSL2 tab.

Then I start tmux and run the same command. It outputs in a fallback mode

$ img2sixel logo.png
SIXEL IMAGE (12x6)
++++++++++++
++++++++++++
++++++++++++
++++++++++++
++++++++++++

Then I (similarly to a suggestion at wez/wezterm#1236)

  • go to a Windows "Command Prompt" tab
  • ssh to WSL2 from there
  • create tmux session and type "img2sixel logo192.png" .

sixel output starts to work, even on a remote server + tmux combination. This is weird, because if it wasn't so, I would think it's a tmux bug. But now it seems something is off with how WSL2 tab works.

Expected Behavior

sixel output inside tmux

Actual Behavior

tmux outputs

SIXEL IMAGE (12x6)
++++++++++++
++++++++++++
++++++++++++
++++++++++++
++++++++++++

but works correctly if ssh to WSL2 from Windows

@galchinsky galchinsky added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 28, 2024
@zadjii-msft
Copy link
Member

Huh. That's super bizarre. Our best guess right now is that it's the same thing as #17813, but we'll leave this open for now to make sure we check this too. Thanks for filing!

@zadjii-msft zadjii-msft added Product-Conpty For console issues specifically related to conpty Area-VT Virtual Terminal sequence support and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 28, 2024
@zadjii-msft zadjii-msft added this to the Terminal v1.23 milestone Aug 28, 2024
@j4james
Copy link
Collaborator

j4james commented Aug 28, 2024

As far I can recall, tmux relies on a TIOCGWINSZ ioctl query to calculate the terminal's cell size, and if it doesn't get back something meaningful, it disables the sixel functionality. I'm not particularly knowledgable about Linux, but my understanding is that it's the responsibility of the terminal to communicate its window size to the OS (also via an ioctl call), and the OS can then pass that on to any apps that query it.

Obviously that's not going to work on Windows because we aren't a Linux app. I'm assuming WSL generates the character dimensions itself by using one of the win32 console APIs to monitor the console buffer size, but it can't know the pixel size, so it leaves those values as 0x0. This is actually not that uncommon a situation, because the pixel fields are not really standard AFAIK, and not all terminals fill them in.

And when it comes to a remote connection to Linux, it's up to ssh to pass the window dimensions to the remote server, but the WSL version of ssh would itself depend on the local TIOCGWINSZ state, so it still won't be able to provide the pixel size.

So why does it work when you ssh from Windows to WSL? That's the bit that was a surprise to me, so I just checked, and it seems that the Windows version of ssh just fakes the pixel dimensions - they're hardcoded to 640x480 regardless of the terminal size. This should be enough to get tmux to work, but I would expect images to be misshapen unless the terminal was exactly 64 columns by 24 rows. I don't have a recent copy of tmux, though, so I haven't actually tested that.

As for fixing this, I don't think there's anything we can do about it. It could be fixed by tmux if they used one of the CSI t queries to determine the cell size (and that would enable them to support a number of other terminals too). It could also possibly be fixed in WSL if they faked the TIOCGWINSZ pixel dimensions (taking the char dimensions and multiplying them by 10x20). That wouldn't work for most third-party terminals though.

@galchinsky
Copy link
Author

galchinsky commented Aug 29, 2024

but I would expect images to be misshapen unless the terminal was exactly 64 columns by 24 row

That's true: if the window size is larger than ~24 rows, there is a big black area.

Also, if I fake xpixel and ypixel values in tmux/tty.c tty_resize function, sixel starts to work and it does query TIOCGWINSZ, so TIOCGWINSZ problem confirmed.

tmux-small-and-big-window

@galchinsky
Copy link
Author

Closing this as the patch suggested by @j4james in the last comment here was accepted to tmux (89adec0ca5)
I tested on 1.22.240823002-preview and the 25b1cc1e8e tmux build from github

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Sep 30, 2024
@j4james
Copy link
Collaborator

j4james commented Sep 30, 2024

@galchinsky Thank you for doing the work to get this fixed in tmux. It's much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

No branches or pull requests

4 participants
@galchinsky @j4james @zadjii-msft and others