mirror of
https://github.com/kovidgoyal/kitty.git
synced 2026-05-29 11:18:50 +02:00
Rework window title bar architecture per review feedback
- Eliminate double set_geometry() call: removed _apply_window_title_bars() which post-processed geometry causing expensive SIGWINCH to children - Move title bar screen ownership to Window objects instead of central manager, with show_title_bar flag set during layout before do_layout() - Window.set_geometry() now handles title bar geometry internally: self.geometry stays at layout-computed value (borders/padding correct), only C-side render data diverges via adjusted top/bottom - Hide title bar for 1-row windows (ynum <= 1) - Hide title bar when template evaluates to empty/whitespace - Optimize C render loop: merge title bar GPU prep and draw into existing per-window loops, use trd pointer and is_visible=false, use num_visible_windows > 1 guard. Eliminates separate iteration passes. - Simplify WindowTitleBarManager to thin coordinator Note on C-side GPU prep placement: the suggested patch placed send_cell_data_to_gpu for title bars inside the is_active_window branch only. This caused a segfault (NULL deref in gleRunVertexSubmitImmediate) because inactive windows' title bars had valid screen/geometry but no GPU data uploaded, yet draw_cells was called for all visible title bars. Moved to the per-window visibility block alongside the main window's send_cell_data_to_gpu call so all visible title bars get GPU data prepared. The draw loop matches the suggested patch exactly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
+9
-16
@@ -781,7 +781,6 @@ prepare_to_render_os_window(OSWindow *os_window, monotonic_t now, unsigned int *
|
||||
set_maximum_wait(OPT(cursor_trail) - now + WD.screen->cursor->position_changed_by_client_at);
|
||||
}
|
||||
}
|
||||
|
||||
} else {
|
||||
if (WD.screen->cursor_render_info.render_even_when_unfocused) {
|
||||
if (collect_cursor_info(&WD.screen->cursor_render_info, w, now, os_window)) needs_render = true;
|
||||
@@ -806,12 +805,12 @@ prepare_to_render_os_window(OSWindow *os_window, monotonic_t now, unsigned int *
|
||||
}
|
||||
if (send_cell_data_to_gpu(WD.vao_idx, WD.screen, os_window)) needs_render = true;
|
||||
if (WD.screen->start_visual_bell_at != 0) needs_render = true;
|
||||
}
|
||||
// Prepare window title bar screen data for GPU
|
||||
if (w->visible && w->window_title_render_data.screen) {
|
||||
CursorRenderInfo *cri = &w->window_title_render_data.screen->cursor_render_info;
|
||||
zero_at_ptr(cri);
|
||||
if (send_cell_data_to_gpu(w->window_title_render_data.vao_idx, w->window_title_render_data.screen, os_window)) needs_render = true;
|
||||
// Prepare window title bar screen data for GPU
|
||||
WindowRenderData *trd = &w->window_title_render_data;
|
||||
if (trd->screen) {
|
||||
trd->screen->cursor_render_info.is_visible = false;
|
||||
if (send_cell_data_to_gpu(trd->vao_idx, trd->screen, os_window)) needs_render = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
return needs_render || was_previously_rendered_with_layers != os_window->needs_layers;
|
||||
@@ -872,15 +871,9 @@ render_prepared_os_window(OSWindow *os_window, unsigned int active_window_id, co
|
||||
if (is_active_window) active_window = w;
|
||||
draw_cells(&WD, os_window, is_active_window, false, num_of_visible_windows == 1, w);
|
||||
if (WD.screen->start_visual_bell_at != 0) set_maximum_wait(ANIMATION_SAMPLE_WAIT);
|
||||
}
|
||||
}
|
||||
// Draw window title bars
|
||||
for (unsigned int i = 0; i < tab->num_windows; i++) {
|
||||
Window *w = tab->windows + i;
|
||||
if (w->visible && w->window_title_render_data.screen &&
|
||||
w->window_title_render_data.geometry.right > w->window_title_render_data.geometry.left &&
|
||||
w->window_title_render_data.geometry.bottom > w->window_title_render_data.geometry.top) {
|
||||
draw_cells(&w->window_title_render_data, os_window, i == tab->active_window, true, false, NULL);
|
||||
WindowRenderData *trd = &w->window_title_render_data;
|
||||
if (trd->screen && num_visible_windows > 1 && trd->geometry.right > trd->geometry.left && trd->geometry.bottom > trd->geometry.top)
|
||||
draw_cells(trd, os_window, i == tab->active_window, true, false, NULL);
|
||||
}
|
||||
}
|
||||
setup_os_window_for_rendering(os_window, tab, active_window, false);
|
||||
|
||||
+6
-18
@@ -367,27 +367,15 @@ class Layout:
|
||||
self._set_dimensions(all_windows)
|
||||
self.update_visibility(all_windows)
|
||||
self.blank_rects = []
|
||||
self.do_layout(all_windows)
|
||||
self._apply_window_title_bars(all_windows)
|
||||
|
||||
def _apply_window_title_bars(self, all_windows: WindowList) -> None:
|
||||
# Set show_title_bar flag on each visible window before layout
|
||||
opts = get_options()
|
||||
position = opts.window_title_bar
|
||||
if position == 'none':
|
||||
return
|
||||
title_bar_enabled = opts.window_title_bar != 'none'
|
||||
visible_groups = list(all_windows.iter_all_layoutable_groups(only_visible=True))
|
||||
if len(visible_groups) < 2:
|
||||
return
|
||||
ch = lgd.cell_height
|
||||
num_visible = len(visible_groups)
|
||||
for wg in visible_groups:
|
||||
geom = wg.geometry
|
||||
if geom is None:
|
||||
continue
|
||||
if position == 'top':
|
||||
new_geom = geom._replace(top=geom.top + ch, ynum=max(1, geom.ynum - 1))
|
||||
else:
|
||||
new_geom = geom._replace(bottom=geom.bottom - ch, ynum=max(1, geom.ynum - 1))
|
||||
wg.set_geometry(new_geom)
|
||||
for w in wg.windows:
|
||||
w.show_title_bar = title_bar_enabled and num_visible > 1
|
||||
self.do_layout(all_windows)
|
||||
|
||||
def layout_single_window_group(self, wg: WindowGroup, add_blank_rects: bool = True) -> None:
|
||||
bw = 1 if self.must_draw_borders else 0
|
||||
|
||||
+94
-4
@@ -84,6 +84,7 @@ from .fast_data_types import (
|
||||
set_window_logo,
|
||||
set_window_padding,
|
||||
set_window_render_data,
|
||||
set_window_title_bar_render_data,
|
||||
update_ime_position_for_window,
|
||||
update_pointer_shape,
|
||||
update_window_title,
|
||||
@@ -732,6 +733,8 @@ class Window:
|
||||
self.tabref: Callable[[], TabType | None] = weakref.ref(tab)
|
||||
self.destroyed = False
|
||||
self.geometry: WindowGeometry = WindowGeometry(0, 0, 0, 0, 0, 0)
|
||||
self.show_title_bar: bool = False
|
||||
self._title_bar_screen: Any = None
|
||||
self.needs_layout = True
|
||||
self.is_visible_in_layout: bool = True
|
||||
self.child = child
|
||||
@@ -977,13 +980,36 @@ class Window:
|
||||
def set_geometry(self, new_geometry: WindowGeometry) -> None:
|
||||
if self.destroyed:
|
||||
return
|
||||
if self.needs_layout or new_geometry.xnum != self.screen.columns or new_geometry.ynum != self.screen.lines:
|
||||
self.screen.resize(max(0, new_geometry.ynum), max(0, new_geometry.xnum))
|
||||
# Determine if we need a title bar and compute adjusted dimensions
|
||||
opts = get_options()
|
||||
position = opts.window_title_bar
|
||||
show_tb = self.show_title_bar and new_geometry.ynum > 1
|
||||
|
||||
if show_tb:
|
||||
render_ynum = new_geometry.ynum - 1
|
||||
cell_width, cell_height = cell_size_for_window(self.os_window_id)
|
||||
if position == 'top':
|
||||
render_top = new_geometry.top + cell_height
|
||||
render_bottom = new_geometry.bottom
|
||||
tb_top = new_geometry.top
|
||||
tb_bottom = new_geometry.top + cell_height
|
||||
else:
|
||||
render_top = new_geometry.top
|
||||
render_bottom = new_geometry.bottom - cell_height
|
||||
tb_top = new_geometry.bottom - cell_height
|
||||
tb_bottom = new_geometry.bottom
|
||||
else:
|
||||
render_ynum = new_geometry.ynum
|
||||
render_top = new_geometry.top
|
||||
render_bottom = new_geometry.bottom
|
||||
|
||||
if self.needs_layout or new_geometry.xnum != self.screen.columns or render_ynum != self.screen.lines:
|
||||
self.screen.resize(max(0, render_ynum), max(0, new_geometry.xnum))
|
||||
self.needs_layout = False
|
||||
call_watchers(weakref.ref(self), 'on_resize', {'old_geometry': self.geometry, 'new_geometry': new_geometry})
|
||||
current_pty_size = (
|
||||
self.screen.lines, self.screen.columns,
|
||||
max(0, new_geometry.right - new_geometry.left), max(0, new_geometry.bottom - new_geometry.top))
|
||||
max(0, new_geometry.right - new_geometry.left), max(0, render_bottom - render_top))
|
||||
update_ime_position = False
|
||||
if current_pty_size != self.last_reported_pty_size:
|
||||
if self._pause_resize_notifications_to_child is None:
|
||||
@@ -993,14 +1019,78 @@ class Window:
|
||||
else:
|
||||
mark_os_window_dirty(self.os_window_id)
|
||||
|
||||
# Store original geometry for borders/padding calculations
|
||||
self.geometry = g = new_geometry
|
||||
# Set C-side render data with adjusted top/bottom for content area
|
||||
set_window_render_data(self.os_window_id, self.tab_id, self.id, self.screen,
|
||||
g.left, g.top, g.right, g.bottom,
|
||||
g.left, render_top, g.right, render_bottom,
|
||||
g.spaces.left, g.spaces.top, g.spaces.right, g.spaces.bottom)
|
||||
self.update_effective_padding()
|
||||
|
||||
# Handle title bar screen
|
||||
if show_tb:
|
||||
from .window_title_bar import WindowTitleBarScreen
|
||||
if self._title_bar_screen is None:
|
||||
self._title_bar_screen = WindowTitleBarScreen(self.os_window_id, cell_width, cell_height)
|
||||
tb_geom = WindowGeometry(
|
||||
left=g.left, top=tb_top, right=g.right, bottom=tb_bottom,
|
||||
xnum=0, ynum=1,
|
||||
)
|
||||
self._title_bar_screen.layout(tb_geom)
|
||||
set_window_title_bar_render_data(
|
||||
self.os_window_id, self.tab_id, self.id, self._title_bar_screen.screen,
|
||||
tb_geom.left, tb_geom.top, tb_geom.right, tb_geom.bottom,
|
||||
)
|
||||
elif self._title_bar_screen is not None:
|
||||
# Clear title bar render data
|
||||
set_window_title_bar_render_data(
|
||||
self.os_window_id, self.tab_id, self.id, self._title_bar_screen.screen,
|
||||
0, 0, 0, 0,
|
||||
)
|
||||
self._title_bar_screen = None
|
||||
|
||||
if update_ime_position:
|
||||
update_ime_position_for_window(self.id, True)
|
||||
|
||||
def update_title_bar(self, is_active: bool = False) -> None:
|
||||
pts = self._title_bar_screen
|
||||
if pts is None:
|
||||
return
|
||||
from .progress import ProgressState
|
||||
from .window_title_bar import WindowTitleData
|
||||
|
||||
progress_percent = ''
|
||||
if self.progress.state is not ProgressState.unset:
|
||||
if self.progress.state is ProgressState.indeterminate:
|
||||
progress_percent = '[…] '
|
||||
elif self.progress.percent > 0:
|
||||
progress_percent = f'[{self.progress.percent}%] '
|
||||
|
||||
has_activity = self.has_activity_since_last_focus
|
||||
|
||||
data = WindowTitleData(
|
||||
title=self.title or '',
|
||||
is_active=is_active,
|
||||
window_id=self.id,
|
||||
tab_id=self.tab_id,
|
||||
needs_attention=self.needs_attention,
|
||||
has_activity_since_last_focus=has_activity,
|
||||
)
|
||||
rendered_title = pts.render(data, progress_percent)
|
||||
|
||||
# If template evaluates to empty/whitespace, zero title bar geometry to hide it
|
||||
if not rendered_title or not rendered_title.strip():
|
||||
set_window_title_bar_render_data(
|
||||
self.os_window_id, self.tab_id, self.id, pts.screen,
|
||||
0, 0, 0, 0,
|
||||
)
|
||||
else:
|
||||
g = pts.geometry
|
||||
set_window_title_bar_render_data(
|
||||
self.os_window_id, self.tab_id, self.id, pts.screen,
|
||||
g.left, g.top, g.right, g.bottom,
|
||||
)
|
||||
|
||||
def close(self) -> None:
|
||||
get_boss().mark_window_for_close(self)
|
||||
|
||||
|
||||
+7
-111
@@ -9,11 +9,8 @@ from .constants import config_dir
|
||||
from .fast_data_types import (
|
||||
DECAWM,
|
||||
Screen,
|
||||
cell_size_for_window,
|
||||
get_options,
|
||||
set_window_title_bar_render_data,
|
||||
)
|
||||
from .progress import ProgressState
|
||||
from .rgb import color_as_sgr, color_from_int, to_color
|
||||
from .types import WindowGeometry, run_once
|
||||
from .utils import color_as_int, log_error, sgr_sanitizer_pat
|
||||
@@ -150,7 +147,7 @@ class WindowTitleBarScreen:
|
||||
self.screen.resize(1, ncells)
|
||||
self.geometry = geometry
|
||||
|
||||
def render(self, data: WindowTitleData, progress_percent: str) -> None:
|
||||
def render(self, data: WindowTitleData, progress_percent: str) -> str:
|
||||
opts = get_options()
|
||||
s = self.screen
|
||||
s.cursor.x = 0
|
||||
@@ -226,122 +223,21 @@ class WindowTitleBarScreen:
|
||||
while s.cursor.x < s.columns:
|
||||
s.draw(' ')
|
||||
|
||||
return title_str
|
||||
|
||||
|
||||
class WindowTitleBarManager:
|
||||
|
||||
def __init__(self, os_window_id: int, tab_id: int):
|
||||
self.os_window_id = os_window_id
|
||||
self.tab_id = tab_id
|
||||
self._screens: dict[int, WindowTitleBarScreen] = {}
|
||||
|
||||
def _clear_all(self) -> None:
|
||||
for wid, pts in self._screens.items():
|
||||
# Zero geometry so the C render loop skips drawing
|
||||
set_window_title_bar_render_data(
|
||||
self.os_window_id, self.tab_id, wid, pts.screen,
|
||||
0, 0, 0, 0,
|
||||
)
|
||||
self._screens.clear()
|
||||
|
||||
def update(self, all_windows: WindowList) -> None:
|
||||
opts = get_options()
|
||||
position = opts.window_title_bar
|
||||
if position == 'none':
|
||||
if self._screens:
|
||||
self._clear_all()
|
||||
return
|
||||
|
||||
visible_groups = list(all_windows.iter_all_layoutable_groups(only_visible=True))
|
||||
if len(visible_groups) < 2:
|
||||
if self._screens:
|
||||
self._clear_all()
|
||||
return
|
||||
|
||||
cell_width, cell_height = cell_size_for_window(self.os_window_id)
|
||||
active_group = all_windows.active_group
|
||||
seen_window_ids: set[int] = set()
|
||||
|
||||
for wg in visible_groups:
|
||||
geom = wg.geometry
|
||||
if geom is None:
|
||||
continue
|
||||
|
||||
window = wg.windows[-1] if wg.windows else None
|
||||
if window is None:
|
||||
continue
|
||||
|
||||
# Validate geometry has enough space for a title bar
|
||||
if geom.right <= geom.left or geom.bottom <= geom.top:
|
||||
continue
|
||||
if position == 'top' and geom.top < cell_height:
|
||||
continue
|
||||
if position == 'bottom' and geom.bottom + cell_height < geom.bottom: # overflow check
|
||||
continue
|
||||
|
||||
wid = window.id
|
||||
seen_window_ids.add(wid)
|
||||
|
||||
if wid not in self._screens:
|
||||
self._screens[wid] = WindowTitleBarScreen(self.os_window_id, cell_width, cell_height)
|
||||
|
||||
pts = self._screens[wid]
|
||||
|
||||
# Calculate title bar geometry
|
||||
if position == 'top':
|
||||
title_geom = WindowGeometry(
|
||||
left=geom.left,
|
||||
top=geom.top - cell_height,
|
||||
right=geom.right,
|
||||
bottom=geom.top,
|
||||
xnum=0, ynum=1,
|
||||
)
|
||||
else:
|
||||
title_geom = WindowGeometry(
|
||||
left=geom.left,
|
||||
top=geom.bottom,
|
||||
right=geom.right,
|
||||
bottom=geom.bottom + cell_height,
|
||||
xnum=0, ynum=1,
|
||||
)
|
||||
|
||||
pts.layout(title_geom)
|
||||
|
||||
for wg in all_windows.iter_all_layoutable_groups(only_visible=True):
|
||||
is_active = wg is active_group
|
||||
|
||||
# Get bell/activity state from the window object
|
||||
needs_attention = getattr(window, 'needs_attention', False)
|
||||
has_activity = getattr(window, 'has_activity_since_last_focus', False)
|
||||
if callable(has_activity):
|
||||
has_activity = has_activity()
|
||||
|
||||
# Get progress info
|
||||
progress_percent = ''
|
||||
progress = getattr(window, 'progress', None)
|
||||
if progress is not None and progress.state is not ProgressState.unset:
|
||||
if progress.state is ProgressState.indeterminate:
|
||||
progress_percent = '[…] '
|
||||
elif progress.percent > 0:
|
||||
progress_percent = f'[{progress.percent}%] '
|
||||
|
||||
data = WindowTitleData(
|
||||
title=window.title or '',
|
||||
is_active=is_active,
|
||||
window_id=wid,
|
||||
tab_id=self.tab_id,
|
||||
needs_attention=needs_attention,
|
||||
has_activity_since_last_focus=has_activity,
|
||||
)
|
||||
pts.render(data, progress_percent)
|
||||
|
||||
set_window_title_bar_render_data(
|
||||
self.os_window_id, self.tab_id, wid, pts.screen,
|
||||
title_geom.left, title_geom.top, title_geom.right, title_geom.bottom,
|
||||
)
|
||||
|
||||
# Clean up screens for windows that are no longer visible
|
||||
stale = set(self._screens) - seen_window_ids
|
||||
for wid in stale:
|
||||
del self._screens[wid]
|
||||
for w in wg.windows:
|
||||
w.update_title_bar(is_active=is_active)
|
||||
|
||||
def destroy(self) -> None:
|
||||
self._screens.clear()
|
||||
pass
|
||||
|
||||
Reference in New Issue
Block a user