Fixes found while reviewing #10471:
- A window squeezed to zero size (by maximize or an extreme bias) was
indistinguishable from a removed one, so closing an unrelated window
handed its share to windows in sub-trees that lost nothing. Track
survivors separately from the numeric weight.
- Pruning a batch of windows gave a different layout than closing them
one by one, since a perpendicular sub-tree stops being perpendicular
once removals collapse it. Apply the policy one window at a time.
- Windows reconciled by do_layout(), rather than split off an existing
window, bypassed the policy entirely. Route them through it too.
Also fix two pre-existing bugs on the same code paths:
- collapse_redundant_pairs() collapsed top down, so a pair whose
children only became empty during the recursion was never
re-examined and was left in the tree, consuming its share of the
space.
- do_layout() kept using a stale pairs root when removing windows
collapsed the tree, so windows added in the same relayout were
attached to the discarded root and vanished for a frame.
Completion patterns are lowercased in make_completer and file names are
lowercased before being matched against them, but the names of files inside
a directory were passed to the matcher unchanged. A directory whose only
matching file was named something like B.CONF was therefore never offered as
a completion candidate for a *.conf pattern, while the same file named
b.conf worked.
Port the fix from 2093fb131 in the C shlex implementation to the Go one.
Previously an argument such as "" or '' was silently dropped, since an
empty word was also used to signal EOF. So "launch --title '' vim" in the
kitty shell was parsed as "launch --title vim". EOF is now signalled with
a negative Pos.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MA8JGs7pCoXFL2W6jEJ8sD
SplitForCompletion computed the position of the new empty word as if
the previous word were one byte long, so tab completing after a space
following a multi-character word replaced part of that word. For example,
completing "launch --type " in the kitten @ shell produced
"launch --window ". The new word starts at the end of the input.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MA8JGs7pCoXFL2W6jEJ8sD
I've been running with the shaped run cache and some extra logging,
decided to bump to a 4KiB slab. The theoretical and actual waste do show
up during use. My initial sizing was only a best guess based of glyph
cache and a 32 cell run typically fitting just below a 2KiB slab.
The running byte counter summed the sizes requested from the arenas.
The arenas serve allocations from fixed size blocks and strand the tail
of a block whenever the next allocation does not fit, so the counter
understated real usage. For runs of 17 to 20 columns, where the value
allocation lands just over half a block, only one value fits per block
and the cache grew to ~2MB against a 1MB cap. As the limit applies per
Font, that overshoot is multiplied across every font in a FontGroup.
Track bytes actually malloced in the arena itself instead, so the cap
check stays O(1), which was the point of the counter, while measuring
the same thing the walking count used to.
The memset was run unconditionally after posix_memalign, so an
allocation failure crashed in memset instead of returning NULL to the
caller, defeating the OOM handling in every arena user.
Another small tweak to the shaped run cache, keep a running byte
counter to avoid the walking count on cache put. Although the walking
count was noise next to the harfbuzz shaping, this is much better.
- Build the cache key once per run rather than once in shaped_run_get()
and again in shaped_run_put(), which had to walk the cells and query
the text cache a second time on every cache miss
- Do not zero the entire groups array on a cache hit, the groups that are
actually read are all overwritten by the cached ones
- Drop the cache when an arena allocation or the insert fails, instead of
leaking the space allocated for the entry, since monotonic arenas
cannot free individual allocations
fill_key() sizes its scratch buffer assuming every cell holds at most
MAX_NUM_CODEPOINTS_PER_CELL codepoints. The draw paths in screen.c
enforce that, but Line.add_combining_char() does not, so a line built via
the Python API with more codepoints in a cell overflows the key buffer
once the run is long enough. Bail out of caching such runs instead.
Wire the shaper-cache to actually be used for shaped runs.
On a 10s loop of the same short-word screen (~16ms/frame), main-thread
CPU dropped ~28% vs master and hb_shape samples went to 0.
Parse speed remains unchanged (and fast), verified with kitten __benchmark__.
Per-font verstable map of exact run keys to glyph infos, positions and
cell groups. Skip runs longer than 32 cells. Full-drop at 2048 entries
or 1 MiB of live storage.
The intention here is to avoid shaping the same runs over and over again
on an active screen. This used to not have much of an impact, but now
"modern" full screen TUIs are actively drawing the screens in such a way
that harfbuzz shaping is showing up as a more significant portion in
profiling runs.
The cache is sized to not be a long-lived history but able to cache most
repeated runs on a few active screens.
Follow-ups to the drag_or_normal_select selected text drag support:
- Reset the pending drag at the top of handle_button_event again, before
the scrollbar, missing screen and mouse position early returns. Moving
it into a helper placed it after those returns, so a scrollbar track
click (with scrollbar_jump_on_click disabled) no longer cleared an
armed URL drag, letting it fire on a later mouse move with no button
held. Split the helper in two, as arming the drag needs an up to date
mouse position but clearing it does not.
- Clear the click queue for all drag kinds, not just text drags. A native
drag consumes mouse-up, so the press lingered and turned the next press
into a double press, selecting a word instead of starting a new
gesture. Reproducible with link drags before this change.
- Note that handle_potential_drag can call into Python, which can
re-alloc the windows array, so w must not be used after it returns.
Also share the drag thumbnail construction between drag_url and
drag_selection (and skip the thumbnail rather than dividing by zero on a
zero width render), use None instead of -1 for the mock event pixel
position so negative positions remain testable, and tighten the
off-by-one bound in clear_click_queue.