-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
feat(term): report terminal size in pixels #32408
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
Conversation
6cee275
to
b28f1a6
Compare
@@ -37,6 +37,8 @@ typedef struct { | |||
bool ui_ext[kUIExtCount]; ///< Externalized UI capabilities. | |||
int width; | |||
int height; | |||
int pixel_width; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are changed to floating points, they would be more generally useable for other purposes. For example, to calculate the pixel coordinate of a given cell. Neovim, could internally round this to integers where needed.
Neovide, for example use a fractional grid cell size, so rounding dividing the pixel_width / width
would give a pretty inaccurate cell width in some cases,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be willing to change this, but can you maybe briefly more clearly explain why? Am I getting this right, that neovide uses non-pixel aligned cells in order to fill out the entire window if for example pixel_width % width != 0
? Couldn't UIs just as well cast the values to float before doing any divisions if needed? Currently, in my mind, the total amount of pixels should always be an integer amount and it should be up to the users of these values to handle division inaccuracies, but I am willing to be convinced otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's say my grid width is 10.1 pixels, and the width is 81 columns, so the real with is 818.1. But the UI passes 819.
Now, we want to display something at column 80. Neovim has to calculate the grid width as 819 / 81 = 10.11111, with the information given. Column 80 is 80 * 10.11111 = 808.88889, so you round and display it at pixel 809. While the correct position should be 80 * 10.1 = 808, so it's displayed one pixel wrong.
It might not matter for the intended use case, but the information could potentially be used for the image protocol for example, and things could become unaligned.
BTW, I'm quite sure that many terminals use fractional grid sizes as well, not just Neovide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see, thank you for the clarification. I'll change it, but I am waiting on some clarification in #31903 wrt to what actual types to use.
I'm guessing in terminal buffers we should then always report the floor of the value to prevent applications from overdrawing. Unfortunately ioctl
only takes shorts for TIOCSWINSZ
.
cef5249
to
ebe9846
Compare
I have now updated this to only require one API change: Also, I still need to switch everything over to Also, I pulled out the terminal code for now, there's enough changes to be reviewed as is, and I think that stuff might be better off in its own PR, or maybe as a last commit once the internals are working. |
Let's put this on hold for a while. The new image protocol design #34093 does not require the exact size to be exposed. Some approximation like a 2:1 ratio is probably good enough when determining how many rows should be allocated for inline markdown images for example. The actual provider will always render with the correct aspect ratio anyway. There are also some unanswered questions and problems that I found when taking a closer look.
I think the above is something that needs to be resolved, but we also need to figure out the actual practical use cases. It might be that it's only needed when you know that you are targeting a specific GUI, and in that case, the GUI could provide some API by itself. Or maybe the plugin that needs this information could some hooks for the UI to hook into, providing more targeted information. In both cases we need generic information from the TUI though, that's an easier case since only one font size is supported. Until the Kitty variable font size protocol is implemented... |
This has been closed since a request for information has not been answered for 30 days. It can be reopened when the requested information is provided. |
Terminal pixel size is currently reported as 0, which prevents some programs from working in the built-in terminal, notably programs displaying images (see #32189).
To be able to report terminal pixel size correctly we need to track it alongside the terminal size in cells. In order to do that UIs need to keep track and report their pixel size, which means changes to the API of
nvim_ui_attach
andnvim_ui_try_resize
. That means this PR depends on #31903 to be resolved.Currently, this PR implements a proof of concept. Terminal pixel size reporting works as is and reports the correct total size of the TUI (or the smallest values of multiple TUIs in the
--headless
and more general--remote-ui
case).The following things need to be done to consider the pixel size reporting useable:
nvim_list_uis
report pixel sizesThe following needs to be done for builtin term ioctl reporting:
The following things need to be done before this PR is in a mergeable state: