CERBERUS/kitty
1
0
Fork 0
mirror of https://github.com/kovidgoyal/kitty.git synced 2026-10-21 12:42:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Splits: Fix issues with proportional sizing

Browse source 
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.
This commit is contained in:
Kovid Goyal 2026-09-16 09:30:25 +05:30
parent 791614ce93
commit 65b02b21f4
No known key found for this signature in database
GPG key ID: 06BC317B515ACE7C
2 changed files with 149 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

107
kitty/layout/splits.py
View file

@ -128,6 +128,7 @@ class Pair:
return None
def window_weights(self) -> dict[int, float]:
"Map every window in this sub-tree to its fraction of this pair, obtained by multiplying the biases along the path to it"
weights: dict[int, float] = {}
def walk(child: Pair | int | None, weight: float) -> None:
@ -144,6 +145,8 @@ class Pair:
return weights
def set_window_weights(self, weights: dict[int, float]) -> None:
"The inverse of :meth:`window_weights`: set the biases in this sub-tree so the windows end up with the specified relative sizes"
def tune(child: Pair | int | None) -> float:
if isinstance(child, Pair):
one, two = tune(child.one), tune(child.two)
@ -155,22 +158,37 @@ class Pair:
tune(self)
def preserve_weights_on_removal(self, removed: Collection[int]) -> bool:
"""
Adjust the biases in this sub-tree so that the windows surviving the removal of
``removed`` keep their current sizes relative to each other. Returns True if this
sub-tree still contains at least one surviving window.
"""
# Each perpendicular subtree is one unit along this split axis. If
# some of its windows survive, retain its full share on this axis.
def tune(child: Pair | int | None, weight: float) -> float:
def tune(child: Pair | int | None, weight: float) -> tuple[float, bool]:
if isinstance(child, Pair):
if child.horizontal != self.horizontal:
return weight if child.preserve_weights_on_removal(removed) else 0.0
alive = child.preserve_weights_on_removal(removed)
return (weight, True) if alive else (0.0, False)
if child.is_redundant:
return tune(child.one or child.two, weight)
one = tune(child.one, weight * child.bias)
two = tune(child.two, weight * (1 - child.bias))
if one > 0 and two > 0:
child.bias = one / (one + two)
return one + two
return weight if child is not None and child not in removed else 0.0
one, one_alive = tune(child.one, weight * child.bias)
two, two_alive = tune(child.two, weight * (1 - child.bias))
if not one_alive and not two_alive:
return 0.0, False
total = one + two
if one_alive and two_alive and total > 0:
child.bias = one / total
# A window squeezed to zero size (by maximize or an extreme bias) has zero
# weight. Falling back to the incoming share keeps such survivors from
# having their space handed to unrelated windows.
return (total if total > 0 else weight), True
if child is None or child in removed:
return 0.0, False
return weight, True
return tune(self, 1.0) > 0
return tune(self, 1.0)[1]
def remove_windows(self, window_ids: Collection[int]) -> None:
if isinstance(self.one, int) and self.one in window_ids:
@ -185,14 +203,19 @@ class Pair:
return self.one is None or self.two is None
def collapse_redundant_pairs(self) -> None:
while isinstance(self.one, Pair) and self.one.is_redundant:
self.one = self.one.one or self.one.two
while isinstance(self.two, Pair) and self.two.is_redundant:
self.two = self.two.one or self.two.two
# Collapse depth first, otherwise a child that only becomes redundant
# because all of its own descendants were removed is never re-examined
# and is left in the tree, consuming its share of the available space.
if isinstance(self.one, Pair):
self.one.collapse_redundant_pairs()
if isinstance(self.two, Pair):
self.two.collapse_redundant_pairs()
while isinstance(self.one, Pair) and self.one.is_redundant:
self.one = self.one.one or self.one.two
while isinstance(self.two, Pair) and self.two.is_redundant:
self.two = self.two.one or self.two.two
if self.one is None and self.two is not None:
self.one, self.two = self.two, None
def balanced_add(self, window_id: int) -> 'Pair':
if self.one is None or self.two is None:
@ -635,6 +658,16 @@ class Splits(Layout):
self._pairs_root = root
def remove_windows(self, *windows_to_remove: int) -> None:
if self.layout_opts.proportional and len(windows_to_remove) > 1:
# Apply the sizing policy one window at a time, so that pruning a
# batch of windows (which happens when they are closed while a
# different layout is active) gives the same result as closing them
# one by one. The policy is not idempotent over a batch because a
# perpendicular sub-tree stops being perpendicular once removals
# collapse it down to a single child.
for window_id in windows_to_remove:
self.remove_windows(window_id)
return
root = self.pairs_root
if self.layout_opts.proportional:
root.preserve_weights_on_removal(frozenset(windows_to_remove))
@ -647,15 +680,18 @@ class Splits(Layout):
self.pairs_root = q
def proportional_container(self, group_id: int, horizontal: bool) -> Pair | None:
root = self.pairs_root
pair = root.pair_for_window(group_id)
if pair is None or pair.horizontal != horizontal:
"The outermost pair that splits along ``horizontal`` and has the window as one of its leaves"
path = self.pairs_root.find_window_in_tree(group_id)
if not path or path[-1][0].horizontal != horizontal:
return None
while (parent := pair.parent(root)) is not None and parent.horizontal == horizontal:
pair = parent
return pair
container = path[-1][0]
for pair, _ in reversed(path[:-1]):
if pair.horizontal != horizontal:
break
container = pair
return container
def split_and_add(self, pair: Pair, group_id: int, new_id: int, horizontal: bool, after: bool, bias: float | None = None) -> None:
def split_and_add_window(self, pair: Pair, group_id: int, new_id: int, horizontal: bool, after: bool, bias: float | None = None) -> None:
container = self.proportional_container(group_id, horizontal) if self.layout_opts.proportional and bias is None else None
weights = container.window_weights() if container is not None else {}
parent = pair.split_and_add(group_id, new_id, horizontal, after)
@ -665,6 +701,25 @@ class Splits(Layout):
weights[new_id] = weight
container.set_window_weights(weights)
def balanced_add_window(self, new_id: int) -> Pair:
"Add a window at a balanced position, applying the proportional sizing policy, if enabled"
root = self.pairs_root
if not self.layout_opts.proportional:
return root.balanced_add(new_id)
weights = root.window_weights()
pair = root.balanced_add(new_id)
# There is no window being split here, so the new window takes the share
# of whatever it ended up being paired with.
sibling = pair.two if pair.one == new_id else pair.one
if isinstance(sibling, Pair):
weight = sum(weights.get(wid, 0.0) for wid in sibling.all_window_ids())
else:
weight = 0.0 if sibling is None else weights.get(sibling, 0.0)
if weight > 0 and (container := self.proportional_container(new_id, pair.horizontal)) is not None:
weights[new_id] = weight
container.set_window_weights(weights)
return pair
def do_layout(self, windows: WindowList) -> None:
groups = tuple(windows.iter_all_layoutable_groups())
root = self.pairs_root
@ -672,10 +727,12 @@ class Splits(Layout):
already_placed_group_ids = frozenset(root.all_window_ids())
if groups_to_remove := already_placed_group_ids - all_present_group_ids:
self.remove_windows(*groups_to_remove)
# removing windows can collapse the tree, replacing the root
root = self.pairs_root
if groups_to_add := all_present_group_ids - already_placed_group_ids:
id_idx_map = {g.id: i for i, g in enumerate(groups)}
for gid in sorted(groups_to_add, key=id_idx_map.__getitem__):
root.balanced_add(gid)
self.balanced_add_window(gid)
if len(groups) == 1:
self.layout_single_window_group(groups[0])
@ -711,12 +768,12 @@ class Splits(Layout):
wheight = aw.geometry.bottom - aw.geometry.top
horizontal = wwidth >= wheight
target_group = all_windows.add_window(window, next_to=aw, before=not after)
self.split_and_add(pair, group_id, target_group.id, horizontal, after, bias)
self.split_and_add_window(pair, group_id, target_group.id, horizontal, after, bias)
return
all_windows.add_window(window)
g = all_windows.group_for_window(window)
assert g is not None
p = self.pairs_root.balanced_add(g.id)
p = self.pairs_root.balanced_add(g.id) if bias is not None else self.balanced_add_window(g.id)
if bias is not None:
p.bias = bias
@ -814,9 +871,9 @@ class Splits(Layout):
# Re-insert next to dest
pair = self.pairs_root.pair_for_window(dest_wg.id)
if pair is not None:
self.split_and_add(pair, dest_wg.id, src_wg.id, horizontal, after)
self.split_and_add_window(pair, dest_wg.id, src_wg.id, horizontal, after)
else:
self.pairs_root.balanced_add(src_wg.id)
self.balanced_add_window(src_wg.id)
def layout_action(self, action_name: str, args: Sequence[str], all_windows: WindowList) -> bool | None:
if action_name == 'rotate':

68
kitty_tests/layout.py
View file

@ -587,6 +587,32 @@ class TestLayout(BaseTest):
self.assertAlmostEqual(root.bias, 0.5, places=5) # w1 vs right column: 1:1
self.assertAlmostEqual(inner1.bias, 0.5, places=5) # w2 vs w3 top/bottom: 1:1
def test_splits_collapse_nested_empty_pairs(self):
# Removing every window of a nested sub-tree in a single pass must not
# leave an empty pair behind, still consuming its share of the space.
q = create_layout(Splits)
all_windows = create_windows(q, num=0)
for i in range(1, 6):
q.add_window(all_windows, Window(i))
q.pairs_root.unserialize({'one': {'one': {'one': 1, 'two': 2}, 'two': {'one': 3, 'two': 4}}, 'two': 5}, lambda x: x)
q.remove_windows(3, 2, 1, 4)
self.ae(list(q.pairs_root.all_window_ids()), [5])
for pair in q.pairs_root.self_and_descendants():
self.assertFalse(pair.one is None and pair.two is None, f'empty pair left in tree: {q.pairs_root}')
self.ae(q.pairs_root.window_weights(), {5: 1.0})
def test_splits_relayout_after_root_collapse(self):
# Windows added in the same relayout that collapses the root must go into
# the new root, not the discarded one.
q = create_layout(Splits)
all_windows = create_windows(q, num=0)
for i in (1, 2, 3):
q.add_window(all_windows, Window(i), location='vsplit')
all_windows.remove_window(all_windows.id_map[1])
all_windows.add_window(Window(4))
q(all_windows)
self.ae(sorted(q.pairs_root.all_window_ids()), [2, 3, 4])
def test_layout_opts_serialization(self):
opts = SplitsLayoutOpts({})
s = opts.serialized()
@ -976,7 +1002,7 @@ class TestProportionalSplits(BaseTest):
q = create_layout(Splits)
q.layout_opts = SplitsLayoutOpts({'proportional': 'yes', **options})
windows = create_windows(q, num=0)
for i in range(1, num + 1 if shape else 2):
for i in range(1, (num + 1) if shape else 2):
q.add_window(windows, Window(i), location='vsplit')
if shape:
q.pairs_root.unserialize(shape, lambda x: x)
@ -1102,3 +1128,43 @@ class TestProportionalSplits(BaseTest):
q.remove_windows(1)
self.assertTrue(q.on_window_removed(windows))
self.check_weights(q, {2: 0.5, 3: 0.5})
def test_proportional_close_with_zero_sized_survivor(self):
# Window 3 has been squeezed to nothing by maximize/bias, so it has zero
# weight. Closing window 2 must not hand its column's space to windows 1
# and 4, which are not in any sub-tree that lost a window.
shape = {'one': 1, 'two': {'one': {'bias': 1.0, 'one': 2, 'two': 3}, 'two': 4}}
q, windows = self.make_layout(shape, num=4)
self.check_weights(q, {1: 0.5, 2: 0.25, 3: 0.0, 4: 0.25})
q.remove_windows(2)
self.check_weights(q, {1: 0.5, 3: 0.25, 4: 0.25})
def test_proportional_batch_close_matches_sequential(self):
# Pruning several windows in one pass (which happens when they are closed
# while a different layout is active) must match closing them one by one.
shape = {
'bias': 0.4,
'one': {'horizontal': False, 'one': 1, 'two': {'one': 2, 'two': 3}},
'two': {'horizontal': False, 'one': 4, 'two': {'horizontal': False, 'one': 5, 'two': 6}},
}
removed = (6, 1, 3)
q, windows = self.make_layout(shape, num=6)
q.remove_windows(*removed)
batch = q.pairs_root.window_weights()
q, windows = self.make_layout(shape, num=6)
for wid in removed:
q.remove_windows(wid)
sequential = q.pairs_root.window_weights()
self.ae(batch.keys(), sequential.keys())
for wid, weight in sequential.items():
self.assertAlmostEqual(batch[wid], weight)
def test_proportional_balanced_add(self):
# A window that appears without being split off an existing one (it was
# created while another layout was active) still gets a proportional share.
q, windows = self.make_layout()
q.add_window(windows, Window(2), location='vsplit')
q.add_window(windows, Window(3), location='vsplit')
self.check_weights(q, dict.fromkeys((1, 2, 3), 1 / 3))
self.ae(q.balanced_add_window(4).horizontal, q.pairs_root.horizontal)
self.check_weights(q, dict.fromkeys((1, 2, 3, 4), 0.25))