From 42840b950c8cf1e62bd7556c0171dc86d549915f Mon Sep 17 00:00:00 2001 From: Jonathan Johnson Date: Wed, 15 Nov 2023 12:39:32 -0800 Subject: [PATCH] Hover updates after widget removal This also fixes some inconsistencies that arose when the focus widget was "stuck" on a removed widget. Button previously handled it hackily in a redraw function, but now Gooey handles it automatically without needing to wait for a repaint. --- examples/tic-tac-toe.rs | 11 ++-- src/context.rs | 112 ++++++++++++++++++++++++++-------------- src/tree.rs | 42 ++++++++------- src/widgets/button.rs | 5 -- src/window.rs | 104 +++++++++++++++++-------------------- 5 files changed, 154 insertions(+), 120 deletions(-) diff --git a/examples/tic-tac-toe.rs b/examples/tic-tac-toe.rs index 8a31892..b28de4c 100644 --- a/examples/tic-tac-toe.rs +++ b/examples/tic-tac-toe.rs @@ -141,9 +141,14 @@ fn game_end(winner: Option, app: &Dynamic) -> impl MakeWidget }; label - .and("Play Again".into_button().on_click(move |_| { - app.set(AppState::Playing); - })) + .and( + "Play Again" + .into_button() + .on_click(move |_| { + app.set(AppState::Playing); + }) + .into_default(), + ) .into_rows() .centered() .expand() diff --git a/src/context.rs b/src/context.rs index 94a301b..0cb8ae0 100644 --- a/src/context.rs +++ b/src/context.rs @@ -21,7 +21,7 @@ use crate::utils::IgnorePoison; use crate::value::{Dynamic, IntoValue, Value}; use crate::widget::{EventHandling, ManagedWidget, WidgetId, WidgetInstance, WidgetRef}; use crate::window::sealed::WindowCommand; -use crate::window::{RunningWindow, ThemeMode}; +use crate::window::{CursorState, RunningWindow, ThemeMode}; use crate::ConstraintLimit; /// A context to an event function. @@ -185,7 +185,10 @@ impl<'context, 'window> EventContext<'context, 'window> { let mut activation_changes = 0; while activation_changes < MAX_ITERS { - let active = self.pending_state.active.clone(); + let active = self + .pending_state + .active + .and_then(|w| self.current_node.tree.widget(w)); if self.current_node.tree.active_widget() == active.as_ref().map(|w| w.node_id) { break; } @@ -202,13 +205,16 @@ impl<'context, 'window> EventContext<'context, 'window> { Err(()) => false, }; if new { - if let Some(active) = self.pending_state.active.clone() { + let active = self + .pending_state + .active + .and_then(|w| self.current_node.tree.widget(w)); + if let Some(active) = &active { active .lock() .as_widget() - .activate(&mut self.for_other(&active)); + .activate(&mut self.for_other(active)); } - self.pending_state.active = active; } else { break; } @@ -222,7 +228,10 @@ impl<'context, 'window> EventContext<'context, 'window> { let mut focus_changes = 0; while focus_changes < MAX_ITERS { - let focus = self.pending_state.focus.clone(); + let focus = self + .pending_state + .focus + .and_then(|w| self.current_node.tree.widget(w)); if self.current_node.tree.focused_widget() == focus.as_ref().map(|w| w.node_id) { break; } @@ -235,7 +244,7 @@ impl<'context, 'window> EventContext<'context, 'window> { drop(focus_context); if accept_focus { - break Some(focus); + break Some(focus.id()); } else if let Some(next_focus) = focus.explicit_focus_target(self.pending_state.focus_is_advancing) { @@ -244,11 +253,7 @@ impl<'context, 'window> EventContext<'context, 'window> { break self.next_focus_after(focus, self.pending_state.focus_is_advancing); } }); - let new = match self - .current_node - .tree - .focus(self.pending_state.focus.as_ref()) - { + let new = match self.current_node.tree.focus(self.pending_state.focus) { Ok(old) => { if let Some(old) = old { let mut old_context = self.for_other(&old); @@ -259,7 +264,11 @@ impl<'context, 'window> EventContext<'context, 'window> { Err(()) => false, }; if new { - if let Some(focus) = self.pending_state.focus.clone() { + if let Some(focus) = self + .pending_state + .focus + .and_then(|w| self.current_node.tree.widget(w)) + { focus.lock().as_widget().focus(&mut self.for_other(&focus)); } } else { @@ -270,13 +279,40 @@ impl<'context, 'window> EventContext<'context, 'window> { if focus_changes == MAX_ITERS { tracing::error!("focus change force stopped after {focus_changes} sequential changes"); } + + // Check that our hover widget still exists. If not, we should try to find a new one. + if let Some(hover) = self.current_node.tree.hovered_widget() { + if self.current_node.tree.widget_from_node(hover).is_none() { + self.update_hovered_widget(); + } + } } - fn next_focus_after( - &mut self, - mut focus: ManagedWidget, - advance: bool, - ) -> Option { + pub(crate) fn update_hovered_widget(&mut self) { + self.cursor.widget = None; + if let Some(location) = self.cursor.location { + for widget in self.current_node.tree.widgets_under_point(location) { + let mut widget_context = self.for_other(&widget); + let Some(widget_layout) = widget_context.last_layout() else { + continue; + }; + let relative = location - widget_layout.origin; + + if widget_context.hit_test(relative) { + widget_context.hover(relative); + drop(widget_context); + self.cursor.widget = Some(widget.id()); + break; + } + } + } + + if self.cursor.widget.is_none() { + self.clear_hover(); + } + } + + fn next_focus_after(&mut self, mut focus: ManagedWidget, advance: bool) -> Option { // First, look within the current focus for any focusable children. let stop_at = focus.id(); if let Some(focus) = self.next_focus_within(&focus, None, stop_at, advance) { @@ -304,7 +340,7 @@ impl<'context, 'window> EventContext<'context, 'window> { focus: &ManagedWidget, stop_at: WidgetId, advance: bool, - ) -> Option { + ) -> Option { self.next_focus_within(&focus.parent()?, Some(focus.id()), stop_at, advance) } @@ -317,7 +353,7 @@ impl<'context, 'window> EventContext<'context, 'window> { start_at: Option, stop_at: WidgetId, advance: bool, - ) -> Option { + ) -> Option { let mut visual_order = self.get(&LayoutOrder); if !advance { visual_order = visual_order.rev(); @@ -346,9 +382,9 @@ impl<'context, 'window> EventContext<'context, 'window> { .as_widget() .accept_focus(&mut self.for_other(&child)) { - return Some(child); + return Some(child.id()); } else if let Some(next_focus) = self.widget().explicit_focus_target(advance) { - return Some(next_focus); + return Some(next_focus.id()); } else if let Some(focus) = self.next_focus_within(&child, None, stop_at, advance) { return Some(focus); } @@ -705,6 +741,7 @@ pub struct WidgetContext<'context, 'window> { redraw_status: &'context InvalidationStatus, window: &'context mut RunningWindow<'window>, theme: Cow<'context, ThemePair>, + cursor: &'context mut CursorState, pending_state: PendingState<'context>, effective_styles: Styles, cache: WidgetCacheKey, @@ -717,6 +754,7 @@ impl<'context, 'window> WidgetContext<'context, 'window> { theme: &'context ThemePair, window: &'context mut RunningWindow<'window>, theme_mode: ThemeMode, + cursor: &'context mut CursorState, ) -> Self { let enabled = current_node.enabled(&WindowHandle { kludgine: window.handle(), @@ -727,11 +765,11 @@ impl<'context, 'window> WidgetContext<'context, 'window> { focus: current_node .tree .focused_widget() - .and_then(|id| current_node.tree.widget_from_node(id)), + .and_then(|id| current_node.tree.widget_from_node(id).map(|w| w.id())), active: current_node .tree .active_widget() - .and_then(|id| current_node.tree.widget_from_node(id)), + .and_then(|id| current_node.tree.widget_from_node(id).map(|w| w.id())), focus_is_advancing: false, }), effective_styles: current_node.effective_styles(), @@ -740,6 +778,7 @@ impl<'context, 'window> WidgetContext<'context, 'window> { enabled, invalidation: current_node.invalidation(), }, + cursor, current_node, redraw_status, theme: Cow::Borrowed(theme), @@ -757,6 +796,7 @@ impl<'context, 'window> WidgetContext<'context, 'window> { pending_state: self.pending_state.borrowed(), cache: self.cache, effective_styles: self.effective_styles.clone(), + cursor: &mut *self.cursor, } } @@ -793,6 +833,7 @@ impl<'context, 'window> WidgetContext<'context, 'window> { window: &mut *self.window, theme, pending_state: self.pending_state.borrowed(), + cursor: &mut *self.cursor, } }) } @@ -829,7 +870,7 @@ impl<'context, 'window> WidgetContext<'context, 'window> { /// contexts for the currently firing event are dropped. pub fn focus(&mut self) { self.pending_state.focus_is_advancing = true; - self.pending_state.focus = Some(self.current_node.clone()); + self.pending_state.focus = Some(self.current_node.id()); } pub(crate) fn clear_focus(&mut self) { @@ -859,16 +900,11 @@ impl<'context, 'window> WidgetContext<'context, 'window> { /// Widget events relating to activation changes are deferred until after /// the all contexts for the currently firing event are dropped. pub fn activate(&mut self) -> bool { - if self - .pending_state - .active - .as_ref() - .map_or(true, |active| active != &self.current_node) - { - self.pending_state.active = Some(self.current_node.clone()); - true - } else { + if self.pending_state.active == Some(self.current_node.id()) { false + } else { + self.pending_state.active = Some(self.current_node.id()); + true } } @@ -895,7 +931,7 @@ impl<'context, 'window> WidgetContext<'context, 'window> { /// Returns true if this widget is currently the active widget. #[must_use] pub fn active(&self) -> bool { - self.pending_state.active.as_ref() == Some(&self.current_node) + self.pending_state.active == Some(self.current_node.id()) } /// Returns true if this widget is currently hovered, even if the cursor is @@ -914,7 +950,7 @@ impl<'context, 'window> WidgetContext<'context, 'window> { /// Returns true if this widget is currently focused for user input. #[must_use] pub fn focused(&self) -> bool { - self.pending_state.focus.as_ref() == Some(&self.current_node) + self.pending_state.focus == Some(self.current_node.id()) } /// Returns true if this widget is the target to activate when the user @@ -1100,8 +1136,8 @@ enum PendingState<'a> { #[derive(Default)] struct PendingWidgetState { focus_is_advancing: bool, - focus: Option, - active: Option, + focus: Option, + active: Option, } impl PendingState<'_> { diff --git a/src/tree.rs b/src/tree.rs index 3aea4c8..462fbf7 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -231,24 +231,27 @@ impl Tree { let hovered = new_hover .map(|new_hover| data.widget_hierarchy(new_hover.node_id, self)) .unwrap_or_default(); - let unhovered = match data.update_tracked_widget(new_hover, self, |data| &mut data.hover) { - Ok(Some(old_hover)) => { - let mut old_hovered = data.widget_hierarchy(old_hover.node_id, self); - // For any widgets that were shared, remove them, as they don't - // need to have their events fired again. - let mut new_index = 0; - while !old_hovered.is_empty() && old_hovered.get(0) == hovered.get(new_index) { - old_hovered.remove(0); - new_index += 1; + let unhovered = + match data.update_tracked_widget(new_hover.map(ManagedWidget::id), self, |data| { + &mut data.hover + }) { + Ok(Some(old_hover)) => { + let mut old_hovered = data.widget_hierarchy(old_hover.node_id, self); + // For any widgets that were shared, remove them, as they don't + // need to have their events fired again. + let mut new_index = 0; + while !old_hovered.is_empty() && old_hovered.get(0) == hovered.get(new_index) { + old_hovered.remove(0); + new_index += 1; + } + old_hovered } - old_hovered - } - _ => Vec::new(), - }; + _ => Vec::new(), + }; HoverResults { unhovered, hovered } } - pub fn focus(&self, new_focus: Option<&ManagedWidget>) -> Result, ()> { + pub fn focus(&self, new_focus: Option) -> Result, ()> { let mut data = self.data.lock().ignore_poison(); data.update_tracked_widget(new_focus, self, |data| &mut data.focus) } @@ -264,7 +267,9 @@ impl Tree { new_active: Option<&ManagedWidget>, ) -> Result, ()> { let mut data = self.data.lock().ignore_poison(); - data.update_tracked_widget(new_active, self, |data| &mut data.active) + data.update_tracked_widget(new_active.map(ManagedWidget::id), self, |data| { + &mut data.active + }) } pub fn widget(&self, id: WidgetId) -> Option { @@ -334,7 +339,7 @@ impl Tree { self.data.lock().ignore_poison().focus } - pub(crate) fn widgets_at_point(&self, point: Point) -> Vec { + pub(crate) fn widgets_under_point(&self, point: Point) -> Vec { let data = self.data.lock().ignore_poison(); data.render_info.widgets_under_point(point, &data, self) } @@ -489,12 +494,13 @@ impl TreeData { fn update_tracked_widget( &mut self, - new_widget: Option<&ManagedWidget>, + new_widget: Option, tree: &Tree, property: impl FnOnce(&mut Self) -> &mut Option, ) -> Result, ()> { + let new_widget = new_widget.and_then(|w| self.widget_from_id(w, tree)); match ( - mem::replace(property(self), new_widget.map(|w| w.node_id)), + mem::replace(property(self), new_widget.as_ref().map(|w| w.node_id)), new_widget, ) { (Some(old_widget), Some(new_widget)) if old_widget == new_widget.node_id => Err(()), diff --git a/src/widgets/button.rs b/src/widgets/button.rs index 5a03829..c8ae9ee 100644 --- a/src/widgets/button.rs +++ b/src/widgets/button.rs @@ -219,11 +219,6 @@ impl Button { kind, }; - // TODO this should be genericized to happen automatically. - if !context.enabled() { - context.blur(); - } - if context.is_default() { kind.colors_for_default(visual_state, context) } else { diff --git a/src/window.rs b/src/window.rs index 073ce33..29dcad0 100644 --- a/src/window.rs +++ b/src/window.rs @@ -278,7 +278,8 @@ struct GooeyWindow { root: ManagedWidget, contents: Drawing, should_close: bool, - mouse_state: MouseState, + cursor: CursorState, + mouse_buttons: AHashMap>, redraw_status: InvalidationStatus, initial_frame: bool, occluded: Dynamic, @@ -319,6 +320,7 @@ where &self.current_theme, window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ) @@ -331,6 +333,7 @@ where &self.current_theme, window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ) @@ -345,6 +348,7 @@ where &self.current_theme, window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ) @@ -466,11 +470,11 @@ where root, contents: Drawing::default(), should_close: false, - mouse_state: MouseState { + cursor: CursorState { location: None, widget: None, - devices: AHashMap::default(), }, + mouse_buttons: AHashMap::default(), redraw_status: InvalidationStatus::default(), initial_frame: true, occluded, @@ -517,6 +521,7 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), gfx: Exclusive::Owned(Graphics::new(graphics)), }; @@ -684,11 +689,9 @@ where is_synthetic: bool, ) { let target = self.root.tree.focused_widget().unwrap_or(self.root.node_id); - let target = self - .root - .tree - .widget_from_node(target) - .expect("missing widget"); + let Some(target) = self.root.tree.widget_from_node(target) else { + return; + }; let mut window = RunningWindow::new(window, &self.focused, &self.occluded); let mut target = EventContext::new( WidgetContext::new( @@ -697,6 +700,7 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ); @@ -731,6 +735,7 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ); @@ -798,6 +803,7 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ); @@ -833,6 +839,7 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ); @@ -849,10 +856,24 @@ where position: PhysicalPosition, ) { let location = Point::::from(position); - self.mouse_state.location = Some(location); + self.cursor.location = Some(location); let mut window = RunningWindow::new(window, &self.focused, &self.occluded); - if let Some(state) = self.mouse_state.devices.get(&device_id) { + + EventContext::new( + WidgetContext::new( + self.root.clone(), + &self.redraw_status, + &self.current_theme, + &mut window, + self.theme_mode.get(), + &mut self.cursor, + ), + kludgine, + ) + .update_hovered_widget(); + + if let Some(state) = self.mouse_buttons.get(&device_id) { // Mouse Drag for (button, handler) in state { let Some(handler) = self.root.tree.widget(*handler) else { @@ -865,6 +886,7 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ); @@ -873,37 +895,6 @@ where }; context.mouse_drag(location - last_rendered_at.origin, device_id, *button); } - } else { - // Hover - let mut context = EventContext::new( - WidgetContext::new( - self.root.clone(), - &self.redraw_status, - &self.current_theme, - &mut window, - self.theme_mode.get(), - ), - kludgine, - ); - self.mouse_state.widget = None; - for widget in self.root.tree.widgets_at_point(location) { - let mut widget_context = context.for_other(&widget); - let Some(widget_layout) = widget_context.last_layout() else { - continue; - }; - let relative = location - widget_layout.origin; - - if widget_context.hit_test(relative) { - widget_context.hover(relative); - drop(widget_context); - self.mouse_state.widget = Some(widget.id()); - break; - } - } - - if self.mouse_state.widget.is_none() { - context.clear_hover(); - } } } @@ -913,7 +904,7 @@ where kludgine: &mut Kludgine, _device_id: DeviceId, ) { - if self.mouse_state.widget.take().is_some() { + if self.cursor.widget.take().is_some() { let mut window = RunningWindow::new(window, &self.focused, &self.occluded); let mut context = EventContext::new( WidgetContext::new( @@ -922,6 +913,7 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ); @@ -947,6 +939,7 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ) @@ -954,10 +947,8 @@ where if let (ElementState::Pressed, Some(location), Some(hovered)) = ( state, - &self.mouse_state.location, - self.mouse_state - .widget - .and_then(|id| self.root.tree.widget(id)), + self.cursor.location, + self.cursor.widget.and_then(|id| self.root.tree.widget(id)), ) { if let Some(handler) = recursively_handle_event( &mut EventContext::new( @@ -967,6 +958,7 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ), @@ -974,12 +966,11 @@ where let Some(layout) = context.last_layout() else { return IGNORED; }; - let relative = *location - layout.origin; + let relative = location - layout.origin; context.mouse_down(relative, device_id, button) }, ) { - self.mouse_state - .devices + self.mouse_buttons .entry(device_id) .or_default() .insert(button, handler.id()); @@ -987,18 +978,19 @@ where } } ElementState::Released => { - let Some(device_buttons) = self.mouse_state.devices.get_mut(&device_id) else { + let Some(device_buttons) = self.mouse_buttons.get_mut(&device_id) else { return; }; let Some(handler) = device_buttons.remove(&button) else { return; }; if device_buttons.is_empty() { - self.mouse_state.devices.remove(&device_id); + self.mouse_buttons.remove(&device_id); } let Some(handler) = self.root.tree.widget(handler) else { return; }; + let cursor_location = self.cursor.location; let mut context = EventContext::new( WidgetContext::new( handler, @@ -1006,12 +998,13 @@ where &self.current_theme, &mut window, self.theme_mode.get(), + &mut self.cursor, ), kludgine, ); let relative = if let (Some(last_rendered), Some(location)) = - (context.last_layout(), self.mouse_state.location) + (context.last_layout(), cursor_location) { Some(location - last_rendered.origin) } else { @@ -1060,10 +1053,9 @@ fn recursively_handle_event( } #[derive(Default)] -struct MouseState { - location: Option>, - widget: Option, - devices: AHashMap>, +pub(crate) struct CursorState { + pub(crate) location: Option>, + pub(crate) widget: Option, } pub(crate) mod sealed {