From 5b21d32b6bd8d55f61e8c43de75c40ab5f9765bd Mon Sep 17 00:00:00 2001 From: JDonaghy Date: Wed, 15 Jul 2026 23:40:37 +0000 Subject: [PATCH] fix(#550): unify GTK/TUI window-rect coordinate convention to absolute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TUI computed window_rects (and everything derived from them — breadcrumb/tab-bar bounds, dividers, window rects) relative to the editor content area's own top-left, then re-added editor_left/menu_rows at ~40 draw and click-hit-test call sites in render_impl.rs and mouse.rs. GTK's window rects were already absolute. This asymmetry is why render::breadcrumb_draw_targets()/tab_bar_draw_targets() carried an origin_offset param that only TUI needed. - build_screen_for_tui now builds content_bounds at the editor area's real absolute origin (activity bar + sidebar, menu bar), matching GTK's convention, instead of always (0.0, 0.0). - Dropped the now-always-(0,0) origin_offset param from breadcrumb_draw_targets()/tab_bar_draw_targets(); updated both backends' call sites. - Removed every now-redundant editor_area.x/.y (render_impl.rs) and editor_left/menu_rows (mouse.rs) offset addition applied to window-rect-derived data — window/tab-bar/breadcrumb/divider bounds, click and hover hit-tests, scrollbar tracks, tab-drag geometry. screen_to_drop_group_bounds's TUI call sites pick a (0,0) vs. real origin drop_origin exactly like GTK's existing #515 fix, so split-mode tab-group-drop overlays don't double-count the origin. - quadraui's TUI-only draw_find_replace(..., editor_left) still expects a content-relative group_bounds internally (can't edit the quadraui repo from here); vimcode's wrapper now always passes 0 since group_bounds is absolute already, keeping that internal translation a no-op. mouse.rs's matching hit-test math updated identically. - Added two regression tests in tui_main::mouse::tests that build a real ScreenLayout via build_screen_for_tui with sidebar + menu bar visible (non-zero origin) and dispatch clicks at the painted absolute coordinates, verified to fail if the offset math regresses. Co-Authored-By: Claude Sonnet 5 --- src/gtk/click.rs | 1 - src/gtk/mod.rs | 16 +- src/render.rs | 121 ++++------ src/tui_main/mouse.rs | 416 +++++++++++++++++++++++++++++------ src/tui_main/quadraui_tui.rs | 17 +- src/tui_main/render_impl.rs | 132 ++++++----- 6 files changed, 480 insertions(+), 223 deletions(-) diff --git a/src/gtk/click.rs b/src/gtk/click.rs index 1eabfe6d..5e0f625e 100644 --- a/src/gtk/click.rs +++ b/src/gtk/click.rs @@ -1255,7 +1255,6 @@ mod frame_hit_map_tests { &screen, tab_row_h, tab_bar_h, - (0.0, 0.0), (0.0, 0.0, 800.0), )) { diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index 0f1c6614..152c6d3a 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -7813,16 +7813,9 @@ impl quadraui::ShellApp for App { // comment for why a plain `Vec` indexed from 0 was wrong. let mut tab_bar_zones: HashMap = HashMap::new(); - for (next_surface_idx, target) in - (window_editors.len()..).zip(render::tab_bar_draw_targets( - &engine, - screen, - tab_row_h, - tab_bar_h, - (0.0, 0.0), - (x, y, w), - )) - { + for (next_surface_idx, target) in (window_editors.len()..).zip( + render::tab_bar_draw_targets(&engine, screen, tab_row_h, tab_bar_h, (x, y, w)), + ) { let tb_rect = target.rect; let hover = self .tab_close_hover @@ -7867,8 +7860,7 @@ impl quadraui::ShellApp for App { // step was never ported over, so breadcrumbs stopped rendering even // though layout space for them was still reserved (`tab_bar_h` // above) and clicks were still hit-tested against them. - for t in render::breadcrumb_draw_targets(screen, engine.terminal_maximized, lh, (0.0, 0.0)) - { + for t in render::breadcrumb_draw_targets(screen, engine.terminal_maximized, lh) { let mut frame = QSL::new(); frame.push(Surface::StatusBar { rect: t.rect, diff --git a/src/render.rs b/src/render.rs index ebdc4c84..b500a782 100644 --- a/src/render.rs +++ b/src/render.rs @@ -738,11 +738,13 @@ pub struct BreadcrumbDrawTarget<'a> { /// Relm4-era draw path that *did* draw breadcrumbs stopped being called /// after the #540 ShellApp migration and nothing replaced it). /// -/// `origin_offset` translates `bc.bounds` (computed relative to the -/// coordinate space of the `window_rects` passed into `build_screen_layout`) -/// into the caller's screen space: GTK's window rects are already absolute, -/// so it passes `(0.0, 0.0)`; TUI's are content-area-relative, so it passes -/// `(editor_area.x, editor_area.y)`. +/// `bc.bounds` is already in the caller's screen space: both backends feed +/// `build_screen_layout` window rects in absolute terminal/pixel coordinates +/// (#550 — TUI used to compute content-area-relative rects and every draw +/// call site had to re-add the editor area's origin via an `origin_offset` +/// param here; that offset is always `(0.0, 0.0)` now that TUI's +/// `content_bounds` origin matches GTK's convention, so the param was +/// dropped). /// /// Targets with zero width (the `min_x == f64::MAX` fallback in /// `build_screen_layout` when a group has no matching window rects, e.g. @@ -754,20 +756,18 @@ pub fn breadcrumb_draw_targets( screen: &ScreenLayout, terminal_maximized: bool, line_height: f64, - origin_offset: (f64, f64), ) -> Vec> { if terminal_maximized { return Vec::new(); } - let (ox, oy) = origin_offset; screen .breadcrumbs .iter() .filter(|bc| !bc.segments.is_empty() && bc.bounds.width > 0.0) .map(|bc| BreadcrumbDrawTarget { rect: quadraui::Rect::new( - (bc.bounds.x + ox) as f32, - (bc.bounds.y + oy) as f32, + bc.bounds.x as f32, + bc.bounds.y as f32, bc.bounds.width as f32, line_height as f32, ), @@ -816,11 +816,10 @@ pub struct TabBarDrawTarget<'a> { /// 2 rows) — used to recover the tab row's own top edge from /// `GroupTabBar::bounds.y`, which is the *window* content's top edge. /// -/// `origin_offset` translates `bounds` (relative to the coordinate space of -/// the `window_rects` passed into `build_screen_layout`) into the caller's -/// screen space, same convention as `breadcrumb_draw_targets`: GTK's window -/// rects are already absolute, so it passes `(0.0, 0.0)`; TUI's are -/// content-area-relative, so it passes `(editor_area.x, editor_area.y)`. +/// `bounds` is already in the caller's screen space, same convention as +/// `breadcrumb_draw_targets` (#550 — the `origin_offset` param this function +/// used to carry for TUI's content-area-relative rects was dropped once TUI +/// started feeding absolute rects like GTK). /// /// `single_group_rect` is `(x, y, width)` for the single-group tab bar, /// already in the caller's output coordinate space (there is no per-group @@ -839,10 +838,8 @@ pub fn tab_bar_draw_targets<'a>( screen: &'a ScreenLayout, tab_row_h: f64, reserved_h: f64, - origin_offset: (f64, f64), single_group_rect: (f64, f64, f64), ) -> Vec> { - let (ox, oy) = origin_offset; if let Some(ref split) = screen.editor_group_split { split .group_tab_bars @@ -850,8 +847,8 @@ pub fn tab_bar_draw_targets<'a>( .filter(|gtb| !engine.is_tab_bar_hidden(gtb.group_id) && gtb.bounds.width > 0.0) .map(|gtb| TabBarDrawTarget { rect: quadraui::Rect::new( - (gtb.bounds.x + ox) as f32, - (gtb.bounds.y - reserved_h + oy) as f32, + gtb.bounds.x as f32, + (gtb.bounds.y - reserved_h) as f32, gtb.bounds.width as f32, tab_row_h as f32, ), @@ -16016,9 +16013,10 @@ mod tests { /// Direct unit test for `breadcrumb_draw_targets` itself (#547 review /// finding: the test above only pins the pre-existing `build_screen_layout` /// bounds computation, never the new shared helper). Covers the - /// `terminal_maximized` early return, the `origin_offset` translation - /// arithmetic, the `segments.is_empty()` filter, and the zero-width - /// fallback filter. + /// `terminal_maximized` early return, the pass-through of already-absolute + /// bounds (#550 — the `origin_offset` translation this used to carry was + /// dropped once both backends feed absolute window rects), the + /// `segments.is_empty()` filter, and the zero-width fallback filter. #[test] fn test_breadcrumb_draw_targets_offset_terminal_maximized_and_filters() { use crate::core::engine::Engine; @@ -16039,7 +16037,9 @@ mod tests { engine.buffer_manager.get_mut(buf_id).unwrap().file_path = Some(std::path::PathBuf::from("src/main.rs")); let tbh = 24.0; - let content_bounds = WindowRect::new(0.0, 0.0, 800.0, 600.0); + // Non-zero origin to prove `breadcrumb_draw_targets` passes + // through absolute bounds untouched rather than assuming (0,0). + let content_bounds = WindowRect::new(10.0, 20.0, 800.0, 600.0); let (rects, _) = engine.calculate_group_window_rects(content_bounds, tbh); build_screen_layout(&engine, &theme, &rects, line_height, char_width, true) }; @@ -16049,45 +16049,28 @@ mod tests { assert!(!screen.breadcrumbs[0].segments.is_empty()); assert!(screen.breadcrumbs[0].bounds.width > 0.0); - // `terminal_maximized` short-circuits to empty regardless of offset. - let targets = breadcrumb_draw_targets(&screen, true, line_height, (10.0, 20.0)); + // `terminal_maximized` short-circuits to empty. + let targets = breadcrumb_draw_targets(&screen, true, line_height); assert!( targets.is_empty(), "terminal_maximized must suppress all breadcrumb targets" ); - // Not maximized: one target, translated by `origin_offset` (TUI's convention). - let targets = breadcrumb_draw_targets(&screen, false, line_height, (10.0, 20.0)); + // Not maximized: one target, matching the already-absolute bounds. + let targets = breadcrumb_draw_targets(&screen, false, line_height); assert_eq!(targets.len(), 1); - assert_eq!( - targets[0].rect.x, - (screen.breadcrumbs[0].bounds.x + 10.0) as f32 - ); - assert_eq!( - targets[0].rect.y, - (screen.breadcrumbs[0].bounds.y + 20.0) as f32 - ); + assert_eq!(targets[0].rect.x, screen.breadcrumbs[0].bounds.x as f32); + assert_eq!(targets[0].rect.y, screen.breadcrumbs[0].bounds.y as f32); assert_eq!( targets[0].rect.width, screen.breadcrumbs[0].bounds.width as f32 ); assert_eq!(targets[0].rect.height, line_height as f32); - // Zero offset (GTK's convention): rect matches raw bounds untouched. - let targets_zero = breadcrumb_draw_targets(&screen, false, line_height, (0.0, 0.0)); - assert_eq!( - targets_zero[0].rect.x, - screen.breadcrumbs[0].bounds.x as f32 - ); - assert_eq!( - targets_zero[0].rect.y, - screen.breadcrumbs[0].bounds.y as f32 - ); - // Empty segments are filtered out even when not maximized. let mut screen_no_segments = build_screen(); screen_no_segments.breadcrumbs[0].segments.clear(); - let targets = breadcrumb_draw_targets(&screen_no_segments, false, line_height, (0.0, 0.0)); + let targets = breadcrumb_draw_targets(&screen_no_segments, false, line_height); assert!( targets.is_empty(), "a breadcrumb bar with no segments must not be drawn" @@ -16098,7 +16081,7 @@ mod tests { // its own `rect.width > 0.0` guard (unlike TUI's pre-existing one). let mut screen_zero_width = build_screen(); screen_zero_width.breadcrumbs[0].bounds.width = 0.0; - let targets = breadcrumb_draw_targets(&screen_zero_width, false, line_height, (0.0, 0.0)); + let targets = breadcrumb_draw_targets(&screen_zero_width, false, line_height); assert!( targets.is_empty(), "a zero-width breadcrumb bar must not be drawn" @@ -16107,9 +16090,11 @@ mod tests { /// Direct unit test for `tab_bar_draw_targets` (#549, follow-up to /// #547's `breadcrumb_draw_targets`). Covers the single-group rect - /// pass-through, the split-group `reserved_h` subtraction + - /// `origin_offset` translation, the `is_tab_bar_hidden` filter in both - /// modes, and the zero-width fallback filter in split mode. + /// pass-through, the split-group `reserved_h` subtraction against + /// already-absolute bounds (#550 — the `origin_offset` translation this + /// used to carry was dropped once both backends feed absolute window + /// rects), the `is_tab_bar_hidden` filter in both modes, and the + /// zero-width fallback filter in split mode. #[test] fn test_tab_bar_draw_targets_single_and_split() { use crate::core::engine::Engine; @@ -16128,14 +16113,8 @@ mod tests { let screen = build_screen_layout(&engine, &theme, &rects, line_height, char_width, false); assert!(screen.editor_group_split.is_none()); - let targets = tab_bar_draw_targets( - &engine, - &screen, - tab_row_h, - reserved_h, - (0.0, 0.0), - (10.0, 20.0, 800.0), - ); + let targets = + tab_bar_draw_targets(&engine, &screen, tab_row_h, reserved_h, (10.0, 20.0, 800.0)); assert_eq!(targets.len(), 1); assert_eq!(targets[0].rect.x, 10.0); assert_eq!(targets[0].rect.y, 20.0); @@ -16152,7 +16131,6 @@ mod tests { &screen_hidden, tab_row_h, reserved_h, - (0.0, 0.0), (10.0, 20.0, 800.0), ); assert!( @@ -16161,9 +16139,12 @@ mod tests { ); // ── Split-group mode ──────────────────────────────────────────── + // Non-zero content_bounds origin to prove `tab_bar_draw_targets` + // passes through absolute bounds untouched rather than assuming (0,0). let mut engine = Engine::new(); engine.execute_command("EditorGroupSplit"); assert_eq!(engine.group_layout.leaf_count(), 2); + let content_bounds = WindowRect::new(5.0, 7.0, 800.0, 600.0); let (rects, _) = engine.calculate_group_window_rects(content_bounds, reserved_h); let screen = build_screen_layout(&engine, &theme, &rects, line_height, char_width, false); let split = screen @@ -16172,13 +16153,12 @@ mod tests { .expect("2 groups must produce Some(editor_group_split)"); assert_eq!(split.group_tab_bars.len(), 2); - // Zero offset (GTK's convention): rect derived from bounds.y - reserved_h. + // Rect derived from the already-absolute `bounds.y - reserved_h`. let targets = tab_bar_draw_targets( &engine, &screen, tab_row_h, reserved_h, - (0.0, 0.0), (0.0, 0.0, 0.0), // unused in split mode ); assert_eq!(targets.len(), 2); @@ -16190,24 +16170,6 @@ mod tests { assert_eq!(target.rect.height, tab_row_h as f32); } - // Non-zero offset (TUI's convention): translated by origin_offset. - let targets = tab_bar_draw_targets( - &engine, - &screen, - tab_row_h, - reserved_h, - (5.0, 7.0), - (0.0, 0.0, 0.0), - ); - assert_eq!( - targets[0].rect.x, - (split.group_tab_bars[0].bounds.x + 5.0) as f32 - ); - assert_eq!( - targets[0].rect.y, - (split.group_tab_bars[0].bounds.y - reserved_h + 7.0) as f32 - ); - // Note: `is_tab_bar_hidden` only ever returns true in single-group // mode (`hide_single_tab` + `leaf_count() <= 1`, see // `Engine::is_tab_bar_hidden`), so there's no reachable per-group @@ -16231,7 +16193,6 @@ mod tests { &screen_zero_width, tab_row_h, reserved_h, - (0.0, 0.0), (0.0, 0.0, 0.0), ); assert_eq!( diff --git a/src/tui_main/mouse.rs b/src/tui_main/mouse.rs index 451c412b..fbe8c73d 100644 --- a/src/tui_main/mouse.rs +++ b/src/tui_main/mouse.rs @@ -315,10 +315,19 @@ pub(super) fn handle_mouse( let row_count: u16 = if panel.show_replace { 2 } else { 1 }; let panel_h: u16 = row_count + 2; // +2 for borders - // Compute panel screen position from group_bounds + // Compute panel screen position from group_bounds. #550: + // `group_bounds` is already absolute terminal-screen space (see + // the matching comment at the `draw_find_replace` call site in + // render_impl.rs, which now passes `editor_left=0` into + // quadraui's rasteriser since the offset is baked into + // `group_bounds` already). This hit-test math must mirror that + // paint math exactly (mismatched math here is the "column drift + // bug" class this overlay's doc comment warns about) — so no + // `editor_left +`/`.max(editor_left)` here either, matching + // quadraui's now-effectively-zero clamp. let gb = &panel.group_bounds; - let gb_right = editor_left + gb.x as u16 + gb.width as u16; - let panel_x = gb_right.saturating_sub(panel_w + 1).max(editor_left); + let gb_right = gb.x as u16 + gb.width as u16; + let panel_x = gb_right.saturating_sub(panel_w + 1); let panel_y = (gb.y as u16).max(1); let content_x = panel_x + 1; // inside left border let find_y = panel_y + 1; // first content row @@ -972,12 +981,12 @@ pub(super) fn handle_mouse( if let Some(split) = last_layout.and_then(|l| l.editor_group_split.as_ref()) { if let Some(div) = split.dividers.iter().find(|d| d.split_index == split_index) { - let mr: u16 = if engine.menu_bar_visible { 1 } else { 0 }; - let editor_row = row.saturating_sub(mr); - let rel_col = col.saturating_sub(editor_left); + // #550: `div.axis_start`/`.axis_size` are already + // absolute terminal-screen coordinates, so `col`/`row` + // compare directly with no editor-origin subtraction. let mouse_pos = match div.direction { - crate::core::window::SplitDirection::Vertical => rel_col as f64, - crate::core::window::SplitDirection::Horizontal => editor_row as f64, + crate::core::window::SplitDirection::Vertical => col as f64, + crate::core::window::SplitDirection::Horizontal => row as f64, }; let new_ratio = (mouse_pos - div.axis_start) / div.axis_size; engine @@ -1031,17 +1040,17 @@ pub(super) fn handle_mouse( ); if col >= editor_left { if let Some(layout) = last_layout { - let menu_rows: u16 = if engine.menu_bar_visible { 1 } else { 0 }; - let editor_row = row.saturating_sub(menu_rows); - let rel_col = col - editor_left; - if let Some(idx) = - render::find_window_at(layout, rel_col as f64, editor_row as f64) - { + // #550: `rw.rect` (and everything `find_window_at`/ + // `window_zone_hit_test` compare it against) is already + // absolute terminal-screen space, so the raw event + // `col`/`row` are used directly — no editor-area-relative + // translation. + if let Some(idx) = render::find_window_at(layout, col as f64, row as f64) { let rw = &layout.windows[idx]; let zone = render::window_zone_hit_test( rw, - (rel_col as f64) - rw.rect.x, - (editor_row as f64) - rw.rect.y, + (col as f64) - rw.rect.x, + (row as f64) - rw.rect.y, 1.0, 1.0, ); @@ -1060,10 +1069,13 @@ pub(super) fn handle_mouse( // text-layout inverse (`EditorLayout::col_at_x`) // instead of hand-rolled cell math, so TUI and // GTK column resolution can never diverge. + // `col_at_x` takes an absolute x matching + // `editor.rect`'s space (mirrors GTK's + // `editor_col_at_x` call, see gtk/click.rs). let (editor, editor_layout) = render::editor_text_layout(rw, 1.0, 1.0); let col_in_text = - editor_layout.col_at_x(&editor, view_row, rel_col as f32); + editor_layout.col_at_x(&editor, view_row, col as f32); engine.mouse_drag(rw.window_id, buf_line, col_in_text); } return sidebar_width; @@ -1359,17 +1371,12 @@ pub(super) fn handle_mouse( return sidebar_width; } "tui:editor_viewport" => { - let scroll_menu_rows: u16 = - if engine.menu_bar_visible { 1 } else { 0 }; - let editor_row = row.saturating_sub(scroll_menu_rows); - let rel_col = col.saturating_sub(editor_left); + // #550: `find_window_at` compares against + // already-absolute `rw.rect`, so the raw + // event `col`/`row` are used directly. let target = last_layout.and_then(|layout| { - render::find_window_at( - layout, - rel_col as f64, - editor_row as f64, - ) - .map(|idx| &layout.windows[idx]) + render::find_window_at(layout, col as f64, row as f64) + .map(|idx| &layout.windows[idx]) }); if let Some(rw) = target { let dir = if down { 1 } else { -1 }; @@ -1483,12 +1490,15 @@ pub(super) fn handle_mouse( if let Some(ref split) = layout.editor_group_split { let click_tbh: u16 = if engine.settings.breadcrumbs { 2 } else { 1 }; for gtb in split.group_tab_bars.iter() { - let tab_bar_row = - menu_rows + (gtb.bounds.y as u16).saturating_sub(click_tbh); + // #550: `gtb.bounds` is already absolute + // terminal-screen space, so no `menu_rows`/ + // `editor_left` offset addition — compare directly + // against the raw event `col`/`row`. + let tab_bar_row = (gtb.bounds.y as u16).saturating_sub(click_tbh); let gx = gtb.bounds.x as u16; let gw = gtb.bounds.width as u16; - if row == tab_bar_row && rel_col >= gx && rel_col < gx + gw { - let local_col = rel_col - gx; + if row == tab_bar_row && col >= gx && col < gx + gw { + let local_col = col - gx; let bar = render::build_tab_bar_primitive( >b.tabs, false, @@ -1783,12 +1793,13 @@ pub(super) fn handle_mouse( if let Some(ref split) = layout.editor_group_split { let click_tbh: u16 = if engine.settings.breadcrumbs { 2 } else { 1 }; for gtb in split.group_tab_bars.iter() { - let tab_bar_row = - menu_rows + (gtb.bounds.y as u16).saturating_sub(click_tbh); + // #550: `gtb.bounds` is already absolute — compare + // against raw `col`, not `rel_col`/`menu_rows`. + let tab_bar_row = (gtb.bounds.y as u16).saturating_sub(click_tbh); let gx = gtb.bounds.x as u16; let gw = gtb.bounds.width as u16; - if row == tab_bar_row && rel_col >= gx && rel_col < gx + gw { - let local_col = rel_col - gx; + if row == tab_bar_row && col >= gx && col < gx + gw { + let local_col = col - gx; tooltip = tab_tooltip_at_col( engine, gtb.group_id, @@ -1827,16 +1838,14 @@ pub(super) fn handle_mouse( ) || engine.is_vscode_mode()) { if let Some(layout) = last_layout { - let menu_rows: u16 = if engine.menu_bar_visible { 1 } else { 0 }; - let editor_row = row.saturating_sub(menu_rows); - let rel_col = col - editor_left; + // #550: `rw.rect` is already absolute — use raw `col`/`row`. let mut found = false; - if let Some(idx) = render::find_window_at(layout, rel_col as f64, editor_row as f64) { + if let Some(idx) = render::find_window_at(layout, col as f64, row as f64) { let rw = &layout.windows[idx]; let zone = render::window_zone_hit_test( rw, - (rel_col as f64) - rw.rect.x, - (editor_row as f64) - rw.rect.y, + (col as f64) - rw.rect.x, + (row as f64) - rw.rect.y, 1.0, 1.0, ); @@ -1847,7 +1856,7 @@ pub(super) fn handle_mouse( // #560: shared quadraui text-layout inverse (see the // drag handler above for the full rationale). let (editor, editor_layout) = render::editor_text_layout(rw, 1.0, 1.0); - let text_col = editor_layout.col_at_x(&editor, view_row, rel_col as f32); + let text_col = editor_layout.col_at_x(&editor, view_row, col as f32); engine.editor_hover_mouse_move(buf_line, text_col, mouse_on_editor_hover); found = true; } @@ -2606,8 +2615,9 @@ pub(super) fn handle_mouse( // ── Breadcrumb click ──────────────────────────────────────────────────── if engine.settings.breadcrumbs { if let Some(layout) = last_layout { - let bc_x = (col - editor_left) as f64; - let bc_y = (row - menu_rows) as f64; + // #550: `layout.breadcrumbs[..].bounds` is already absolute. + let bc_x = col as f64; + let bc_y = row as f64; match render::resolve_breadcrumb_click(&layout.breadcrumbs, bc_x, bc_y, 1.0) { render::BreadcrumbClickResult::Hit(idx) => { if !matches!(ev.kind, MouseEventKind::Down(MouseButton::Left)) { @@ -2638,14 +2648,16 @@ pub(super) fn handle_mouse( if engine.is_tab_bar_hidden(gtb.group_id) { continue; } - let tab_bar_row = menu_rows + (gtb.bounds.y as u16).saturating_sub(click_tbh); + // #550: `gtb.bounds` is already absolute — no `menu_rows`/ + // `editor_left` offset addition, compare against raw `col`/`row`. + let tab_bar_row = (gtb.bounds.y as u16).saturating_sub(click_tbh); let gx = gtb.bounds.x as u16; let gw = gtb.bounds.width as u16; - if row == tab_bar_row && rel_col >= gx && rel_col < gx + gw { + if row == tab_bar_row && col >= gx && col < gx + gw { let was_active = gtb.group_id == split.active_group; matched_group = Some(( gtb.group_id, - rel_col - gx, + col - gx, gw, >b.tabs, gtb.diff_toolbar.as_ref(), @@ -2796,11 +2808,10 @@ pub(super) fn handle_mouse( } } - let rel_col = col - editor_left; - // editor_row is 0-indexed relative to the editor content area. - // Window rects already include the tab_bar_height offset (y >= 1), - // so we only subtract menu_rows here (not the tab bar row). - let editor_row = row.saturating_sub(menu_rows); + // #550: `rw.rect`/`div.position`/`.cross_start` etc. below are all + // already absolute terminal-screen coordinates, so the raw event + // `col`/`row` are used directly throughout this block — no + // editor-area-relative translation needed. // ── Group divider click — start drag ────────────────────────────────────── // #452: must use the same float-to-int conversion as the divider @@ -2818,16 +2829,16 @@ pub(super) fn handle_mouse( let hit = match div.direction { crate::core::window::SplitDirection::Vertical => { let div_col = div.position as u16; - rel_col == div_col - && (editor_row as f64) >= div.cross_start - && (editor_row as f64) < div.cross_start + div.cross_size + col == div_col + && (row as f64) >= div.cross_start + && (row as f64) < div.cross_start + div.cross_size } crate::core::window::SplitDirection::Horizontal => { let div_row = div.position as u16; - editor_row >= div_row - && editor_row < div_row + tab_bar_rows - && (rel_col as f64) >= div.cross_start - && (rel_col as f64) < div.cross_start + div.cross_size + row >= div_row + && row < div_row + tab_bar_rows + && (col as f64) >= div.cross_start + && (col as f64) < div.cross_start + div.cross_size } }; if hit { @@ -2845,11 +2856,11 @@ pub(super) fn handle_mouse( let ww = rw.rect.width as u16; let wh = rw.rect.height as u16; - if rel_col >= wx && rel_col < wx + ww && editor_row >= wy && editor_row < wy + wh { + if col >= wx && col < wx + ww && row >= wy && row < wy + wh { // Per-window status bar click — hit-test segments for actions. - if rw.status_line.is_some() && wh > 1 && editor_row == wy + wh - 1 { + if rw.status_line.is_some() && wh > 1 && row == wy + wh - 1 { if let Some(ref status) = rw.status_line { - let click_col = (rel_col - wx) as usize; + let click_col = (col - wx) as usize; if let Some(action) = status_segment_hit_test(status, ww as usize, click_col) { @@ -2894,9 +2905,10 @@ pub(super) fn handle_mouse( let has_h_scrollbar = rw.max_col > viewport_cols && content_height > 1; // Vertical scrollbar click/drag-start (rightmost column) - if has_v_scrollbar && rel_col == wx + ww - 1 { - // menu_rows = menu bar offset; wy already includes tab_bar_height - let track_abs_start = menu_rows + wy; + if has_v_scrollbar && col == wx + ww - 1 { + // #550: `wy` is already absolute (includes both the menu + // bar offset and tab_bar_height), so no `menu_rows +`. + let track_abs_start = wy; // V-track loses 1 row to each of: per-window status line, // horizontal scrollbar (if either present). let track_len = @@ -2983,11 +2995,12 @@ pub(super) fn handle_mouse( } else { wy + wh - 1 }; - if has_h_scrollbar && editor_row == h_sb_row { + if has_h_scrollbar && row == h_sb_row { let track_x = wx + gutter; let track_w = ww.saturating_sub(gutter + if has_v_scrollbar { 1 } else { 0 }); - if rel_col >= track_x && rel_col < track_x + track_w && track_w > 0 { - let track_abs_start = editor_left + track_x; + if col >= track_x && col < track_x + track_w && track_w > 0 { + // #550: `track_x` (derived from `wx`) is already absolute. + let track_abs_start = track_x; let track_visible = viewport_cols; // Track-click vs thumb-click: page-jump on the // empty track, drag-start on the thumb (mirrors @@ -3048,10 +3061,10 @@ pub(super) fn handle_mouse( } // Check gutter area — shared resolution via render::resolve_gutter_action (#344). - let view_row = (editor_row - wy) as usize; - if gutter > 0 && rel_col >= wx && rel_col < wx + gutter { + let view_row = (row - wy) as usize; + if gutter > 0 && col >= wx && col < wx + gutter { if let Some(rl) = rw.lines.get(view_row) { - let gutter_col = (rel_col - wx) as usize; + let gutter_col = (col - wx) as usize; use crate::render::GutterAction; match crate::render::resolve_gutter_action(rw, rl.line_idx, gutter_col) { Some(GutterAction::ToggleBreakpoint(line)) => { @@ -3103,7 +3116,7 @@ pub(super) fn handle_mouse( // function GTK's `Backend::editor_col_at_x` falls back to, // so both backends' click math derives from one source. let (editor, editor_layout) = crate::render::editor_text_layout(rw, 1.0, 1.0); - let col_in_text = editor_layout.col_at_x(&editor, view_row, rel_col as f32); + let col_in_text = editor_layout.col_at_x(&editor, view_row, col as f32); // Double-click detection let now = Instant::now(); @@ -3417,4 +3430,263 @@ mod tests { let result = menu_system.handle(&outside_event, &mut backend, bar_rect); assert_eq!(result, quadraui::MenuEvent::Ignored); } + + // ── #550: absolute window-rect coordinate convention ──────────────────── + // + // These tests build a *real* `ScreenLayout` via `build_screen_for_tui` + // (the same production function `draw_frame` uses to paint) with the + // sidebar and menu bar both visible, so `window_rects` carry a non-zero + // origin. They then dispatch a click through `handle_mouse` at a + // coordinate read straight off that `ScreenLayout` (not re-derived by + // hand) and assert the click resolves correctly. Before #550, TUI's + // window rects were content-area-relative and every click site + // re-added `editor_left`/`menu_rows` on top of them — a bug reintroduced + // in either the paint or the click math would show up here as a + // click/paint coordinate mismatch (wrong group hit, or no hit at all). + + /// Build a hermetic engine with a vertical group split, sidebar visible, + /// and the menu bar visible — the scenario with the largest non-zero + /// `(x, y)` editor-area origin, to maximize the chance of catching an + /// offset regression. + fn split_engine_with_sidebar_and_menu() -> Engine { + let mut e = Engine::new(); + e.settings = crate::core::settings::Settings::default(); + e.mode = crate::core::Mode::Normal; + e.menu_bar_visible = true; + if !e.app_shell.sidebar_visible() { + e.toggle_sidebar(); + } + e.open_editor_group(crate::core::window::SplitDirection::Vertical); + e + } + + fn dispatch_left_click( + engine: &mut Engine, + col: u16, + row: u16, + last_layout: Option<&render::ScreenLayout>, + dragging_group_divider: &mut Option, + ) { + let ev = MouseEvent { + kind: MouseEventKind::Down(MouseButton::Left), + column: col, + row, + modifiers: KeyModifiers::NONE, + }; + let mut sidebar = TuiSidebar::new(); + let mut drag_state = quadraui::DragState::default(); + let mut modal_stack = quadraui::ModalStack::new(); + let mut last_click_time = Instant::now(); + let mut last_click_pos: (u16, u16) = (0, 0); + let mut should_quit = false; + + handle_mouse( + ev, + &mut sidebar, + engine, + &Some(Size { + width: 120, + height: 40, + }), + SIDEBAR_WIDTH, + &mut false, + &mut false, + &mut false, + dragging_group_divider, + &mut drag_state, + &mut modal_stack, + last_layout, + &mut last_click_time, + &mut last_click_pos, + &mut None, + &mut None, + &mut false, + &mut should_quit, + &mut None, + &mut None, + &mut None, + &mut false, + &mut None, + &mut None, + &mut crate::core::window::DropZone::None, + &[], + None, + None, + &[], + None, + &mut false, + &mut false, + None, + None, + None, + ); + } + + /// A click exactly on a group divider (per the freshly-painted + /// `ScreenLayout`) must start the divider drag, and a click one column + /// off it must not. With the sidebar (30 cols) + activity bar (3 cols) + + /// menu bar (1 row) all visible, the divider's absolute column sits well + /// past both the old content-relative value AND a "double-counted + /// offset" value would — pinning it via the real painted position (not a + /// hand-derived formula) means either regression breaks this test. + #[test] + fn group_divider_click_matches_painted_divider_position() { + let engine = split_engine_with_sidebar_and_menu(); + let theme = crate::render::Theme::onedark(); + let sidebar = TuiSidebar::new(); + let area = Rect { + x: 0, + y: 0, + width: 120, + height: 40, + }; + let screen = super::render_impl::build_screen_for_tui( + &engine, + &theme, + area, + &sidebar, + SIDEBAR_WIDTH, + ); + let split = screen + .editor_group_split + .as_ref() + .expect("vertical split must produce Some(editor_group_split)"); + let div = split + .dividers + .first() + .expect("a vertical split has exactly one divider"); + assert_eq!(div.direction, crate::core::window::SplitDirection::Vertical); + + // Sanity: this is genuinely testing a non-trivial offset, not a + // degenerate zero-origin case. + let editor_left = ACTIVITY_BAR_WIDTH + SIDEBAR_WIDTH + 1; + assert!( + (div.position as u16) > editor_left, + "divider column {} should sit inside the editor area (left edge {editor_left})", + div.position + ); + + let col = div.position as u16; + let row = (div.cross_start + 1.0) as u16; + + let mut engine_hit = split_engine_with_sidebar_and_menu(); + let mut dragging = None; + dispatch_left_click(&mut engine_hit, col, row, Some(&screen), &mut dragging); + assert_eq!( + dragging, + Some(div.split_index), + "click at the painted divider column ({col}, {row}) must start the divider drag" + ); + + // One column off must NOT hit the divider. + let mut engine_miss = split_engine_with_sidebar_and_menu(); + let mut dragging_miss = None; + dispatch_left_click( + &mut engine_miss, + col - 1, + row, + Some(&screen), + &mut dragging_miss, + ); + assert_eq!( + dragging_miss, None, + "click one column off the divider must not start a drag" + ); + } + + /// Right-clicking a split group's tab bar at its painted absolute + /// position must open that group's tab context menu. Exercises the + /// same `gtb.bounds`-vs-`editor_left`/`menu_rows` arithmetic as the + /// divider test above, via the right-click path instead of the + /// left-click path. + #[test] + fn split_group_tab_bar_right_click_matches_painted_position() { + let mut engine = split_engine_with_sidebar_and_menu(); + let theme = crate::render::Theme::onedark(); + let sidebar = TuiSidebar::new(); + let area = Rect { + x: 0, + y: 0, + width: 120, + height: 40, + }; + let screen = super::render_impl::build_screen_for_tui( + &engine, + &theme, + area, + &sidebar, + SIDEBAR_WIDTH, + ); + let split = screen + .editor_group_split + .as_ref() + .expect("vertical split must produce Some(editor_group_split)"); + let gtb = split + .group_tab_bars + .first() + .expect("a 2-group split has two group tab bars"); + let tab_bar_row = (gtb.bounds.y as u16).saturating_sub(1); + let col = gtb.bounds.x as u16 + 1; + + assert!(engine.context_menu.is_none()); + + let ev = MouseEvent { + kind: MouseEventKind::Down(MouseButton::Right), + column: col, + row: tab_bar_row, + modifiers: KeyModifiers::NONE, + }; + let mut sidebar_state = TuiSidebar::new(); + let mut drag_state = quadraui::DragState::default(); + let mut modal_stack = quadraui::ModalStack::new(); + let mut last_click_time = Instant::now(); + let mut last_click_pos: (u16, u16) = (0, 0); + let mut should_quit = false; + + handle_mouse( + ev, + &mut sidebar_state, + &mut engine, + &Some(Size { + width: 120, + height: 40, + }), + SIDEBAR_WIDTH, + &mut false, + &mut false, + &mut false, + &mut None, + &mut drag_state, + &mut modal_stack, + Some(&screen), + &mut last_click_time, + &mut last_click_pos, + &mut None, + &mut None, + &mut false, + &mut should_quit, + &mut None, + &mut None, + &mut None, + &mut false, + &mut None, + &mut None, + &mut crate::core::window::DropZone::None, + &[], + None, + None, + &[], + None, + &mut false, + &mut false, + None, + None, + None, + ); + + assert!( + engine.context_menu.is_some(), + "right-click at the painted tab-bar position ({col}, {tab_bar_row}) must open the tab context menu" + ); + } } diff --git a/src/tui_main/quadraui_tui.rs b/src/tui_main/quadraui_tui.rs index 3dc0178b..00b1736b 100644 --- a/src/tui_main/quadraui_tui.rs +++ b/src/tui_main/quadraui_tui.rs @@ -156,10 +156,16 @@ pub(super) fn draw_activity_bar( /// drift bugs (the same class fixed for debug toolbar + breadcrumb) /// can't recur on this overlay. /// -/// `editor_left` is the absolute screen column of the editor area's -/// left edge (after activity bar + sidebar). `panel.group_bounds.x/y` -/// are content-relative; the overlay anchors at the top-right of the -/// active editor group. +/// `panel.group_bounds.x/y` is already absolute terminal-screen space +/// (#550 — it's derived from `window_rects`, which TUI now feeds in +/// absolute coordinates like GTK, rather than content-area-relative). The +/// underlying `quadraui::tui::draw_find_replace` rasteriser still takes an +/// `editor_left` translation param (it's TUI-only — GTK never calls this +/// path — and quadraui's signature can't be changed from here); this +/// wrapper always passes `0` so that internal translation is a no-op +/// instead of double-counting the origin already baked into +/// `group_bounds`. The click-hit-test mirroring this paint math is in +/// `mouse.rs`'s find/replace handler — keep the two in sync. /// /// Painting that the hit-region list doesn't directly cover — /// borders, the match-count text (a non-clickable status string), and @@ -170,9 +176,8 @@ pub(super) fn draw_find_replace( area: Rect, panel: &crate::render::FindReplacePanel, theme: &Theme, - editor_left: u16, ) { - quadraui::tui::draw_find_replace(buf, area, panel, &q_theme(theme), editor_left); + quadraui::tui::draw_find_replace(buf, area, panel, &q_theme(theme), 0); } /// Draw a `quadraui::RichTextPopup` into the buffer via the lifted diff --git a/src/tui_main/render_impl.rs b/src/tui_main/render_impl.rs index f45bfb34..312d2e37 100644 --- a/src/tui_main/render_impl.rs +++ b/src/tui_main/render_impl.rs @@ -49,7 +49,19 @@ pub(super) fn build_screen_for_tui( ACTIVITY_BAR_WIDTH }; let content_cols = area.width.saturating_sub(ab_width + sidebar_cols); - let content_bounds = WindowRect::new(0.0, 0.0, content_cols as f64, content_rows as f64); + // #550: window rects are absolute terminal-screen coordinates, matching + // GTK's convention, rather than relative to the editor content area's own + // top-left. `editor_area`'s origin here must match the `Layout` split + // `draw_frame` performs on the same `area` (menu bar row, then activity + // bar + sidebar columns) — see the mirrored computation there. + let editor_origin_x = area.x as f64 + ab_width as f64 + sidebar_cols as f64; + let editor_origin_y = area.y as f64 + menu_height as f64; + let content_bounds = WindowRect::new( + editor_origin_x, + editor_origin_y, + content_cols as f64, + content_rows as f64, + ); let tui_tab_bar_height = if engine.settings.breadcrumbs && !engine.terminal_maximized { 2.0 } else { @@ -309,7 +321,6 @@ pub(super) fn draw_frame( screen, 1.0, tui_tbh, - (editor_area.x as f64, editor_area.y as f64), ( editor_area.x as f64, editor_area.y as f64, @@ -339,7 +350,7 @@ pub(super) fn draw_frame( } // Render windows first so tab bars draw on top (prevents window content // from overwriting an adjacent group's tab bar in horizontal splits). - render_all_windows(backend, frame, editor_area, &screen.windows, theme); + render_all_windows(backend, frame, &screen.windows, theme); // Draw each group's tab bar. Tab bar sits tab_bar_height rows above // the group's window content (bounds.y - tab_bar_height). for target in &tab_bar_targets { @@ -357,12 +368,7 @@ pub(super) fn draw_frame( // conditions + rect math (including the zero-width fallback filter) // come from `render::breadcrumb_draw_targets`, shared with GTK, so // the two backends can't drift apart (#547). - for t in render::breadcrumb_draw_targets( - screen, - engine.terminal_maximized, - 1.0, - (editor_area.x as f64, editor_area.y as f64), - ) { + for t in render::breadcrumb_draw_targets(screen, engine.terminal_maximized, 1.0) { let bc_rect = Rect { x: t.rect.x as u16, y: t.rect.y as u16, @@ -373,12 +379,15 @@ pub(super) fn draw_frame( *t.draw_layout.borrow_mut() = Some(layout); } // Draw divider lines (vertical only — horizontal splits use the tab bar as divider). + // `div.position`/`.cross_start` are already absolute terminal-screen + // coordinates (#550), matching `editor_area`'s own coordinate space — + // no offset addition needed. let sep_fg = rc(theme.separator); let sep_bg = rc(theme.background); for div in &split.dividers { if div.direction == SplitDirection::Vertical { - let div_x = editor_area.x + div.position as u16; - let y_start = editor_area.y + div.cross_start as u16; + let div_x = div.position as u16; + let y_start = div.cross_start as u16; let y_end = y_start + div.cross_size as u16; for y in y_start..y_end { if div_x < editor_area.x + editor_area.width { @@ -423,12 +432,7 @@ pub(super) fn draw_frame( // → `adjust_group_rects_for_hidden_tabs` shifts the window rect (and // therefore the derived breadcrumb bounds) up by one row in that case, // so this no longer needs its own `is_tab_bar_hidden` special case (#547). - for t in render::breadcrumb_draw_targets( - screen, - engine.terminal_maximized, - 1.0, - (editor_area.x as f64, editor_area.y as f64), - ) { + for t in render::breadcrumb_draw_targets(screen, engine.terminal_maximized, 1.0) { let bc_rect = Rect { x: t.rect.x as u16, y: t.rect.y as u16, @@ -438,7 +442,7 @@ pub(super) fn draw_frame( let layout = draw_breadcrumb_bar(backend, frame, bc_rect, t.bar, theme); *t.draw_layout.borrow_mut() = Some(layout); } - render_all_windows(backend, frame, editor_area, &screen.windows, theme); + render_all_windows(backend, frame, &screen.windows, theme); } // Register the editor viewport as a scroll surface so dispatch_scroll @@ -496,8 +500,8 @@ pub(super) fn draw_frame( { if let Some((cursor_pos, _)) = &active_win.cursor { let gutter_w = active_win.gutter_char_width as u16; - let win_x = editor_area.x + active_win.rect.x as u16; - let win_y = editor_area.y + active_win.rect.y as u16; + let win_x = active_win.rect.x as u16; + let win_y = active_win.rect.y as u16; let raw = active_win .lines .get(cursor_pos.view_line) @@ -546,8 +550,8 @@ pub(super) fn draw_frame( .find(|w| w.window_id == screen.active_window_id) { let gutter_w = active_win.gutter_char_width as u16; - let win_x = editor_area.x + active_win.rect.x as u16; - let win_y = editor_area.y + active_win.rect.y as u16; + let win_x = active_win.rect.x as u16; + let win_y = active_win.rect.y as u16; let anchor_view = hover.anchor_line.saturating_sub(active_win.scroll_top) as u16; let vis_col = hover.anchor_col.saturating_sub(active_win.scroll_left) as u16; let popup_x = win_x + gutter_w + vis_col; @@ -576,8 +580,8 @@ pub(super) fn draw_frame( .find(|w| w.window_id == screen.active_window_id) { let gutter_w = active_win.gutter_char_width as u16; - let win_x = editor_area.x + active_win.rect.x as u16; - let win_y = editor_area.y + active_win.rect.y as u16; + let win_x = active_win.rect.x as u16; + let win_y = active_win.rect.y as u16; // Use frozen scroll offsets so the popup stays fixed on screen let anchor_view = eh.anchor_line.saturating_sub(eh.frozen_scroll_top) as u16; let vis_col = eh.anchor_col.saturating_sub(eh.frozen_scroll_left) as u16; @@ -599,8 +603,8 @@ pub(super) fn draw_frame( .find(|w| w.window_id == screen.active_window_id) { let gutter_w = active_win.gutter_char_width as u16; - let win_x = editor_area.x + active_win.rect.x as u16; - let win_y = editor_area.y + active_win.rect.y as u16; + let win_x = active_win.rect.x as u16; + let win_y = active_win.rect.y as u16; let anchor_view = peek.anchor_line.saturating_sub(active_win.scroll_top) as u16; let popup_x = win_x + gutter_w; // anchor at the cursor's own row; placement=Bottom (with @@ -628,8 +632,8 @@ pub(super) fn draw_frame( .find(|w| w.window_id == screen.active_window_id) { let gutter_w = active_win.gutter_char_width as u16; - let win_x = editor_area.x + active_win.rect.x as u16; - let win_y = editor_area.y + active_win.rect.y as u16; + let win_x = active_win.rect.x as u16; + let win_y = active_win.rect.y as u16; let anchor_view = sig.anchor_line.saturating_sub(active_win.scroll_top) as u16; let vis_col = sig.anchor_col.saturating_sub(active_win.scroll_left) as u16; let popup_x = win_x + gutter_w + vis_col; @@ -921,14 +925,16 @@ pub(super) fn draw_frame( // ── Find/replace overlay (top-right of active group) ─────────────────── if let Some(ref find_replace) = screen.find_replace { - let editor_left = h_chunks[0].width + h_chunks[1].width; - super::quadraui_tui::draw_find_replace( - frame.buffer_mut(), - area, - find_replace, - theme, - editor_left, - ); + // #550: `find_replace.group_bounds` is derived from `window_rects` + // (render.rs's `active_group_bounds`) and is now absolute + // terminal-screen space, not content-relative. quadraui's shared + // `draw_find_replace(..., editor_left)` still expects to translate a + // content-relative `group_bounds` by `editor_left` internally (it's + // TUI-only — GTK never calls this path, so there's no established + // absolute-input convention to lean on there); passing `0` here + // keeps that internal translation a no-op instead of double- + // counting the origin now baked into `group_bounds` itself. + super::quadraui_tui::draw_find_replace(frame.buffer_mut(), area, find_replace, theme); } // ── Unified picker modal (above terminal/status so it's fully visible) ── @@ -1219,13 +1225,18 @@ fn build_tui_tab_slots( let mut map = std::collections::HashMap::new(); if let Some(ref split) = screen.editor_group_split { for gtb in &split.group_tab_bars { - let abs_x = editor_x + gtb.bounds.x as f32; + // #550: `gtb.bounds` is already absolute terminal-screen space + // (same convention as GTK), so no `editor_x` offset addition. + let abs_x = gtb.bounds.x as f32; map.insert( gtb.group_id.0, tab_drag_slots_from_hit_regions(>b.hit_regions, abs_x), ); } } else { + // Single-group bar spans the editor area's own left edge; hit + // regions are bar-relative offsets, not window-rect-derived, so + // `editor_x` is still the correct base here. map.insert( engine.active_group.0, tab_drag_slots_from_hit_regions(&screen.tab_bar_hit_regions, editor_x), @@ -1256,10 +1267,21 @@ pub(super) fn render_tab_drag_overlay( } else { 1.0 }; + // #550/#515: in split mode `gtb.bounds` is already absolute (built from + // absolute window rects, matching GTK), so the origin here must be + // (0,0) — adding `editor_area`'s origin again would double-count it, + // the same bug GTK's #515 fix addressed for its own call site. + // Single-group mode returns the origin directly (no `gtb.bounds` to + // derive it from), so it still needs the real editor origin. + let drop_origin = if screen.editor_group_split.is_some() { + (0.0, 0.0) + } else { + (editor_area.x as f32, editor_area.y as f32) + }; let bounds = render::screen_to_drop_group_bounds( screen, engine, - (editor_area.x as f32, editor_area.y as f32), + drop_origin, (editor_area.width as f32, editor_area.height as f32), tbh_f, ); @@ -1347,10 +1369,17 @@ pub(super) fn compute_tui_tab_drop_zone( } else { 1.0 }; + // #550/#515: same double-count hazard as `render_tab_drag_overlay` above — + // split-mode `gtb.bounds` is already absolute, so the origin must be (0,0). + let drop_origin = if layout.editor_group_split.is_some() { + (0.0, 0.0) + } else { + (editor_left as f32, menu_rows as f32) + }; let bounds = render::screen_to_drop_group_bounds( layout, engine, - (editor_left as f32, menu_rows as f32), + drop_origin, (editor_w as f32, editor_h as f32), tbh_f, ); @@ -1416,20 +1445,20 @@ pub(super) fn draw_breadcrumb_bar( pub(super) fn render_all_windows( backend: &mut super::backend::TuiBackend, frame: &mut ratatui::Frame, - editor_area: Rect, windows: &[RenderedWindow], theme: &Theme, ) { for window in windows { + // #550: `window.rect` is already absolute terminal-screen coordinates. let win_rect = Rect { - x: editor_area.x + window.rect.x as u16, - y: editor_area.y + window.rect.y as u16, + x: window.rect.x as u16, + y: window.rect.y as u16, width: window.rect.width as u16, height: window.rect.height as u16, }; render_window(backend, frame, win_rect, window, theme); } - render_separators(frame.buffer_mut(), editor_area, windows, theme); + render_separators(frame.buffer_mut(), windows, theme); } /// Render the unified picker popup. Supports single-pane (no preview) and @@ -1570,7 +1599,6 @@ pub(super) fn char_col_to_visual(raw_text: &str, char_col: usize, tabstop: usize pub(super) fn render_separators( buf: &mut ratatui::buffer::Buffer, - editor_area: Rect, windows: &[RenderedWindow], theme: &Theme, ) { @@ -1592,10 +1620,11 @@ pub(super) fn render_separators( let v_overlap = a.rect.y.max(b.rect.y) < (a.rect.y + a.rect.height).min(b.rect.y + b.rect.height); if (a.rect.x + a.rect.width - b.rect.x).abs() < 1.0 && v_overlap { - let sep_x = editor_area.x + (a.rect.x + a.rect.width) as u16; - let y_start = editor_area.y + a.rect.y.max(b.rect.y) as u16; - let y_end = - editor_area.y + (a.rect.y + a.rect.height).min(b.rect.y + b.rect.height) as u16; + // #550: `a.rect`/`b.rect` are already absolute terminal-screen + // coordinates, so no `editor_area` offset addition needed. + let sep_x = (a.rect.x + a.rect.width) as u16; + let y_start = a.rect.y.max(b.rect.y) as u16; + let y_end = (a.rect.y + a.rect.height).min(b.rect.y + b.rect.height) as u16; // #481 (iter4): `quadraui::tui::draw_editor` already paints // window `a`'s own vertical scrollbar in this exact column @@ -1644,10 +1673,9 @@ pub(super) fn render_separators( false }; if (a.rect.y + a.rect.height - b.rect.y).abs() < 1.0 && h_overlap && !upper_has_status { - let sep_y = editor_area.y + (a.rect.y + a.rect.height) as u16; - let x_start = editor_area.x + a.rect.x.max(b.rect.x) as u16; - let x_end = - editor_area.x + (a.rect.x + a.rect.width).min(b.rect.x + b.rect.width) as u16; + let sep_y = (a.rect.y + a.rect.height) as u16; + let x_start = a.rect.x.max(b.rect.x) as u16; + let x_end = (a.rect.x + a.rect.width).min(b.rect.x + b.rect.width) as u16; for x in x_start..x_end.max(x_start) { set_cell(buf, x, sep_y.saturating_sub(1), '─', sep_fg, sep_bg); }