Repository navigation
Size cells from the buffer's font, not the frame's - #590
Merged
Merged
Conversation
`ghostel--reported-cell-width' and `ghostel--cell-height' — the cell pixel dimensions handed to libghostty and reported to programs through XTWINOPS (CSI 14/16 t) — were built from `frame-char-width' and `frame-char-height'. Both are frame-level and blind to a buffer-local `default' face remapping, so any local font change (`text-scale-mode', `buffer-face-mode', a `ghostel-default' `:height') left the reported cell size at the frame's while the row/column count, which comes from the remap-aware `window-screen-lines' and `window-max-chars-per-line', tracked the new font. A terminal scaled to text-scale 6 kept reporting an 11x22 px cell for a grid whose cells were 35x65. The kitty graphics paths had the same base: both sized the image and its per-row slices off the frame's cell, so a placement that asked for 4x2 cells drew at the unscaled pixel size inside a scaled grid. Use `default-font-width' / `default-font-height', which resolve through `face-remapping-alist'. The float `line-spacing' branch keeps multiplying by `frame-char-height': redisplay scales a float by FRAME_LINE_HEIGHT (xdisp.c), so the frame's char height is the right base there. Slices keep using the font height rather than `default-line-height' — buffer `line-spacing' is applied per glyph, so a spacing-inclusive slice would double-pad image rows. Cost is not a concern on the per-placement kitty path: the pair costs ~11 us even when the default face is remapped and `font-info' runs. Verified in a GUI Emacs driving a live bash, with a Python helper in the shell reading back the terminal's own CSI 16 t / CSI 14 t replies. At the default font: cell 11x22, area 1320x748 for a 34x120 grid. After `text-scale-set 6' (font 22x41, frame char dims unchanged at 7x14): cell 35x65, area 1330x715 for 11x38, and a 4x2-cell kitty placement grew from 28x28 px with 14 px slices to 88x82 px with 41 px slices. `buffer-face-set (:height 240)': cell 22x44, area 1320x748 for 17x60. With `line-spacing' 4 the reported cell height picks up the spacing while the slices stay at the font height.
`ghostel-compile--start' reconciles the VT to the output window after `display-buffer' may have placed the buffer in a window other than the one `--prepare-buffer' measured. That call passed only rows and cols. The native binding defaults omitted cell dimensions to 1 px, so the resize overwrote the dimensions `ghostel--init-buffer' had just seeded and libghostty ran the rest of the compilation with a 1x1 px cell: CSI 14/16 t reported a 1 px cell to the command, and kitty graphics placements derived their grid geometry from it. Nothing repaired it afterwards. `ghostel--adjust-size' only re-sends dimensions when the row/column count changes, and the reconcile had just made those match the output window, so the 1x1 cell survived until the user resized the window. Route the reconcile through `ghostel--set-size-with-cell-dims' like the other resize sites. The compile buffer is already current there, which is what the wrapper needs to resolve the dimensions. The wrapper's docstring claimed five resize sites; there are two. Verified in a GUI Emacs: `M-x ghostel-compile' running a probe that queries CSI 16 t on /dev/tty reports a 11x22 px cell, matching `ghostel--reported-cell-width'/`-height' for the buffer's font, where dropping the dimensions again reports 1x1. `ghostel-test-compile-reconciles-vt-size-to-outwin' had been masking this: it stubbed `ghostel--set-size-with-cell-dims' away and recorded only the three-argument `ghostel--set-size'. Both reconcile tests now let the wrapper run and assert at the native boundary — one that the reconcile carries the cell dimensions and still precedes the header render and the spawn, the other that a missing output window leaves just the `--prepare-buffer' seed.
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #589. That PR made
buffer-face-moderescale the terminal grid; the cell pixel dimensions ghostel reports were still computed from the frame.Cells were measured off the wrong thing
ghostel--reported-cell-widthandghostel--cell-height— the dimensions handed to libghostty and reported to programs through XTWINOPS (CSI 14 t/CSI 16 t) — usedframe-char-width/frame-char-height. Both are frame-level and ignore a buffer-localdefaultface remapping, while the row/column count comes from the remap-awarewindow-screen-linesandwindow-max-chars-per-line. Any local font change (text-scale-mode,buffer-face-mode, aghostel-default:height) therefore left the reported cell at the frame's size while the grid tracked the new font.The kitty graphics paths shared the base, so a placement asking for 4×2 cells was sized for the frame's cell and drew at the wrong scale inside a scaled grid.
Both now use
default-font-width/default-font-height, which resolve throughface-remapping-alist.Two things deliberately keep the frame metric:
line-spacingbranch, because redisplay scales a float byFRAME_LINE_HEIGHT(xdisp.c), matchingdefault-line-height;default-line-height— bufferline-spacingis applied per glyph, so a spacing-inclusive slice would double-pad image rows.Measured in a real Emacs before adding any caching: the
default-font-width+default-font-heightpair costs ~11 µs even on thefont-infopath, against ~0.06 µs forframe-char-*. Not worth caching on the per-placement kitty path.Verified live
GUI Emacs 31.0.91 driving a real bash, with a Python helper in the shell reading back the terminal's own
CSI 16 t/CSI 14 treplies.frame-char-*stayed 7×14 throughout.:width 28 :height 28,(slice 0 0 28 14)text-scale-set 6:width 88 :height 82,(slice 0 0 88 41)buffer-face-set '(:height 240)line-spacing4Every text-area figure equals rows × cell-height and cols × cell-width. Before the change all four rows reported 22;11 and sized the image at 28×28, so at text-scale 6 an image drew at about a third of its declared cell footprint.
A compile terminal reset its cell to 1×1 px
Found while reviewing the above; it is an independent pre-existing bug, in the second commit.
ghostel-compile--startreconciles the VT to the output window afterdisplay-buffermay have placed the buffer somewhere other than the window--prepare-buffermeasured. It calledghostel--set-sizewith only rows and cols, and the native binding defaults omitted cell dimensions to 1 px — wiping the dimensionsghostel--init-bufferhad just seeded.ghostel--adjust-sizenever repaired it, because it only re-sends when the row/column count changes and the reconcile had just made those match. So a compile terminal reported a 1 px cell for the whole run.The reconcile now goes through
ghostel--set-size-with-cell-dimslike the other resize sites. Verified live:M-x ghostel-compilerunning a probe that queriesCSI 16 ton/dev/ttyreports6;22;11, where dropping the dimensions again reports6;1;1.Tests
Stubs in the kitty fixture and the cell-dimension tests now make the frame dimensions differ from the font dimensions, so a revert to
frame-char-*fails rather than passing on coincidence. New:ghostel-test-kitty-display-image-sized-from-buffer-font.ghostel-test-compile-reconciles-vt-size-to-outwinhad been masking the compile bug — it stubbedghostel--set-size-with-cell-dimsaway and recorded only the three-argumentghostel--set-size. Both reconcile tests now let the wrapper run and assert at the native boundary.make -j8 allpasses from a clean.build/tests.Not addressed here
The float
line-spacingbranch rounds wherexdisp.canddefault-line-heighttruncate — up to 1 px of over-report at e.g.line-spacing0.25 with a 14 px font. Predates this work; only the base term changed.