From 4fd1b8b16fa1fc0501bc96b85bd38a62a022b01b Mon Sep 17 00:00:00 2001 From: JDonaghy Date: Fri, 10 Jul 2026 20:00:19 +0000 Subject: [PATCH 1/2] feat(#449): route GTK Editor/TabBar click dispatch through quadraui::FrameHitMap Uses quadraui::ScreenLayout::hit_map() (quadraui#425) to recover a FrameHitMap from the same Editor/TabBar objects render_content already paints, without a second backend draw pass. click::pixel_to_click_target consults it first, falling back to render::screen_zone_hit_test's manual rect-walk for breadcrumb/divider zones (no FrameZone equivalent) and for the brief window before the first paint populates the cache. Scoped narrowly to Editor+TabBar only: including per-window StatusBar in the same hit map would have let it win over Editor for a window's bottom status row (StatusBar pushed after Editor, last-drawn-wins hit_test), changing top-level zone resolution for status-bar clicks. Dialog/ ContextMenu/Completions/etc. keep their existing cached Layout::hit_test() resolution in the overlay-arbitration cascade, which already runs before pixel_to_click_target and is unaffected. --- src/gtk/click.rs | 126 ++++++++++++++++++++++++++++++++++++++--------- src/gtk/mod.rs | 68 +++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 24 deletions(-) diff --git a/src/gtk/click.rs b/src/gtk/click.rs index 4f8ca409..f4a9fb0a 100644 --- a/src/gtk/click.rs +++ b/src/gtk/click.rs @@ -54,19 +54,33 @@ pub(super) fn pixel_to_click_target( _split_btn_map: &SplitBtnMap, _action_btn_map: &ActionBtnMap, status_segment_map: &StatusSegmentMap, + // Cached `quadraui::FrameHitMap` covering the Editor/TabBar surfaces + // painted this frame (#449), plus the parallel `(GroupId, rect)` table + // for resolving `FrameZone::TabBar { idx }`. `None` before the first + // paint. See `frame_zone_to_screen_zone` for how these replace + // `screen_zone_hit_test`'s manual Window/TabBar rect-walk. + frame_hit_map: Option<&quadraui::FrameHitMap>, + tab_bar_zones: &[(GroupId, quadraui::Rect)], mutate_focus: bool, ) -> ClickTarget { let tab_bar_height = render_mod::tab_bar_height_px(line_height, engine.settings.breadcrumbs); let single_tab_hidden = engine.is_tab_bar_hidden(engine.active_group); - let zone = render_mod::screen_zone_hit_test( - cached_layout, - x, - y, - tab_bar_height, - single_tab_hidden, - engine.active_group, - ); + let zone = frame_hit_map + .and_then(|hit_map| { + let z = frame_zone_to_screen_zone(hit_map, tab_bar_zones, cached_layout, x, y); + (!matches!(z, ScreenZone::None)).then_some(z) + }) + .unwrap_or_else(|| { + render_mod::screen_zone_hit_test( + cached_layout, + x, + y, + tab_bar_height, + single_tab_hidden, + engine.active_group, + ) + }); match zone { ScreenZone::TabBar { group_id, @@ -159,6 +173,47 @@ pub(super) fn pixel_to_click_target( } } +/// Resolve the top-level `ScreenZone` using the cached `quadraui::FrameHitMap` +/// (#449), which covers exactly the `Editor`/`TabBar` surfaces painted in +/// `App::render_content` via `quadraui::ScreenLayout::hit_map()` +/// (quadraui#425) — pushed from the SAME objects/rects already painted, so +/// this can never drift from what's on screen. Returns `ScreenZone::None` +/// when the point isn't in an Editor/TabBar zone (including breadcrumb/ +/// divider pixels, which have no `FrameZone` equivalent — the caller falls +/// back to `render_mod::screen_zone_hit_test` for those). +fn frame_zone_to_screen_zone( + hit_map: &quadraui::FrameHitMap, + tab_bar_zones: &[(GroupId, quadraui::Rect)], + cached_layout: &render::ScreenLayout, + x: f64, + y: f64, +) -> ScreenZone { + match hit_map.hit_test(x as f32, y as f32) { + quadraui::FrameZone::TabBar { idx } => { + if let Some((group_id, rect)) = tab_bar_zones.get(idx) { + return ScreenZone::TabBar { + group_id: *group_id, + local_x: x - rect.x as f64, + bar_width: rect.width as f64, + }; + } + } + quadraui::FrameZone::Editor { idx } => { + if let Some(rw) = cached_layout.windows.get(idx) { + let r = &rw.rect; + return ScreenZone::Window { + window_id: rw.window_id, + window_idx: idx, + rel_x: x - r.x, + rel_y: y - r.y, + }; + } + } + _ => {} + } + ScreenZone::None +} + /// Build the Pango context the *click* backend uses to resolve editor /// columns, matched to the editor's **painted** font. /// @@ -303,6 +358,7 @@ fn resolve_charcell_tab_click( /// This mirrors `pixel_to_click_target`'s zone resolution (read-only) so the /// caller can tell a tab-bar right-click apart from an editor right-click /// before deciding which `Msg` to dispatch. +#[allow(clippy::too_many_arguments)] pub(super) fn resolve_tab_right_click( engine: &Engine, x: f64, @@ -311,19 +367,28 @@ pub(super) fn resolve_tab_right_click( char_width: f64, cached_layout: &render::ScreenLayout, tab_pixel_hits: &TabPixelHitMap, + frame_hit_map: Option<&quadraui::FrameHitMap>, + tab_bar_zones: &[(GroupId, quadraui::Rect)], ) -> Option<(GroupId, usize)> { use crate::core::engine::TabBarClickTarget as T; let tab_bar_height = render_mod::tab_bar_height_px(line_height, engine.settings.breadcrumbs); let single_tab_hidden = engine.is_tab_bar_hidden(engine.active_group); - let zone = render_mod::screen_zone_hit_test( - cached_layout, - x, - y, - tab_bar_height, - single_tab_hidden, - engine.active_group, - ); + let zone = frame_hit_map + .and_then(|hit_map| { + let z = frame_zone_to_screen_zone(hit_map, tab_bar_zones, cached_layout, x, y); + (!matches!(z, ScreenZone::None)).then_some(z) + }) + .unwrap_or_else(|| { + render_mod::screen_zone_hit_test( + cached_layout, + x, + y, + tab_bar_height, + single_tab_hidden, + engine.active_group, + ) + }); let ScreenZone::TabBar { group_id, local_x, .. } = zone @@ -433,6 +498,8 @@ pub(super) fn handle_mouse_click( split_btn_map: &SplitBtnMap, action_btn_map: &ActionBtnMap, status_segment_map: &StatusSegmentMap, + frame_hit_map: Option<&quadraui::FrameHitMap>, + tab_bar_zones: &[(GroupId, quadraui::Rect)], ) -> (Option, Option) { match pixel_to_click_target( engine, @@ -448,6 +515,8 @@ pub(super) fn handle_mouse_click( split_btn_map, action_btn_map, status_segment_map, + frame_hit_map, + tab_bar_zones, true, // real click: focus/tab/gutter side effects are intended ) { ClickTarget::BufferPos(wid, line, col) => { @@ -533,6 +602,8 @@ pub(super) fn handle_mouse_double_click( split_btn_map: &SplitBtnMap, action_btn_map: &ActionBtnMap, status_segment_map: &StatusSegmentMap, + frame_hit_map: Option<&quadraui::FrameHitMap>, + tab_bar_zones: &[(GroupId, quadraui::Rect)], ) { if let ClickTarget::BufferPos(wid, line, col) = pixel_to_click_target( engine, @@ -548,6 +619,8 @@ pub(super) fn handle_mouse_double_click( split_btn_map, action_btn_map, status_segment_map, + frame_hit_map, + tab_bar_zones, true, // real click: focus/tab/gutter side effects are intended ) { engine.mouse_double_click(wid, line, col); @@ -577,6 +650,8 @@ pub(super) fn handle_mouse_drag( split_btn_map: &SplitBtnMap, action_btn_map: &ActionBtnMap, status_segment_map: &StatusSegmentMap, + frame_hit_map: Option<&quadraui::FrameHitMap>, + tab_bar_zones: &[(GroupId, quadraui::Rect)], ) { if let ClickTarget::BufferPos(wid, line, col) = pixel_to_click_target( engine, @@ -592,6 +667,8 @@ pub(super) fn handle_mouse_drag( split_btn_map, action_btn_map, status_segment_map, + frame_hit_map, + tab_bar_zones, false, // drag continuation: pure query, no focus/tab/gutter side effects ) { engine.mouse_drag(wid, line, col); @@ -757,16 +834,14 @@ mod emoji_click_column_tests { let layout = pango::Layout::new(&pango_ctx); layout.set_font_description(Some(&font_desc)); let metrics = pango_ctx.metrics(Some(&font_desc), None); - let line_height = - (metrics.ascent() + metrics.descent()) as f64 / pango::SCALE as f64; + let line_height = (metrics.ascent() + metrics.descent()) as f64 / pango::SCALE as f64; layout.set_text("0"); let char_width = layout.pixel_size().0 as f64; let theme = Theme::onedark(); let bounds = WindowRect::new(0.0, 0.0, 800.0, 600.0); let (rects, _) = engine.calculate_group_window_rects(bounds, (line_height * 1.6).ceil()); - let screen = - build_screen_layout(&engine, &theme, &rects, line_height, char_width, false); + let screen = build_screen_layout(&engine, &theme, &rects, line_height, char_width, false); let rw = &screen.windows[0]; assert_eq!(rw.lines[0].raw_text, text, "line should not wrap"); @@ -878,8 +953,7 @@ mod emoji_click_column_tests { probe.set_text("0"); let paint_cw = probe.pixel_size().0 as f64; let metrics = pctx.metrics(Some(&paint_font), None); - let line_height = - (metrics.ascent() + metrics.descent()) as f64 / pango::SCALE as f64; + let line_height = (metrics.ascent() + metrics.descent()) as f64 / pango::SCALE as f64; // ── The click context production actually builds, matched to the // painted char width — NOT to any `settings.font_size`. ── @@ -917,8 +991,7 @@ mod emoji_click_column_tests { let good_layout = pango::Layout::new(&click_ctx); // The pre-fix bug: font the resolver from `settings.font_size` (14). - let bad_surface = - ImageSurface::create(Format::ARgb32, 1, 1).expect("bad ImageSurface"); + let bad_surface = ImageSurface::create(Format::ARgb32, 1, 1).expect("bad ImageSurface"); let bad_cr = Context::new(&bad_surface).expect("bad Context"); let bad_ctx = pangocairo::create_context(&bad_cr); bad_ctx.set_font_description(Some(&pango::FontDescription::from_string("Monospace 14"))); @@ -1060,6 +1133,9 @@ mod cross_split_drag_focus_tests { &split_btn_map, &action_btn_map, &status_segment_map, + None, // no cached FrameHitMap in this test — exercises the + // `screen_zone_hit_test` fallback path (#449) + &[], false, // mutate_focus: drag continuation ); assert_eq!( @@ -1105,6 +1181,8 @@ mod cross_split_drag_focus_tests { &split_btn_map, &action_btn_map, &status_segment_map, + None, + &[], true, // mutate_focus: genuine click ); assert_eq!( diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index 917539f8..b8e6a3de 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -579,6 +579,22 @@ struct App { /// Cached ScreenLayout from the last draw_editor paint pass. Click handlers /// read this instead of recomputing geometry from engine state (#344). cached_screen_layout: Rc>>, + /// Accumulated `quadraui::FrameHitMap` covering the `Editor`/`TabBar` + /// zones painted in `render_content` (#449). Built via + /// `quadraui::ScreenLayout::hit_map()` (quadraui#425): pushes the SAME + /// `Editor`/`TabBar` objects and rects already painted at their existing + /// call sites, purely for hit-testing, so it can never reorder or + /// duplicate real painting. `click::pixel_to_click_target` consults this + /// first to resolve the top-level Editor/TabBar zone, falling back to + /// `render::screen_zone_hit_test`'s manual rect-walk for + /// breadcrumb/divider zones (which have no `FrameZone` equivalent) and + /// for the brief window before the first paint populates this cache. + cached_frame_hit_map: Rc>>, + /// Parallel table for resolving `FrameZone::TabBar { idx }`: `idx` + /// indexes this Vec in the same order tab bars were pushed into + /// `cached_frame_hit_map`, recovering the owning `GroupId` and drawn + /// rect that `FrameZone` itself doesn't carry. + cached_tab_bar_zones: Rc>>, /// Per-group tab-drop geometry (absolute pixel bounds) computed each frame in /// `render_content`. Both the drag overlay (same frame) and the drag hit-test /// in `handle_mouse_drag_msg` (next mouse-move) read this, so the drop-zone @@ -1384,6 +1400,8 @@ impl App { action_btn_map: Rc::new(RefCell::new(HashMap::new())), status_segment_map: Rc::new(RefCell::new(HashMap::new())), cached_screen_layout: Rc::new(RefCell::new(None)), + cached_frame_hit_map: Rc::new(RefCell::new(None)), + cached_tab_bar_zones: Rc::new(RefCell::new(Vec::new())), cached_drop_groups: Rc::new(RefCell::new(Vec::new())), cached_drop_tbh: Rc::new(Cell::new(0.0)), cached_explorer_metrics: Rc::new(Cell::new((16.0, 8.0))), @@ -1537,6 +1555,8 @@ impl App { &self.split_btn_map.borrow(), &self.action_btn_map.borrow(), &self.status_segment_map.borrow(), + self.cached_frame_hit_map.borrow().as_ref(), + &self.cached_tab_bar_zones.borrow(), true, // real click: focus/tab/gutter side effects are intended ) { engine.add_cursor_at_pos(line, col); @@ -1604,6 +1624,8 @@ impl App { &self.split_btn_map.borrow(), &self.action_btn_map.borrow(), &self.status_segment_map.borrow(), + self.cached_frame_hit_map.borrow().as_ref(), + &self.cached_tab_bar_zones.borrow(), ); } } @@ -4349,6 +4371,8 @@ impl App { &self.split_btn_map.borrow(), &self.action_btn_map.borrow(), &self.status_segment_map.borrow(), + self.cached_frame_hit_map.borrow().as_ref(), + &self.cached_tab_bar_zones.borrow(), ) } else { (None, None) @@ -4645,6 +4669,8 @@ impl App { &self.split_btn_map.borrow(), &self.action_btn_map.borrow(), &self.status_segment_map.borrow(), + self.cached_frame_hit_map.borrow().as_ref(), + &self.cached_tab_bar_zones.borrow(), true, // resolving the original tab-bar mouse-down; switching tabs is intended ); if let ClickTarget::TabBar = target { @@ -4764,6 +4790,8 @@ impl App { &self.split_btn_map.borrow(), &self.action_btn_map.borrow(), &self.status_segment_map.borrow(), + self.cached_frame_hit_map.borrow().as_ref(), + &self.cached_tab_bar_zones.borrow(), ); } self.draw_needed.set(true); @@ -7681,6 +7709,11 @@ impl quadraui::ShellApp for App { } // ── Draw editor windows ─────────────────────────────────────────────── + // `window_editors` stashes each window's owned `quadraui::Editor` + // past the loop (#449) so the FrameHitMap built just below can + // reference the SAME objects just painted, instead of constructing + // a second copy that could drift from what's on screen. + let mut window_editors: Vec = Vec::with_capacity(screen.windows.len()); for rw in &screen.windows { let editor = render::to_q_editor(rw); let rect = editor.rect; @@ -7690,6 +7723,7 @@ impl quadraui::ShellApp for App { editor: &editor, }); frame.draw(backend); + window_editors.push(editor); // Per-window status bar (when window_status_line=true, which is // the default; global_status_bar is None in that mode). @@ -7716,6 +7750,23 @@ impl quadraui::ShellApp for App { } } + // ── Recover a FrameHitMap for Editor/TabBar zone detection (#449) ────── + // Pure `.push()` accumulation into a *separate* `ScreenLayout`, built + // from the same `Editor` objects just painted above (`window_editors`, + // same order as `screen.windows` so `FrameZone::Editor { idx }` maps + // straight back to `cached_layout.windows[idx]`) plus the `TabBar` + // surfaces pushed in the loop just below. `ScreenLayout::hit_map()` + // (quadraui#425) makes no `backend.draw_*()` calls, so accumulating + // into it can never reorder or repeat the real painting done above — + // see `click::pixel_to_click_target` for the consumer side. + let mut hit_frame = QSL::new(); + for editor in &window_editors { + hit_frame.push(Surface::Editor { + rect: editor.rect, + editor, + }); + } + // ── Draw tab bar(s) — one per editor group ──────────────────────────── // Multi-group (post-split) layouts have a tab bar per group, each drawn // at the top edge of its own bounds. Single-group draws one full-width @@ -7729,6 +7780,11 @@ impl quadraui::ShellApp for App { pixel_hits.clear(); close_abs.clear(); slots_abs.clear(); + // Parallel table for `FrameZone::TabBar { idx }` resolution (#449) — + // `idx` indexes this Vec in the same order tab bars are pushed into + // `hit_frame` below, recovering the `GroupId`/rect `FrameZone` itself + // doesn't carry. See `cached_tab_bar_zones`'s doc comment. + let mut tab_bar_zones: Vec<(core::window::GroupId, quadraui::Rect)> = Vec::new(); for target in render::tab_bar_draw_targets( &engine, screen, @@ -7748,6 +7804,14 @@ impl quadraui::ShellApp for App { hovered_close: hover, }); frame.draw(backend); + // `target.bar` borrows from `screen` (function-scoped), so this + // can push directly into `hit_frame` without hoisting (#449). + hit_frame.push(Surface::TabBar { + rect: tb_rect, + bar: target.bar, + hovered_close: hover, + }); + tab_bar_zones.push((target.group_id, tb_rect)); // Recover the exact pixel geometry the rasteriser just drew and // cache it (relative to the bar's left edge) for hit-testing. let hits = backend.tab_bar_layout(tb_rect, target.bar); @@ -7763,6 +7827,8 @@ impl quadraui::ShellApp for App { drop(close_abs); drop(slots_abs); drop(pixel_hits); + *self.cached_frame_hit_map.borrow_mut() = Some(hit_frame.hit_map()); + *self.cached_tab_bar_zones.borrow_mut() = tab_bar_zones; // ── Draw breadcrumb bar(s) below tab bar(s) ───────────────────────────── // (#547) `render_content` is the active ShellApp draw path since the @@ -8304,6 +8370,8 @@ impl quadraui::ShellApp for App { self.cached_char_width, layout, &self.cached_tab_pixel_hits.borrow(), + self.cached_frame_hit_map.borrow().as_ref(), + &self.cached_tab_bar_zones.borrow(), ) }) }; From 9888aa71f1ac943de278179abb00c77ee236aac4 Mon Sep 17 00:00:00 2001 From: JDonaghy Date: Fri, 10 Jul 2026 21:00:09 +0000 Subject: [PATCH 2/2] fix(#449): unblock quadraui::ScreenLayout::hit_map() build, add FrameHitMap test coverage, fix TabBar idx offset bug Addresses review findings on the #449 FrameHitMap migration: - Confirmed `ScreenLayout::hit_map()` (quadraui#425) landed in the sibling quadraui checkout this repo actually path-depends on (../quadraui -> quadraui-fresh-560, commit c316f15). The prior request-changes review checked a stale, unrelated ~/src/quadraui clone that this repo's Cargo.toml does not resolve to. `cargo build --features gui` compiles clean. - Added unit test coverage for `frame_zone_to_screen_zone` and the `frame_hit_map` branch of `pixel_to_click_target`, building a real `quadraui::FrameHitMap` via `ScreenLayout::hit_map()` from the same Editor/TabBar surface construction `render_content` uses, instead of only exercising the `screen_zone_hit_test` fallback. - That new test coverage caught a real bug: `FrameZone::TabBar { idx }` carries the *global* surface index across the whole `ScreenLayout` (editors pushed first, tab bars after), but `cached_tab_bar_zones` was a plain `Vec` indexed 0.. per tab bar. Whenever at least one editor window was on screen (always), the index was off by the editor count and every tab-bar FrameHitMap lookup silently missed, falling back to `screen_zone_hit_test` without ever exercising the new dispatch path. Fixed by keying `cached_tab_bar_zones` by the real global surface index (a `HashMap` instead of a `Vec`). Co-Authored-By: Claude Sonnet 5 --- src/gtk/click.rs | 229 ++++++++++++++++++++++++++++++++++++++++++++--- src/gtk/mod.rs | 53 ++++++----- 2 files changed, 249 insertions(+), 33 deletions(-) diff --git a/src/gtk/click.rs b/src/gtk/click.rs index f4a9fb0a..1eabfe6d 100644 --- a/src/gtk/click.rs +++ b/src/gtk/click.rs @@ -55,12 +55,15 @@ pub(super) fn pixel_to_click_target( _action_btn_map: &ActionBtnMap, status_segment_map: &StatusSegmentMap, // Cached `quadraui::FrameHitMap` covering the Editor/TabBar surfaces - // painted this frame (#449), plus the parallel `(GroupId, rect)` table - // for resolving `FrameZone::TabBar { idx }`. `None` before the first - // paint. See `frame_zone_to_screen_zone` for how these replace - // `screen_zone_hit_test`'s manual Window/TabBar rect-walk. + // painted this frame (#449), plus a `FrameZone::TabBar { idx } -> (GroupId, + // rect)` table keyed by the tab bar's *global* surface index (editors are + // pushed into the same `ScreenLayout` before any tab bar, so a tab bar's + // `idx` is offset by however many editor surfaces preceded it — a plain + // 0-based `Vec` here would look up the wrong entry, or none at all). + // `None` before the first paint. See `frame_zone_to_screen_zone` for how + // these replace `screen_zone_hit_test`'s manual Window/TabBar rect-walk. frame_hit_map: Option<&quadraui::FrameHitMap>, - tab_bar_zones: &[(GroupId, quadraui::Rect)], + tab_bar_zones: &HashMap, mutate_focus: bool, ) -> ClickTarget { let tab_bar_height = render_mod::tab_bar_height_px(line_height, engine.settings.breadcrumbs); @@ -183,14 +186,17 @@ pub(super) fn pixel_to_click_target( /// back to `render_mod::screen_zone_hit_test` for those). fn frame_zone_to_screen_zone( hit_map: &quadraui::FrameHitMap, - tab_bar_zones: &[(GroupId, quadraui::Rect)], + // Keyed by `FrameZone::TabBar { idx }`'s global surface index, NOT a + // per-tab-bar position — see the doc comment on `pixel_to_click_target`'s + // `tab_bar_zones` parameter. + tab_bar_zones: &HashMap, cached_layout: &render::ScreenLayout, x: f64, y: f64, ) -> ScreenZone { match hit_map.hit_test(x as f32, y as f32) { quadraui::FrameZone::TabBar { idx } => { - if let Some((group_id, rect)) = tab_bar_zones.get(idx) { + if let Some((group_id, rect)) = tab_bar_zones.get(&idx) { return ScreenZone::TabBar { group_id: *group_id, local_x: x - rect.x as f64, @@ -368,7 +374,7 @@ pub(super) fn resolve_tab_right_click( cached_layout: &render::ScreenLayout, tab_pixel_hits: &TabPixelHitMap, frame_hit_map: Option<&quadraui::FrameHitMap>, - tab_bar_zones: &[(GroupId, quadraui::Rect)], + tab_bar_zones: &HashMap, ) -> Option<(GroupId, usize)> { use crate::core::engine::TabBarClickTarget as T; @@ -499,7 +505,7 @@ pub(super) fn handle_mouse_click( action_btn_map: &ActionBtnMap, status_segment_map: &StatusSegmentMap, frame_hit_map: Option<&quadraui::FrameHitMap>, - tab_bar_zones: &[(GroupId, quadraui::Rect)], + tab_bar_zones: &HashMap, ) -> (Option, Option) { match pixel_to_click_target( engine, @@ -603,7 +609,7 @@ pub(super) fn handle_mouse_double_click( action_btn_map: &ActionBtnMap, status_segment_map: &StatusSegmentMap, frame_hit_map: Option<&quadraui::FrameHitMap>, - tab_bar_zones: &[(GroupId, quadraui::Rect)], + tab_bar_zones: &HashMap, ) { if let ClickTarget::BufferPos(wid, line, col) = pixel_to_click_target( engine, @@ -651,7 +657,7 @@ pub(super) fn handle_mouse_drag( action_btn_map: &ActionBtnMap, status_segment_map: &StatusSegmentMap, frame_hit_map: Option<&quadraui::FrameHitMap>, - tab_bar_zones: &[(GroupId, quadraui::Rect)], + tab_bar_zones: &HashMap, ) { if let ClickTarget::BufferPos(wid, line, col) = pixel_to_click_target( engine, @@ -1135,7 +1141,7 @@ mod cross_split_drag_focus_tests { &status_segment_map, None, // no cached FrameHitMap in this test — exercises the // `screen_zone_hit_test` fallback path (#449) - &[], + &HashMap::new(), false, // mutate_focus: drag continuation ); assert_eq!( @@ -1182,7 +1188,7 @@ mod cross_split_drag_focus_tests { &action_btn_map, &status_segment_map, None, - &[], + &HashMap::new(), true, // mutate_focus: genuine click ); assert_eq!( @@ -1192,3 +1198,200 @@ mod cross_split_drag_focus_tests { assert!(matches!(click_target, ClickTarget::BufferPos(wid, _, _) if wid == wid_b)); } } + +#[cfg(test)] +mod frame_hit_map_tests { + //! #449 regression: `frame_zone_to_screen_zone` and the `frame_hit_map` + //! branch of `pixel_to_click_target` are the actual mechanism this issue + //! introduced — mapping `quadraui::FrameZone::TabBar { idx }` / + //! `FrameZone::Editor { idx }` back to `render::ScreenZone`. These tests + //! build a *real* `quadraui::FrameHitMap` via `ScreenLayout::hit_map()` + //! (quadraui#425, landed as `c316f15`) from the same `Surface::Editor` / + //! `Surface::TabBar` construction `App::render_content` uses + //! (`src/gtk/mod.rs` ~7712-7830), instead of exercising only the + //! pre-existing `screen_zone_hit_test` fallback (as the two tests in + //! `cross_split_drag_focus_tests` above do by passing `None, &[]`). + use super::*; + use quadraui::{ScreenLayout as QSL, Surface}; + + /// Lay out a single window / single (unsplit) tab bar and build the + /// `FrameHitMap` + `tab_bar_zones` table the way `render_content` does + /// (`src/gtk/mod.rs` ~7712-7839), so these tests exercise the production + /// construction, not a hand-rolled stand-in — crucially including the + /// same "editors pushed first, tab bars after" ordering, since + /// `FrameZone::TabBar { idx }` carries the *global* surface index across + /// that whole `ScreenLayout`, not a per-tab-bar position. `tab_bar_zones` + /// must therefore be keyed by that same global index, not `0..`. + fn build_hit_map( + engine: &Engine, + theme: &Theme, + line_height: f64, + char_width: f64, + ) -> ( + render::ScreenLayout, + quadraui::FrameHitMap, + HashMap, + ) { + let bounds = WindowRect::new(0.0, 0.0, 800.0, 600.0); + let (rects, _) = engine.calculate_group_window_rects(bounds, (line_height * 1.6).ceil()); + let screen = build_screen_layout(engine, theme, &rects, line_height, char_width, false); + + let window_editors: Vec = + screen.windows.iter().map(render_mod::to_q_editor).collect(); + let mut hit_frame = QSL::new(); + for editor in &window_editors { + hit_frame.push(Surface::Editor { + rect: editor.rect, + editor, + }); + } + + let tab_row_h = render_mod::tab_row_height_px(line_height); + let tab_bar_h = render_mod::tab_bar_height_px(line_height, engine.settings.breadcrumbs); + let mut tab_bar_zones: HashMap = HashMap::new(); + for (next_surface_idx, target) in + (window_editors.len()..).zip(render_mod::tab_bar_draw_targets( + engine, + &screen, + tab_row_h, + tab_bar_h, + (0.0, 0.0), + (0.0, 0.0, 800.0), + )) + { + hit_frame.push(Surface::TabBar { + rect: target.rect, + bar: target.bar, + hovered_close: None, + }); + tab_bar_zones.insert(next_surface_idx, (target.group_id, target.rect)); + } + + let hit_map = hit_frame.hit_map(); + (screen, hit_map, tab_bar_zones) + } + + #[test] + fn frame_zone_to_screen_zone_resolves_editor_point() { + let mut engine = Engine::new(); + engine.buffer_mut().insert(0, "hello world"); + let wid = engine.active_window_id(); + let theme = Theme::onedark(); + let line_height = 18.0; + let char_width = 9.0; + + let (screen, hit_map, tab_bar_zones) = + build_hit_map(&engine, &theme, line_height, char_width); + let rw = screen + .windows + .first() + .expect("single window should be laid out"); + let x = rw.rect.x + char_width * 2.0; + let y = rw.rect.y + line_height * 2.0; + + let zone = frame_zone_to_screen_zone(&hit_map, &tab_bar_zones, &screen, x, y); + match zone { + ScreenZone::Window { + window_id, + window_idx, + .. + } => { + assert_eq!(window_id, wid); + assert_eq!(window_idx, 0); + } + other => panic!("expected ScreenZone::Window from the FrameHitMap path, got {other:?}"), + } + } + + #[test] + fn frame_zone_to_screen_zone_resolves_tab_bar_point() { + let mut engine = Engine::new(); + engine.buffer_mut().insert(0, "hello world"); + let group_id = engine.active_group; + let theme = Theme::onedark(); + let line_height = 18.0; + let char_width = 9.0; + + let (screen, hit_map, tab_bar_zones) = + build_hit_map(&engine, &theme, line_height, char_width); + assert!( + !tab_bar_zones.is_empty(), + "single-group mode should still push one tab bar surface" + ); + let (zone_group, rect) = *tab_bar_zones + .values() + .next() + .expect("just asserted tab_bar_zones is non-empty"); + assert_eq!(zone_group, group_id); + let x = rect.x as f64 + 2.0; + let y = rect.y as f64 + 2.0; + + let zone = frame_zone_to_screen_zone(&hit_map, &tab_bar_zones, &screen, x, y); + match zone { + ScreenZone::TabBar { + group_id: resolved, .. + } => assert_eq!(resolved, group_id), + other => { + panic!("expected ScreenZone::TabBar from the FrameHitMap path, got {other:?}") + } + } + } + + #[test] + fn pixel_to_click_target_consults_the_cached_frame_hit_map_for_editor_clicks() { + // Proves `pixel_to_click_target` actually takes the `frame_hit_map` + // branch (not silently falling through to `screen_zone_hit_test`) + // when a real `Some(&FrameHitMap)` is supplied — the production + // `Some` path exercised by `render_content`'s cached hit map, as + // opposed to the `None` fallback path already covered by + // `cross_split_drag_focus_tests`. + let mut engine = Engine::new(); + engine.buffer_mut().insert(0, "hello world"); + let wid = engine.active_window_id(); + let theme = Theme::onedark(); + let line_height = 18.0; + let char_width = 9.0; + + let (screen, hit_map, tab_bar_zones) = + build_hit_map(&engine, &theme, line_height, char_width); + let rw = screen + .windows + .first() + .expect("single window should be laid out"); + let x = rw.rect.x + char_width * 2.0; + let y = rw.rect.y + line_height * 2.0; + + let backend = Rc::new(RefCell::new(super::super::backend::GtkBackend::new())); + let empty_pixel_hits: TabPixelHitMap = HashMap::new(); + let empty_slots: TabSlotMap = HashMap::new(); + let empty_diff: DiffBtnMap = HashMap::new(); + let empty_split: SplitBtnMap = HashMap::new(); + let empty_action: ActionBtnMap = HashMap::new(); + let empty_status: StatusSegmentMap = HashMap::new(); + + let target = pixel_to_click_target( + &mut engine, + &backend, + x, + y, + line_height, + char_width, + &screen, + &empty_pixel_hits, + &empty_slots, + &empty_diff, + &empty_split, + &empty_action, + &empty_status, + Some(&hit_map), + &tab_bar_zones, + true, + ); + match target { + ClickTarget::BufferPos(id, _, _) => assert_eq!(id, wid), + other => panic!( + "expected a BufferPos hit resolved via the cached FrameHitMap, got {other:?}" + ), + } + } +} diff --git a/src/gtk/mod.rs b/src/gtk/mod.rs index b8e6a3de..a7169bdd 100644 --- a/src/gtk/mod.rs +++ b/src/gtk/mod.rs @@ -590,11 +590,19 @@ struct App { /// breadcrumb/divider zones (which have no `FrameZone` equivalent) and /// for the brief window before the first paint populates this cache. cached_frame_hit_map: Rc>>, - /// Parallel table for resolving `FrameZone::TabBar { idx }`: `idx` - /// indexes this Vec in the same order tab bars were pushed into - /// `cached_frame_hit_map`, recovering the owning `GroupId` and drawn - /// rect that `FrameZone` itself doesn't carry. - cached_tab_bar_zones: Rc>>, + /// Parallel table for resolving `FrameZone::TabBar { idx }`, keyed by the + /// *global* surface index `FrameZone::TabBar { idx }` actually carries — + /// `ScreenLayout::zone_for`'s `idx` enumerates ALL surfaces pushed into + /// `cached_frame_hit_map` (editors THEN tab bars), not a per-tab-bar + /// count, so a tab bar's global index is offset by however many editor + /// surfaces were pushed before it. A plain `Vec` indexed 0.. (the + /// original #449 shape) silently mismatched by that offset and made + /// every `FrameZone::TabBar` lookup miss whenever at least one editor + /// window was on screen — i.e. always — falling back to + /// `screen_zone_hit_test` without ever exercising the new path. Keying + /// by the real global `idx` instead of position fixes that regardless + /// of how many editor windows precede the tab bars. + cached_tab_bar_zones: Rc>>, /// Per-group tab-drop geometry (absolute pixel bounds) computed each frame in /// `render_content`. Both the drag overlay (same frame) and the drag hit-test /// in `handle_mouse_drag_msg` (next mouse-move) read this, so the drop-zone @@ -1401,7 +1409,7 @@ impl App { status_segment_map: Rc::new(RefCell::new(HashMap::new())), cached_screen_layout: Rc::new(RefCell::new(None)), cached_frame_hit_map: Rc::new(RefCell::new(None)), - cached_tab_bar_zones: Rc::new(RefCell::new(Vec::new())), + cached_tab_bar_zones: Rc::new(RefCell::new(HashMap::new())), cached_drop_groups: Rc::new(RefCell::new(Vec::new())), cached_drop_tbh: Rc::new(Cell::new(0.0)), cached_explorer_metrics: Rc::new(Cell::new((16.0, 8.0))), @@ -7780,19 +7788,24 @@ impl quadraui::ShellApp for App { pixel_hits.clear(); close_abs.clear(); slots_abs.clear(); - // Parallel table for `FrameZone::TabBar { idx }` resolution (#449) — - // `idx` indexes this Vec in the same order tab bars are pushed into - // `hit_frame` below, recovering the `GroupId`/rect `FrameZone` itself - // doesn't carry. See `cached_tab_bar_zones`'s doc comment. - let mut tab_bar_zones: Vec<(core::window::GroupId, quadraui::Rect)> = Vec::new(); - for target in render::tab_bar_draw_targets( - &engine, - screen, - tab_row_h, - tab_bar_h, - (0.0, 0.0), - (x, y, w), - ) { + // Parallel table for `FrameZone::TabBar { idx }` resolution (#449), + // keyed by the *global* surface index — `hit_frame` already has + // `window_editors.len()` `Surface::Editor`s pushed into it above, so + // the first tab bar's `FrameZone::TabBar { idx }` is + // `window_editors.len()`, not `0`. See `cached_tab_bar_zones`'s doc + // 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), + )) + { let tb_rect = target.rect; let hover = self .tab_close_hover @@ -7811,7 +7824,7 @@ impl quadraui::ShellApp for App { bar: target.bar, hovered_close: hover, }); - tab_bar_zones.push((target.group_id, tb_rect)); + tab_bar_zones.insert(next_surface_idx, (target.group_id, tb_rect)); // Recover the exact pixel geometry the rasteriser just drew and // cache it (relative to the bar's left edge) for hit-testing. let hits = backend.tab_bar_layout(tb_rect, target.bar);