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

Neither TIOCGWINSZ nor 14t allow introspection of the window size in pixels #8581

Open
csdvrx opened this issue Dec 14, 2020 · 12 comments
Open
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Area-VT Virtual Terminal sequence support Issue-Scenario Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal.
Milestone

Comments

@csdvrx
Copy link

csdvrx commented Dec 14, 2020

Environment

Windows 10 Pro 20H2 (amd64)

Latest Windows Terminal Preview, running either msys2 or wsl

Steps to reproduce

compile and run the following code

#include <sys/ioctl.h>
#include <stdio.h>

int main (void)
{
    struct winsize ws;
    ioctl(1, TIOCGWINSZ, &ws);
    printf("rows: %d cols: %d xpixel: %d ypixel: %d\n", ws.ws_row, ws.ws_col, ws.ws_xpixel, ws.ws_ypixel);
}

Separately, send the xterm 14t sequence as documented in https://www.xfree86.org/current/ctlseqs.html :

 P s = 1 4 → Report xterm window in pixels as CSI 4 ; height ; width t

You can do that from a shell with:

echo -en "\033[14t\033\\"

Expected behavior

Both should work and return identical non 0 values

On mintty for example:

$ /d/wingetsize.exe
rows: 27 cols: 88 xpixel: 1056 ypixel: 729

This means the terminal is 729x1056

On wsltty running debian (WSL) for example:

$ echo -en "\033[14t\033\\"
^[[4;480;880t

This means the terminal is 880x480

Actual behavior

On a Windows Terminal running msys2:

$ /d/wingetsize.exe
rows: 25 cols: 98 xpixel: 0 ypixel: 0

The problem is not related to msys2 but purely Windows Terminal, as the above was obtained on a Windows Terminal running the exact same binaries and bash environment.

The absence of msys2 doesn't help: on a Windows Terminal running an official debian image (WSL) :

$ /mnt/d/wingetsize
rows: 21 cols: 84 xpixel: 0 ypixel: 0

To ignore any API issues, you can also try to send the specific xterm control sequence that should return the window size in pixels, and notice it returns nothing.

$ echo -en "\033[14t\033\\"

This properly returns the size on wsltty running debian-slim in WSL, so the problem is not related to WSL either:

wsltty-example

This issue may be related to #3718 (querying the colors via OSC) if you only consider the xterm control sequence. However the 0 values in TIOCGWINSZ is a tty ioctl, making this a wider problem, so I believe this is a separate issue.

At least one of these methods should work: introspection is required by some software to adjust their rendering to the actual space available on the terminal.

The absence of TIOCGWINSZ also impacts issue #448, as explained more precisely in #448 (comment) and illustrated in #448 (comment) : the screen size, along with the number of row and columns would allow inferring the font size, to avoid sixel images being stretched, squizzed, or a fraction of the correct size by sending them to the right size (in pixels) for their placeholder (in characters)

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Dec 14, 2020
@skyline75489
Copy link
Collaborator

Related: #5094

@DHowett
Copy link
Member

DHowett commented Dec 14, 2020

This is likely complicated by the fact that the Windows console subsystem does not and has never tried to support TTY IOCTLs. None of those requests ever reach Terminal because they’re shimmed out by whatever application compatibility layer you’re using to actually dispatch a TIOC... ioctl.

@csdvrx csdvrx closed this as completed Dec 15, 2020
@csdvrx csdvrx reopened this Dec 15, 2020
@csdvrx
Copy link
Author

csdvrx commented Dec 15, 2020

(oops, I accidentally hit Close instead of Comment, I'm not sure how to undo that, so I'm reopening the issue)

Related: #5094

@skyline75489 maybe exposing information to userland applications (as you must already keep track of the size of the window size somewhere :-) to let them adjust their behavior will be easier than controlling the terminal?

None of those requests ever reach Terminal because they’re shimmed out by whatever application compatibility layer you’re using

Then what about simply replying to OSC 14?

It's be nice to have TIOCGWINSZ , as several applications already use that (ex: tmux) but anything at all will be better than nothing :)

@skyline75489
Copy link
Collaborator

I think @j4james might have more insights on this one.

And It's CSI 14t, right? Not OSC.

@j4james
Copy link
Collaborator

j4james commented Dec 15, 2020

I don't think it's worth considering until we have a genuine use case for it. Right now we don't support Sixel, and if we did, our implementation would probably have to be independent of font size, so there's no value in reporting a window pixel size for that (other than maybe faking the values if that's necessary to make some apps work).

On top of that, it's not obvious to me what this sequence is even meant to report. I think it was originally defined as the window size, which some terminals interpret as the actual application window including chrome (i.e. title bar, borders, status bar, etc.), while others report some variation of the text area (although not necessarily the exact writeable area).

Then there's the issue of physical pixels vs density-independent pixels (i.e. on a high-dpi device with scaling). I would have thought DIP pixels made the most sense (otherwise how do you deal with a multi monitor setup with varying DPI), but most terminals I've been able to test seem to report physical pixels.

Another thing to bear in mind is that CSI t is also the DECSLPP sequence, so some terminals will simply interpret CSI 14 t as setting the screen height to 14 lines. That's admittedly rare - I think most that support DECSLPP only accept heights of 24 or more - but given the variability of the responses, even when supported, this doesn't seem like something you'd want to rely on.

It's also worth mentioning that TIOCGWINSZ has similar problems - in some cases it reports the windows size (i.e. including chrome), and some report the text area. In the few terminals I've found that supported both TIOCGWINSZ and CSI 14 t, most did not return the same values for them (although I haven't tested enough to suggest that that's typical behaviour).

@csdvrx
Copy link
Author

csdvrx commented Dec 16, 2020

And It's CSI 14t, right? Not OSC.

@skyline75489 my mistake!

BTW I'd love to try your Windows Terminal branch with sixel support, especially if advanced SGR sequences are also supported!

I don't think it's worth considering until we have a genuine use case for it.

@j4james, the use case is being compatible with... about every other terminal emulator.

Right now we don't support Sixel,

Well, eventually you may, and I already do, through tmux-sixel (to be released soon) which passes the sixel sequence "as-is" if the terminal supports them, and if not adapts that into a fallback rendition that works about everywhere.

Here is an example: tmux-sixel-fallback-161-40

You will recognize the current Windows Terminal Preview, straight from the store. It's showing a picture thanks to a conversion of the sixels in colored unicode blocks. Unfortunately, TIOCGWINSZ only works on remote hosts.

I need the size of the terminal in pixels to "maintains compatibility with existing implementations so images do not run off the edge of the screen or only fill half the screen" as mentioned by @OhMeadhbh in #448 (comment)

IMHO, that's at least 3 usecases: compatibility with xterm, proper sixel supoprt, sixel fallback mode support.

and if we did, our implementation would probably have to be independent of font size,

That's very wrong, for reason explained in #448 with several example - images will be stretched or will not fit with the rest of the TUI. Also, it will mess with the display as soon as the font size is altered - something as fast as a ctrl-+ to make the text more legible

so there's no value in reporting a window pixel size for that (other than maybe faking the values if that's necessary to make some apps work).

Well, maybe I'm wrong, but after reading this, it seems to me you want to play the clock on sixel support, waiting out until enough people complain with their "genuine use cases", after which you've already decided you'll do a shoddy job by reporting fake values - basically doing the bare minimum so that apps work poorly -- but hey, at least they may not crash thanks to the fake values!

It doesn't seem very ambitious, and it certainly doesn't inspire me trust in the Terminal future...

On top of that, it's not obvious to me what this sequence is even meant to report

Let's make an educated guess here: don't you think that, maybe, an application drawing pixels on a canvas is interested by the size of the actual canvas it's drawing on, and not by the window decorations?

If in doubt, just fire xterm, and do measurements, you'll see.

some terminals interpret as the actual application window including chrome (i.e. title bar, borders, status bar, etc.), while others report some variation of the text area (although not necessarily the exact writeable area).

Some terminals even report different sizes between TIOCGWINSZ and 14t, until the window is resized and then the values match, like mintty/mintty#1071

There's a name for that: a bug. I documented it in detail as I'm preparing a software demo, and making sure everything will go according to the script.

Another thing to bear in mind is that CSI t is also the DECSLPP sequence, so some terminals will simply interpret CSI 14 t as setting the screen height to 14 lines. That's admittedly rare - I think most that support DECSLPP only accept heights of 24 or more - but given the variability of the responses, even when supported, this doesn't seem like something you'd want to rely on

Terminals are 80x25, as you say yourself DECSLPP only accept heights of 24 or more. But even if it was >25, this is irrelevant: let's not forget that CSI 14t is send not by the terminal, but by the application to ask the terminal size, The only thing Windows Terminal has to do is to answer the query. I fail to see how answering the query may cause issues.

An application that needs to query the terminal may not have been run on a DEC terminal that did interpret 14t as a resize (!!!)

Still, if you believe answering the query from an app making use of xterm control sequences may cause issues, make it a flag: have by default the Terminal refuse do anything.

IMHO that's a bad default, but at least that's something I may be able to work with!

@DHowett
Copy link
Member

DHowett commented Dec 16, 2020

Well hold up.

after which you've already decided you'll do a shoddy job by reporting fake values [...] It doesn't seem very ambitious, and it certainly doesn't inspire me trust in the Terminal future...

You do not need to slander us into doing your preferred thing 😄

If in doubt, just fire xterm, and do measurements, you'll see.

Fortunately, the inimitable egmontkob did exactly this and found existing terminal emulators to be lacking.

"base" means that the reported numbers are the number of cells multiplied by each cell's size,
"extended" means it also includes the standard padding (border), the extra padding in case the
window size is not grid-aligned (e.g. maximized), and the scrollbar.

                \e[14t       ws_[xy]pixel
xterm            base(*)      extended
urxvt            extended     base
pterm (putty)    extended     base
st (suckless)    -            base
VTE              base         -
konsole          -            -
terminology      -            -

(*) Ctrl + right click -> Allow Window Ops

I especially "love" how xterm and urxvt are the exact opposite of each other :-)

(source (2017))

But even if it was >25, this is irrelevant: let's not forget that CSI 14t is send not by the terminal, but by the application to ask the terminal size

Right. I believe that's James' point: the application would also be the source of DECSLPP. The xterm control sequences documentation does call out the alternate treatment of DECSLPP and XTWINOPS.

@csdvrx
Copy link
Author

csdvrx commented Dec 16, 2020

Well hold up.

I don't want to assume wrongly, but I just don't understand at all the logic behind what was proposed in the answer. Seriously.

So I gave the only reasonable explanation that could connect all the dots, since the proposed choices seems contradictory with the stated goals of having a terminal that doesn't break compatibility.

after which you've already decided you'll do a shoddy job by reporting fake values [...] It doesn't seem very ambitious, and it certainly doesn't inspire me trust in the Terminal future...

You do not need to slander us into doing your preferred thing 😄

It's not about doing "my" preferred thing: fake values are in general a bad idea, here especially because it's ignoring the precedent set by what you are trying to replace, thus breaking backwards compatibility.

It's not about preferences or tastes: breaking backwards compatibility without good reasons is very wrong, and I'm sorry but laziness is not a valid reason 😄

If in doubt, just fire xterm, and do measurements, you'll see.

Fortunately, the inimitable egmontkob did exactly this and found existing terminal emulators to be lacking.

Actually, I'd say he found out exactly what I was saying: xterm returns the base through \e[14t.

I especially "love" how xterm and urxvt are the exact opposite of each other :-)

Let's be honest: Windows Terminal aims to replace the currently deployed terminals whenever possible, and besides not breaking compatibility whenever possible, that means putting a focus on the most widely used, not on the obscure ones.

Do you really think there're a lot of corporate deployments of st and urxvt? Especially when compared to xterm?

Personally, I don't think so, but if you REALLY want to cover all bases, add 2 flags:

  • one to plainly ignore 14t queries, which should be answered by default
  • one to return extended instead of base (even if I can't honestly see any reason to use the extended coordinates, and it seems to me to be a historical bug that became a quirk)

If you can "telemetry" that, I think you'll find <0.5% of people using Windows Terminal will use them

Right. I believe that's James' point: the application would also be the source of DECSLPP. The xterm control sequences documentation does call out the alternate treatment of DECSLPP and XTWINOPS.

So what you say is that you want to reserve the opportunity to answer DECSLPP requests in the future? Fair enough! More backwards compatibility is better!

Here's an idea:

  • 14t will mean 99.5% of the time "give me the coordinates", as terminals are 80x25, and 14<<<25 enough to be barely usable, and without a clear goal (setting to 2 lines? I could buy that, to have a command prompt somwhere. 25? I could buy that, to restore to normal. >25? Of course! But nothing makes 14 special)
  • so a safe default would be, resize with any other values besides 14, but with 14 answer the query by providing the size of the base window
  • have a flag to "remove" this exception for 14 if you are really worried about people with terminals having 14 lines
  • eventually have another flag to answer 14 by giving the extended coordinates if you are really worried about keeping compatibility with the anecdotal terminals like st and urxvt

@DHowett DHowett added Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. labels Dec 17, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Dec 17, 2020
@DHowett DHowett added this to the Console Backlog milestone Dec 17, 2020
@DHowett
Copy link
Member

DHowett commented Dec 17, 2020

@zadjii-msft If you agree, yank Triage. Conhost backlog, requires server work (we don't even have support for ioctls, and no windows console API allows for pixel sizing) and design. Did some prototyping on a new communication channel in the hax/l9 branch to see how easy it would be for us to extend the Windows API without shipping a new version of Windows. Impacts all three product areas (terminal, conhost, conpty) but I only marked the two.

@skyline75489
Copy link
Collaborator

@csdvrx I appreciate your interest in my work. However, my sixel branch should be considered very obsolete. The main blocker is the performance. It takes a huge amount of refactoring to implement streaming sixel data directly to the terminal and make it possible to fix the performance issue. Until then my branch is just a PoC.

If you’re still interested, @yatli actually got it up and running, I believe. He might had a better idea than I do.

@zadjii-msft
Copy link
Member

Sure yea, we can stick this on the backlog, with the caveat of I'm upgrading this to a scenario, because implementing an entirely new API layer through conpty to the Terminal is definitely not something I'd consider a "task" or even "deliverable" 😄

@zadjii-msft zadjii-msft added Issue-Scenario and removed Issue-Task It's a feature request, but it doesn't really need a major design. labels Dec 17, 2020
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Dec 17, 2020
@csdvrx
Copy link
Author

csdvrx commented Dec 19, 2020

implementing an entirely new API layer through conpty to the Terminal

@zadjii-msft Maybe I'm not correctly perceiving the magnitude of the problem, but without talking about TIOCGWINSZ, is it that difficult for the terminal to intercept the 14 sequence and reply with its own geometry? Or is it because you want to make a more generic solution?

The way I see it, the terminal must knows about the font it's using, and the geometry of the canvas it's flowing the text to, so it seems to be just a multiplication away.

@DHowett About the difference between xterm and urxvt, I've learned xterm now supports 14;2 which gives the dimension with the decorations included: mintty/mintty#1071 (comment)

@skyline75489 Yes, I'm very interested in sixel support for some very specific reasons, so thanks a lot for the suggestion, I'll follow up with yatli!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Area-VT Virtual Terminal sequence support Issue-Scenario Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

5 participants