Skip to content

Commit 45a01f5

Browse files
authored
Merge pull request #116 from pragmatrix/keyboard-and-hover-rect
Improve the hover rect
2 parents bbfd534 + a3ac0fd commit 45a01f5

7 files changed

Lines changed: 117 additions & 43 deletions

File tree

.github/copilot-instructions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ Update it whenever you learn something new about the project's patterns, convent
3838
- When code guarantees an invariant, avoid defensive fallback branches for that path; keep the direct path and fail explicitly if the invariant is violated.
3939
- For purely defensive invariant checks on hot paths, prefer debug-only assertions to avoid unnecessary release-build work.
4040
- For platform-specific window commands, detect shortcuts where aggregated input state is available and keep the actual window mutation in the shell/window abstraction.
41+
- When multiple transient affordances represent the same interaction mode, keep them behind one shared state instead of parallel flags.
42+
- Cache repeated window state requests at the caller when the underlying platform mutation may hop to the main thread or otherwise be non-trivial.
4143
- On macOS, prefer native menu selector wiring for commands that users can remap in App Shortcuts, instead of hardcoded key-chord matching.
4244

4345
## Testing

desktop/src/desktop.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ pub struct Desktop {
2828
scene: Scene,
2929
renderer: AsyncWindowRenderer,
3030
window: ShellWindow,
31+
cursor_visible: bool,
3132
system: DesktopSystem,
3233

3334
event_manager: EventManager<ViewEvent>,
@@ -132,6 +133,7 @@ impl Desktop {
132133
scene,
133134
renderer,
134135
window,
136+
cursor_visible: true,
135137
system,
136138
event_manager,
137139
instance_manager,
@@ -153,13 +155,21 @@ impl Desktop {
153155
match event {
154156
ShellEvent::WindowEvent(_window_id, window_event) => {
155157
if let Some(view_event) = ViewEvent::from_window_event(&window_event)
156-
&& let Some(input_event) = self.event_manager.add_event(view_event, Instant::now()) {
157-
let cmd = self.system.process_input_event(
158+
&& let Some(input_event) =
159+
self.event_manager.add_event(view_event, Instant::now())
160+
{
161+
let cmd = self.system.process_input_event(
158162
&input_event,
159163
&self.instance_manager,
160164
self.renderer.geometry(),
161165
)?;
162-
self.system.transact(cmd, &self.scene, &mut self.instance_manager)?;
166+
let cursor_visible = self.system.cursor_visible();
167+
if self.cursor_visible != cursor_visible {
168+
self.window.set_cursor_visible(cursor_visible);
169+
self.cursor_visible = cursor_visible;
170+
}
171+
self.system
172+
.transact(cmd, &self.scene, &mut self.instance_manager)?;
163173

164174
let allow_camera_movements = !input_event.any_buttons_pressed();
165175
self.system.update_effects(true, allow_camera_movements)?;

desktop/src/desktop_system.rs

Lines changed: 67 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88
//! The goal here is to remove as much as possible from the specific instances into separate systems
99
//! and aggregates that are event driven.
1010
11-
use std::cmp::max;
12-
use std::collections::HashSet;
13-
1411
use anyhow::{Result, anyhow, bail};
1512
use derive_more::{Debug, From};
1613
use log::warn;
14+
use std::cmp::max;
15+
use std::collections::HashSet;
16+
use std::time::Duration;
1717
use winit::event::ElementState;
1818
use winit::keyboard::{Key, NamedKey};
1919

@@ -49,7 +49,8 @@ use crate::send_transition::{SendTransition, convert_to_send_transitions};
4949
use crate::{DesktopEnvironment, DirectionBias, EventRouter, Map, OrderedHierarchy, navigation};
5050

5151
const SECTION_SPACING: u32 = 20;
52-
52+
const POINTER_FEEDBACK_REENABLE_MIN_DISTANCE_PX: f64 = 24.0;
53+
const POINTER_FEEDBACK_REENABLE_MAX_DURATION: Duration = Duration::from_millis(200);
5354
/// This enum specifies a unique target inside the navigation and layout history.
5455
#[derive(Debug, Clone, PartialEq, Eq, Hash, From)]
5556
pub enum DesktopTarget {
@@ -114,6 +115,7 @@ pub struct DesktopSystem {
114115

115116
event_router: EventRouter<DesktopTarget>,
116117
camera: Animated<PixelCamera>,
118+
pointer_feedback_enabled: bool,
117119

118120
#[debug(skip)]
119121
layouter: IncrementalLayouter<DesktopTarget, 2>,
@@ -178,6 +180,7 @@ impl DesktopSystem {
178180

179181
event_router,
180182
camera: scene.animated(PixelCamera::default()),
183+
pointer_feedback_enabled: true,
181184
layouter,
182185

183186
aggregates: Aggregates::new(OrderedHierarchy::default(), project_presenter),
@@ -452,41 +455,78 @@ impl DesktopSystem {
452455
self.camera.value()
453456
}
454457

458+
pub fn cursor_visible(&self) -> bool {
459+
self.pointer_feedback_enabled
460+
}
461+
455462
pub fn process_input_event(
456463
&mut self,
457464
event: &Event<ViewEvent>,
458465
instance_manager: &InstanceManager,
459466
render_geometry: &RenderGeometry,
460467
) -> Result<Cmd> {
461468
let keyboard_cmd = self.preprocess_keyboard_input(event)?;
462-
if !keyboard_cmd.is_none() {
463-
return Ok(keyboard_cmd);
464-
}
465469

466-
let hit_tester = AggregateHitTester::new(
467-
&self.aggregates.hierarchy,
468-
&self.layouter,
469-
&self.aggregates.launchers,
470-
&self.aggregates.instances,
471-
render_geometry,
472-
);
470+
let cmd = if !keyboard_cmd.is_none() {
471+
keyboard_cmd
472+
} else {
473+
let hit_tester = AggregateHitTester::new(
474+
&self.aggregates.hierarchy,
475+
&self.layouter,
476+
&self.aggregates.launchers,
477+
&self.aggregates.instances,
478+
render_geometry,
479+
);
473480

474-
// Capture focus before routing the event so we can detect focus transitions afterwards.
475-
let focused_before = self.event_router.focused().cloned();
476-
let transitions = self.event_router.process(event, &hit_tester)?;
477-
// Read focus after routing and immediately recompute launcher layouts when it changed.
478-
let focused_after = self.event_router.focused().cloned();
479-
self.apply_launcher_layout_for_focus_change(focused_before, focused_after, true);
481+
let transitions = self.event_router.process(event, &hit_tester)?;
482+
if let Some((from, to)) = transitions.keyboard_focus_change() {
483+
self.apply_launcher_layout_for_focus_change(from.cloned(), to.cloned(), true);
484+
}
480485

481-
self.forward_event_transitions(transitions, instance_manager)
486+
self.forward_event_transitions(transitions, instance_manager)?
487+
};
488+
489+
self.update_pointer_feedback(event);
490+
491+
Ok(cmd)
492+
}
493+
494+
fn update_pointer_feedback(&mut self, event: &Event<ViewEvent>) {
495+
match (self.pointer_feedback_enabled, event.event()) {
496+
(
497+
true,
498+
ViewEvent::KeyboardInput {
499+
event: key_event, ..
500+
},
501+
) if key_event.state == ElementState::Pressed && !key_event.repeat => {
502+
self.pointer_feedback_enabled = false;
503+
self.aggregates.project_presenter.set_hover_rect(None);
504+
}
505+
(false, ViewEvent::MouseInput { .. } | ViewEvent::MouseWheel { .. }) => {
506+
self.pointer_feedback_enabled = true;
507+
let pointer_focus = self.event_router.pointer_focus().cloned();
508+
self.sync_hover_rect_to_pointer_path(pointer_focus.as_ref());
509+
}
510+
(false, ViewEvent::CursorMoved { .. })
511+
if event.cursor_has_velocity(
512+
POINTER_FEEDBACK_REENABLE_MIN_DISTANCE_PX,
513+
POINTER_FEEDBACK_REENABLE_MAX_DURATION,
514+
) =>
515+
{
516+
self.pointer_feedback_enabled = true;
517+
let pointer_focus = self.event_router.pointer_focus().cloned();
518+
self.sync_hover_rect_to_pointer_path(pointer_focus.as_ref());
519+
}
520+
_ => {}
521+
}
482522
}
483523

484524
fn focus(&mut self, target: &DesktopTarget, instance_manager: &InstanceManager) -> Result<Cmd> {
485525
// Focus changes can alter launcher layout targets.
486-
let focused_before = self.event_router.focused().cloned();
487526
let transitions = self.event_router.focus(target);
488-
let focused_after = self.event_router.focused().cloned();
489-
self.apply_launcher_layout_for_focus_change(focused_before, focused_after, true);
527+
if let Some((from, to)) = transitions.keyboard_focus_change() {
528+
self.apply_launcher_layout_for_focus_change(from.cloned(), to.cloned(), true);
529+
}
490530
self.forward_event_transitions(transitions, instance_manager)
491531
}
492532

@@ -923,7 +963,9 @@ impl DesktopSystem {
923963
transitions: EventTransitions<DesktopTarget>,
924964
instance_manager: &InstanceManager,
925965
) -> Result<Cmd> {
926-
if let Some(pointer_focus) = transitions.pointer_focus_target() {
966+
if self.pointer_feedback_enabled
967+
&& let Some(pointer_focus) = transitions.pointer_focus_target()
968+
{
927969
self.sync_hover_rect_to_pointer_path(pointer_focus);
928970
}
929971

desktop/src/event_router.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,13 @@ impl<T> Default for EventTransitions<T> {
370370
}
371371

372372
impl<T> EventTransitions<T> {
373+
pub fn keyboard_focus_change(&self) -> Option<(Option<&T>, Option<&T>)> {
374+
self.0.iter().find_map(|transition| match transition {
375+
EventTransition::ChangeKeyboardFocus { from, to } => Some((from.as_ref(), to.as_ref())),
376+
_ => None,
377+
})
378+
}
379+
373380
pub fn pointer_focus_target(&self) -> Option<Option<&T>> {
374381
self.0.iter().find_map(|transition| match transition {
375382
EventTransition::ChangePointerFocus { to, .. } => Some(to.as_ref()),

desktop/src/projects/project_presenter.rs

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,20 @@ pub struct ProjectPresenter {
2121
// Idea: Use a type that combines Alpha with another Interpolatable type.
2222
// Robustness: Alpha should be a type.
2323
hover_alpha: Animated<f32>,
24-
hover_rect: Animated<Rect>,
24+
hover_rect: Rect,
2525
// Idea: can't we just animate a visual / Handle<Visual>?
2626
// Performance: This is a visual that _always_ lives inside the renderer, even though it does not contain a single shape when alpha = 0.0
2727
hover_visual: Handle<Visual>,
2828
}
2929

3030
impl ProjectPresenter {
3131
const HOVER_STROKE: (f64, f64) = (10.0, 10.0);
32+
3233
pub fn new(location: Handle<Location>, scene: &Scene) -> Self {
3334
Self {
3435
location: location.clone(),
3536
hover_alpha: scene.animated(0.0),
36-
hover_rect: scene.animated(Rect::ZERO),
37+
hover_rect: Rect::ZERO,
3738
hover_visual: create_hover_shapes(None)
3839
.into_visual()
3940
.at(location)
@@ -46,23 +47,13 @@ impl ProjectPresenter {
4647
pub fn set_hover_rect(&mut self, rect: Option<Rect>) {
4748
match rect {
4849
Some(rect) => {
49-
let was_visible = self.hover_alpha.final_value() == 1.0;
50-
5150
self.hover_alpha.animate_if_changed(
5251
1.0,
5352
Self::HOVER_ANIMATION_DURATION,
5453
Interpolation::CubicOut,
5554
);
5655

57-
if was_visible {
58-
self.hover_rect.animate_if_changed(
59-
rect,
60-
Self::HOVER_ANIMATION_DURATION,
61-
Interpolation::CubicOut,
62-
);
63-
} else {
64-
self.hover_rect.set_immediately(rect);
65-
}
56+
self.hover_rect = rect;
6657
}
6758
None => {
6859
self.hover_alpha.animate_if_changed(
@@ -76,7 +67,7 @@ impl ProjectPresenter {
7667

7768
pub fn apply_animations(&mut self) {
7869
let alpha = self.hover_alpha.value();
79-
let rect_alpha = (alpha != 0.0).then(|| (self.hover_rect.value(), alpha));
70+
let rect_alpha = (alpha != 0.0).then_some((self.hover_rect, alpha));
8071

8172
// Ergonomics: What something like apply_to_if_changed(&mut self.hover_visual) or so?
8273
//

input/src/event.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use massive_geometry::{Point, Vector};
77
use winit::keyboard::ModifiersState;
88

99
use crate::event_aggregator::DeviceStates;
10-
use crate::event_history::{EventHistory, EventRecord};
10+
use crate::event_history::{EventHistory, EventRecord, HistoryIterator};
1111
use crate::tracker::Movement;
1212
use crate::{AggregationEvent, ButtonSensor, InputEvent, MouseGesture, PointingDeviceState};
1313

@@ -98,6 +98,23 @@ impl<'history, E: InputEvent> Event<'history, E> {
9898
}
9999
}
100100

101+
/// Returns `true` when the current cursor move covers at least `min_distance` within
102+
/// `max_duration`.
103+
pub fn cursor_has_velocity(&self, min_distance: f64, max_duration: Duration) -> bool {
104+
let Some(device) = self.cursor_moved() else {
105+
return false;
106+
};
107+
let Some(pos) = self.device_pos(device) else {
108+
return false;
109+
};
110+
111+
self.history
112+
.iter()
113+
.until(self.time() - max_duration)
114+
.max_distance_moved(device, pos)
115+
>= min_distance
116+
}
117+
101118
pub fn detect_click(&self, mouse_button: MouseButton) -> Option<Point> {
102119
match self.to_aggregation_event()? {
103120
AggregationEvent::MouseInput { state, button, .. }

shell/src/shell_window.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ impl ShellWindow {
5959
self.shared.window().set_cursor(icon);
6060
}
6161

62+
// Note: This may schedule work on the main thread, so callers should avoid redundant updates.
63+
pub fn set_cursor_visible(&self, visible: bool) {
64+
self.shared.window().set_cursor_visible(visible);
65+
}
66+
6267
// DI: Use SizeI to represent initial_size.
6368
pub fn renderer(&self) -> WindowRendererBuilder {
6469
WindowRendererBuilder::new(self.shared.clone())

0 commit comments

Comments
 (0)