From 58b8dd7b901516ad24e929e3ee958b70ca23bde4 Mon Sep 17 00:00:00 2001 From: "Pedro M. de Echanove Pasquin" Date: Mon, 14 Sep 2026 11:50:27 +0200 Subject: [PATCH] Focus requests reach widgets clipped by a scroll viewport, and stop retrying after a few frames A scroll viewport keeps in `widget_rects` only the widgets at least partly inside its visible area (`draw/layout.rs`, the translation of the sub-canvas rects to surface space). A focus request whose target sits below the fold therefore never finds it: `App::take_focus_request` was honoured only for widgets already laid out on screen. The Wi-Fi page of settings is one such case by construction: tapping a secured network inserts the password form under the row and asks for focus on the field, so for a row in the lower part of the list the field is born outside the viewport and the request misses. Worse, the miss branch parked the id in `focus_retry`, marked the view dirty and requested a redraw on every iteration, so the application rebuilt and repainted at the compositor's frame rate for as long as the widget stayed hidden. The layout pass now records the widgets it drops for carrying a `WidgetId`: `DrawCtx::scroll_clipped` holds `(id, scroll flat_idx, rect)` with the rect in the scroll's content coordinates, the same frame the navigable-item list uses, and `draw_frame` hands it to `FrameState`. When a focus request misses, `AppData::reveal_clipped_widget` looks the id up in that list on the main surface and the overlays, finds the viewport in `scroll_rects`, and moves its offset on the axes the scroll allows so the rect lands inside; the next frame lays the widget out and the retry focuses it. Nested scrolls resolve one viewport per frame. The rule that computes the new offset is `scroll::offset_into_view`, extracted from the keyboard navigation in `input/keyboard/nav.rs`, which now calls it instead of carrying its own copy; unit tests cover the three cases. `focus_retry` carries a budget of drawn frames (`FOCUS_RETRY_FRAMES`, four), decremented only on iterations that actually drew, so a request made while a frame callback is pending does not lose its chance. A fresh request from the application replaces a pending retry with a full budget. When the budget runs out the request is dropped, which ends the redraw loop for a widget that never appears. Documented in the `take_focus_request` doc comment, the cookbook and the changelog; Debian revision 0.3.0-3. --- CHANGELOG.md | 1 + debian/changelog | 6 ++++++ docs/cookbook.md | 5 ++++- src/app.rs | 5 +++++ src/core.rs | 1 + src/draw/layout.rs | 3 +++ src/draw/mod.rs | 9 ++++++++- src/event_loop/app_data.rs | 5 +++-- src/event_loop/focus.rs | 26 +++++++++++++++++++++++++- src/event_loop/run.rs | 22 +++++++++++++++++----- src/event_loop/subsurface.rs | 1 + src/event_loop/surface.rs | 4 ++++ src/input/keyboard/nav.rs | 19 ++----------------- src/widget/scroll/mod.rs | 15 +++++++++++++++ src/widget/scroll/tests.rs | 26 +++++++++++++++++++++++++- 15 files changed, 120 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa81438..d8fc91f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ All notable changes to `ltk` are documented here. The format is based on [Keep a ### Fixed +- **A focus request reaches a widget scrolled out of view.** Scroll viewports drop the widgets outside their visible area from the hit-test list, so `App::take_focus_request` for a text input that a tap had just added below the fold (a password field under the selected network of a Wi-Fi list) never found it — and the runtime kept re-requesting a rebuild and a redraw every frame while it waited. Widgets with a `WidgetId` that a scroll clips are now recorded with their content position (`scroll_clipped`); a request whose target is among them moves that viewport with the same rule keyboard navigation uses (`scroll::offset_into_view`) and is satisfied on the next frame. The wait is bounded to a few drawn frames, after which the request is dropped instead of spinning. - **Fast two-thumb typing no longer drops taps.** A finger landing while another was still down went to the auxiliary slot and never reached the gesture machine, so on a touch keypad every overlapping tap was lost. A finger landing on a primary that is still a plain press (`GestureState::is_plain_press`: no drag, scroll, swipe or slider in flight) now commits that press as a tap at its last position and takes the primary slot; the earlier finger's release reports through `App::on_touch_up`. Presses that have become a drag keep the old behaviour, so a second finger during a pattern stroke or a slider drag still stays out of the gesture machine. - **`PatternGrid::on_node` fires once per node, not once per motion event.** Every touch motion within a node's snap radius re-emitted the node's message and requested a repaint, so drawing a pattern rebuilt and re-rastered the surface at the touchscreen's motion rate. The gesture machine now remembers the node the finger is over (`last_pattern_node`) and emits only on change; a `Slider` move outcome without a message no longer requests a redraw. - **A tap on a button survives finger motion inside the button.** Any travel past the slop turned the press into a vertical or horizontal drag and its release skipped the button, so a thumb that rolled on a large key lost the tap. While the pressed widget fires on press (`WidgetHandlers::has_press`) and the finger is still inside its rect, motion is ignored; the swipe machinery takes over from the original press point once the finger leaves, and presses inside a scroll viewport are unaffected. diff --git a/debian/changelog b/debian/changelog index 49ec0ef..59fa06e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +ltk (0.3.0-3) unstable; urgency=low + + * Focus requests reach widgets clipped by a scroll viewport: the runtime moves the viewport to reveal the target and waits a bounded number of frames instead of redrawing indefinitely. + + -- Pedro M. de Echanove Pasquin Mon, 14 Sep 2026 12:00:00 +0200 + ltk (0.3.0-2) unstable; urgency=low * PatternGrid: a 3×3 drag widget for pattern unlock that reports the node under the pointer and the release; the app owns the path. Theme tokens under widget/pattern_grid/theme.rs, AccessKit GenericContainer role, docs, tests and a Security tab in the widgets example. diff --git a/docs/cookbook.md b/docs/cookbook.md index a1c902e..73c127a 100644 --- a/docs/cookbook.md +++ b/docs/cookbook.md @@ -721,7 +721,10 @@ impl App for LoginApp `take_focus_request` is consumed once: the runtime dispatches focus to the returned id and the next call returns `None` because the slot was -drained. The application owns "when should focus move". +drained. The application owns "when should focus move". The target may +be added to the view by the same `update` that requests it, and may sit +inside a `scroll` below the fold: the runtime waits a few frames for it +and moves the viewport so the widget is visible when it takes focus. Tab and Shift+Tab traverse focusable widgets in declaration order — no opt-in needed beyond the widget being interactive. diff --git a/src/app.rs b/src/app.rs index 7b68e38..a707f03 100644 --- a/src/app.rs +++ b/src/app.rs @@ -736,6 +736,11 @@ pub trait App: 'static /// Return `Some(id)` once to focus the widget with that [`WidgetId`](crate::types::WidgetId). /// The app must return `None` after the first call. + /// + /// The runtime waits a few drawn frames for the widget to appear, so a + /// request made from the `update` that adds the widget to the view is + /// honoured, and a target scrolled out of a viewport is brought into + /// view before it is focused. A widget that never appears is dropped. fn take_focus_request( &mut self ) -> Option { None } /// Called on every pointer motion (mouse move, button press, button diff --git a/src/core.rs b/src/core.rs index 7bdfdf9..fd60769 100644 --- a/src/core.rs +++ b/src/core.rs @@ -402,6 +402,7 @@ impl UiSurface scroll_rects: Vec::new(), scroll_canvases: std::mem::take( &mut self.scroll_canvases ), scroll_navigable_items: std::mem::take( &mut self.scroll_navigable_items ), + scroll_clipped: Vec::new(), previous_widget_rects: self.widget_rects.clone(), accessible_extras: Vec::new(), live_depth: 0, diff --git a/src/draw/layout.rs b/src/draw/layout.rs index 8f04b3c..a9040a6 100644 --- a/src/draw/layout.rs +++ b/src/draw/layout.rs @@ -303,6 +303,7 @@ pub( crate ) fn layout_and_draw( for mut w in new_rects { + let content = Rect { x: w.rect.x + ox, y: w.rect.y + oy, ..w.rect }; // Sub-canvas origin (0,0) maps to global (rect.x, rect.y). w.rect.x += rect.x; w.rect.y += rect.y; @@ -317,6 +318,8 @@ pub( crate ) fn layout_and_draw( if visible_x && visible_y { ctx.widget_rects.push( w ); + } else if let Some( id ) = w.id { + ctx.scroll_clipped.push( ( id, my_idx, content ) ); } } diff --git a/src/draw/mod.rs b/src/draw/mod.rs index a9e2b92..ecb14ed 100644 --- a/src/draw/mod.rs +++ b/src/draw/mod.rs @@ -32,7 +32,7 @@ use std::collections::HashMap; use crate::render::Canvas; -use crate::types::Rect; +use crate::types::{ Rect, WidgetId }; use crate::widget::LaidOutWidget; pub( crate ) mod software; @@ -77,6 +77,11 @@ pub( crate ) struct DrawCtx /// needed to bring an item into view without depending on the /// current scroll position. pub scroll_navigable_items: HashMap>, + /// Widgets carrying a [`WidgetId`] that a scroll viewport clipped out + /// of `widget_rects` this frame: `(id, scroll flat_idx, rect)` with the + /// rect in the scroll's content coordinates, so a focus request can + /// move that scroll until the widget is in view. + pub scroll_clipped: Vec<( WidgetId, usize, Rect )>, /// Snapshot of the previous frame's `widget_rects`. Read by /// [`crate::widget::anchored_overlay::AnchoredOverlay`] at draw time /// to look up the rect of an anchor widget by [`crate::WidgetId`] and @@ -116,6 +121,7 @@ pub( crate ) fn build_draw_ctx( scroll_rects: Vec::new(), scroll_canvases: std::mem::take( &mut frame.scroll_canvases ), scroll_navigable_items: std::mem::take( &mut frame.scroll_navigable_items ), + scroll_clipped: Vec::new(), previous_widget_rects: frame.widget_rects.clone(), accessible_extras: Vec::new(), live_depth: 0, @@ -147,6 +153,7 @@ pub( crate ) fn commit_draw_ctx( frame.scroll_offsets = ctx.scroll_offsets; frame.accessible_extras = ctx.accessible_extras; frame.scroll_navigable_items = ctx.scroll_navigable_items; + frame.scroll_clipped = ctx.scroll_clipped; } /// Paint the built-in Copy / Cut / Paste context menu on top of the diff --git a/src/event_loop/app_data.rs b/src/event_loop/app_data.rs index 051b5f7..9b0a2ce 100644 --- a/src/event_loop/app_data.rs +++ b/src/event_loop/app_data.rs @@ -271,8 +271,9 @@ pub struct AppData /// Used to drive `App::on_first_frame_committed` exactly once. pub first_frame_committed: bool, /// Focus request deferred because the target widget was absent from - /// `widget_rects` (e.g. read_only, not yet rebuilt). Retried next draw. - pub focus_retry: Option, + /// `widget_rects` (not yet rebuilt, or clipped by a scroll viewport), + /// with the number of drawn frames it may still wait for. + pub focus_retry: Option<( crate::types::WidgetId, u8 )>, /// Runtime performance guardrails (stuck-animation / poll-interval /// diagnostics and the software animation-rate cap). See [`super::perf`]. pub perf: super::perf::PerfState, diff --git a/src/event_loop/focus.rs b/src/event_loop/focus.rs index 02f2f06..d8f223c 100644 --- a/src/event_loop/focus.rs +++ b/src/event_loop/focus.rs @@ -7,8 +7,9 @@ use super::app_data::AppData; use super::surface::SurfaceFocus; use crate::app::{ App, OverlayId }; use crate::tree::find_handlers; -use crate::types::Point; +use crate::types::{ Point, WidgetId }; use crate::widget::WidgetHandlers; +use crate::widget::scroll::offset_into_view; impl AppData { @@ -184,6 +185,29 @@ impl AppData self.activate_text_input( qh, secure ); } } + + /// Scroll the viewport that clipped the widget with `id` so the widget + /// lands inside it on the next frame. No-op when no scroll reported + /// the id as clipped. + pub( crate ) fn reveal_clipped_widget( &mut self, id: WidgetId ) + { + for ss in std::iter::once( &mut self.main ).chain( self.overlays.values_mut() ) + { + let Some( &( _, scroll_idx, target ) ) = ss.frame.scroll_clipped.iter().find( |( w, _, _ )| *w == id ) else { continue }; + let Some( &( viewport, _, axis ) ) = ss.frame.scroll_rects.iter().find( |( _, i, _ )| *i == scroll_idx ) else { continue }; + let entry = ss.frame.scroll_offsets.entry( scroll_idx ).or_insert( ( 0.0, 0.0 ) ); + if axis.allows_x() + { + entry.0 = offset_into_view( entry.0, target.x, target.width, viewport.width ); + } + if axis.allows_y() + { + entry.1 = offset_into_view( entry.1, target.y, target.height, viewport.height ); + } + ss.request_redraw(); + return; + } + } } /// Whether the surface's focused widget is a text input in its current frame. diff --git a/src/event_loop/run.rs b/src/event_loop/run.rs index e883c91..6b66de9 100644 --- a/src/event_loop/run.rs +++ b/src/event_loop/run.rs @@ -35,6 +35,10 @@ use super::frame::draw_frame; use super::invalidation::apply_invalidation; use super::overlays_reconcile::reconcile_overlays; +/// Drawn frames a focus request waits for its target to appear: one for +/// the rebuilt view, one per scroll viewport moved to reveal the widget. +const FOCUS_RETRY_FRAMES: u8 = 4; + /// Run the application, panicking on init failure. Thin wrapper over /// [`try_run`] kept for backwards-compatibility — embedders that need /// to recover from a missing compositor or a stripped-down driver @@ -903,8 +907,11 @@ pub( crate ) fn try_run( mut app: A ) -> Result<(), RunError> // Walks main + every overlay because the targeted widget can live // on any of them (a search field on a launcher overlay, a text // edit on a dialog modal, …). - let focus_id = data.app.take_focus_request().or_else( || data.focus_retry.take() ); - if let Some( id ) = focus_id + let retry = data.focus_retry.take(); + let focus_request = data.app.take_focus_request() + .map( |id| ( id, FOCUS_RETRY_FRAMES ) ) + .or( retry ); + if let Some( ( id, frames_left ) ) = focus_request { let mut hit: Option<( SurfaceFocus, usize )> = data.main.frame.widget_rects.iter() .find( |w| w.id == Some( id ) ) @@ -936,9 +943,14 @@ pub( crate ) fn try_run( mut app: A ) -> Result<(), RunError> } } } else { - data.focus_retry = Some( id ); - data.view_dirty = true; - data.main.request_redraw(); + let frames_left = if any_drawable { frames_left.saturating_sub( 1 ) } else { frames_left }; + if frames_left > 0 + { + data.reveal_clipped_widget( id ); + data.focus_retry = Some( ( id, frames_left ) ); + data.view_dirty = true; + data.main.request_redraw(); + } } } } diff --git a/src/event_loop/subsurface.rs b/src/event_loop/subsurface.rs index 2432445..675b5cb 100644 --- a/src/event_loop/subsurface.rs +++ b/src/event_loop/subsurface.rs @@ -234,6 +234,7 @@ fn empty_draw_ctx() -> DrawCtx scroll_rects: Vec::new(), scroll_canvases: HashMap::new(), scroll_navigable_items: HashMap::new(), + scroll_clipped: Vec::new(), previous_widget_rects: Vec::new(), accessible_extras: Vec::new(), live_depth: 0, diff --git a/src/event_loop/surface.rs b/src/event_loop/surface.rs index 05a524a..06b74ef 100644 --- a/src/event_loop/surface.rs +++ b/src/event_loop/surface.rs @@ -177,6 +177,9 @@ pub( crate ) struct FrameState /// `hovered_idx` item-by-item without regenerating the layout pass. Y is in /// pre-translation, pre-offset content coordinates. pub scroll_navigable_items: HashMap>, + /// Widgets with a [`WidgetId`] clipped out of `widget_rects` by a scroll + /// viewport this frame: `(id, scroll flat_idx, rect)` in content coordinates. + pub scroll_clipped: Vec<( WidgetId, usize, Rect )>, /// Previous frame interaction state for damage tracking. pub prev_focused: Option, pub prev_hovered: Option, @@ -197,6 +200,7 @@ impl FrameState scroll_rects: Vec::new(), scroll_canvases: HashMap::new(), scroll_navigable_items: HashMap::new(), + scroll_clipped: Vec::new(), prev_focused: None, prev_hovered: None, prev_pressed: None, diff --git a/src/input/keyboard/nav.rs b/src/input/keyboard/nav.rs index 0cd7f3a..6354744 100644 --- a/src/input/keyboard/nav.rs +++ b/src/input/keyboard/nav.rs @@ -3,6 +3,7 @@ use crate::app::App; use crate::event_loop::{ AppData, SurfaceFocus }; +use crate::widget::scroll::offset_into_view; impl AppData { @@ -42,28 +43,12 @@ impl AppData }; let ( new_idx, content_y, content_h ) = items[ next_pos ]; - // Auto-scroll on the Y axis to bring the new item fully into view. - // Item Y is in pre-offset content coordinates, so the offset that - // places the item flush with the top of the viewport is `content_y`, - // and flush with the bottom is `content_y + content_h - viewport_h`. // Keyboard navigation only steps on the navigable-item axis (vertical // list-style); the X offset is preserved as-is. - let viewport_h = scroll_rect.height; let ( current_x, current_y ) = self.surface( focus ) .frame.scroll_offsets.get( &scroll_idx ) .copied().unwrap_or( ( 0.0, 0.0 ) ); - let new_y = if content_y < current_y - { - content_y - } - else if content_y + content_h > current_y + viewport_h - { - ( content_y + content_h - viewport_h ).max( 0.0 ) - } - else - { - current_y - }; + let new_y = offset_into_view( current_y, content_y, content_h, scroll_rect.height ); let ss = self.surface_mut( focus ); ss.hovered_idx = Some( new_idx ); diff --git a/src/widget/scroll/mod.rs b/src/widget/scroll/mod.rs index c502571..61b4191 100644 --- a/src/widget/scroll/mod.rs +++ b/src/widget/scroll/mod.rs @@ -152,6 +152,21 @@ pub(crate) fn clamp_offset( offset: f32, content_h: f32, viewport_h: f32 ) -> f3 offset.clamp( 0.0, max ) } +/// Smallest move of a 1-D scroll offset that brings `[start, start + len)` +/// (content coordinates) inside a viewport of `viewport` pixels. An item +/// already in view leaves the offset untouched. +pub( crate ) fn offset_into_view( offset: f32, start: f32, len: f32, viewport: f32 ) -> f32 +{ + if start < offset + { + start + } else if start + len > offset + viewport { + ( start + len - viewport ).max( 0.0 ) + } else { + offset + } +} + /// Create a scrollable viewport wrapping `child`. Defaults to vertical /// scrolling; chain `Scroll::horizontal` or `Scroll::both` to /// switch axes. diff --git a/src/widget/scroll/tests.rs b/src/widget/scroll/tests.rs index 9edc0b8..6a4b27c 100644 --- a/src/widget/scroll/tests.rs +++ b/src/widget/scroll/tests.rs @@ -1,7 +1,7 @@ // SPDX-License-Identifier: LGPL-2.1-only // Copyright (C) 2026 Liberux Labs, S. L. -use super::clamp_offset; +use super::{ clamp_offset, offset_into_view }; #[ test ] fn offset_zero_when_content_fits() @@ -48,3 +48,27 @@ fn content_equal_to_viewport_gives_zero_max() // No overflow — max = 0 assert_eq!( clamp_offset( 10.0, 400.0, 400.0 ), 0.0 ); } + +#[ test ] +fn into_view_keeps_offset_for_visible_item() +{ + assert_eq!( offset_into_view( 100.0, 150.0, 40.0, 300.0 ), 100.0 ); +} + +#[ test ] +fn into_view_aligns_item_above_to_top() +{ + assert_eq!( offset_into_view( 100.0, 60.0, 40.0, 300.0 ), 60.0 ); +} + +#[ test ] +fn into_view_aligns_item_below_to_bottom() +{ + assert_eq!( offset_into_view( 100.0, 500.0, 40.0, 300.0 ), 240.0 ); +} + +#[ test ] +fn into_view_tall_item_aligns_to_top() +{ + assert_eq!( offset_into_view( 50.0, 0.0, 400.0, 300.0 ), 0.0 ); +}